svn commit: r306337 - head/sys/kern
Lohith Bellad
lohith.bellad at me.com
Tue Oct 4 21:27:42 UTC 2016
Hi Gleb and Hiren,
> On Oct 4, 2016, at 1:59 PM, Hiren Panchasara <hiren at FreeBSD.org> wrote:
>
> + Lohith
>
> On 10/04/16 at 01:53P, Gleb Smirnoff wrote:
>> Hiren,
>>
>> On Mon, Sep 26, 2016 at 10:13:58AM +0000, Hiren Panchasara wrote:
>> H> Author: hiren
>> H> Date: Mon Sep 26 10:13:58 2016
>> H> New Revision: 306337
>> H> URL: https://svnweb.freebsd.org/changeset/base/306337
>> H>
>> H> Log:
>> H> In sendit(), if mp->msg_control is present, then in sockargs() we are allocating
>> H> mbuf to store mp->msg_control. Later in kern_sendit(), call to getsock_cap(),
>> H> will check validity of file pointer passed, if this fails EBADF is returned but
>> H> mbuf allocated in sockargs() is not freed. Fix this possible leak.
>> H>
>> H> Submitted by: Lohith Bellad <lohith.bellad at me.com>
>> H> Reviewed by: adrian
>> H> MFC after: 3 weeks
>> H> Differential Revision: https://reviews.freebsd.org/D7910
>>
>> The commit appeared to be incorrect, but a problem exists. I'd like to look at it.
>> Is there any reproduce recipe for the leak or bug filed?
>>
I figured out a way to prevent this leak. I completed running stress test on the image built with this patch and it looks fine. I will send out for review once I reach my FreeBSD system tonight.
This is regarding the following commit, which led to kernel panic!!!
https://svnweb.freebsd.org/base?view=revision&revision=306337 <https://svnweb.freebsd.org/base?view=revision&revision=306337>
Discussion thread regarding the kernel panic,
https://lists.freebsd.org/pipermail/svn-src-head/2016-September/092110.html <https://lists.freebsd.org/pipermail/svn-src-head/2016-September/092110.html>
Thanks a lot for the input and sorry for the trouble created.
Modified diff:
Since its not possible to check and free the control mbuf correclty in sendit() routine.
We can clear the control mbuf in kern_sendit() routine after checking correctly.
Here is the diff,
Index: sys/kern/uipc_syscalls.c
===================================================================
--- sys/kern/uipc_syscalls.c (revision 305955)
+++ sys/kern/uipc_syscalls.c (working copy)
@@ -809,6 +809,9 @@
}
if (error == 0)
td->td_retval[0] = len - auio.uio_resid;
+
+ /* call to sosend would have cleared control */
+ control = NULL;
#ifdef KTRACE
if (ktruio != NULL) {
ktruio->uio_resid = td->td_retval[0];
@@ -816,6 +819,8 @@
}
#endif
bad:
+ if (control != NULL)
+ m_freem(control);
fdrop(fp, td);
return (error);
}
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.
Cheers,
Lohith
More information about the svn-src-head
mailing list