NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/60749: compat: successive copyout error branch mistakes



>Number:         60749
>Category:       kern
>Synopsis:       compat: successive copyout error branch mistakes
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Sep 20 10:50:00 +0000 2026
>Originator:     Taylor R Campbell
>Release:        current, 11, 10, ...
>Organization:
The CompatBSD Copying, Ouc.
>Environment:
>Description:

	compat_43_sys_recvmsg copies out the socket name, and then only
	if that _failed_ copies out the msghdr -- presumably it was
	meant to copy out the msghdr only if that _succeeded_:

    271 	error = copyout_sockname((struct sockaddr *)omsg.msg_name, &omsg.msg_namelen, 0, from);
    272 	if (from != NULL)
    273 		m_free(from);
    274 
    275 	if (error != 0)
    276 		 error = copyout(&omsg, SCARG(uap, msg), sizeof(omsg));

	https://nxr.netbsd.org/xref/src/sys/compat/common/uipc_syscalls_43.c?r=1.51#271

	Same with the compat32 version:

    491 	error = copyout_sockname(NETBSD32PTR64(omsg.msg_name),
    492 	    &omsg.msg_namelen, 0, from);
    493 	if (from != NULL)
    494 		m_free(from);
    495 
    496 	if (error != 0)
    497 		 error = copyout(&omsg, SCARG_P32(uap, msg), sizeof(omsg));

	https://nxr.netbsd.org/xref/src/sys/compat/netbsd32/netbsd32_compat_43.c?r=1.65#491

>How-To-Repeat:

	code inspection

>Fix:

	Always, always, always structure error branches like this as

		error = ...;
		if (error)
			goto out;
		error = ...;
		if (error)
			goto out;

	instead of tangling success cases inside branches with failure
	cases.  (Exception only if there is a chain of fallbacks in
	case of failure.)




Home | Main Index | Thread Index | Old Index