Rework: Fix to avoid potential mbuf leak in network stack, if userland sends control messages to bad file descriptors.

[lohithbsd_gmail.com]

Summary

In sendit(), if mp->msg_control is present, then in sockargs() we are allocating mbuf to store mp->msg_control. Later in kern_sendit(), call to getsock_cap(), will check validity of file pointer passed, if this fails EBADF is returned but mbuf allocated in sockargs() is not freed. Made code changes to free the same.

Since its not possible to check and free the control mbuf correctly in sendit() routine.
We can clear the control mbuf in kern_sendit() routine after checking correctly.

Since, we know for sure sosend() routine will consume the control mbuf if its present else it will clear the mbuf. So, making control = NULL, after the call to sosend() will prevent double freeing of control mbuf.

If there are any errors before call to sosend() in kern_sendit(), for example EBADF (Bad File Descriptor) then we will fall to "bad:" and if control != NULL, we will clear the mbuf. This way mbuf leak for EBADF is also prevented.

Test Plan

Built custom kernel with this change and ran stress test suite.

[glebius]

I'd suggest to just put m_freem(control) into all error places, including getsock_cap() failure, instead of editing end of function.

sys/kern/uipc_syscalls.c

766 ↗

(On Diff #21052)

Doesn't the leak happen here?

[lohithbsd_gmail.com]

Made suitable modifications to take care of all return paths leading to leak.

[lohithbsd_gmail.com]

yes, taken care. Thanks!

[lohithbsd_gmail.com]

Made code changes to clear control mbuf at all error places instead of at end of function.

[glebius]

m_freem() can accept NULL pointer just like free(). I'd suggest to make code more laconic and remove ifs.

[lohithbsd_gmail.com]

Removed redundant (control != NULL) checks since m_freem() checks for NULL pointers.