From owner-svn-src-head@freebsd.org Tue Oct 4 21:27:42 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B4E5CAF5DD1 for ; Tue, 4 Oct 2016 21:27:42 +0000 (UTC) (envelope-from lohith.bellad@me.com) Received: from pv33p04im-asmtp002.me.com (pv33p04im-asmtp002.me.com [17.143.181.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 8BB89289; Tue, 4 Oct 2016 21:27:42 +0000 (UTC) (envelope-from lohith.bellad@me.com) Received: from process-dkim-sign-daemon.pv33p04im-asmtp002.me.com by pv33p04im-asmtp002.me.com (Oracle Communications Messaging Server 7.0.5.38.0 64bit (built Feb 26 2016)) id <0OEJ00300IPR5M00@pv33p04im-asmtp002.me.com>; Tue, 04 Oct 2016 21:27:28 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=me.com; s=4d515a; t=1475616447; bh=oGrFMXsvNyeQVeqCP4w9IpYU8+Nk42mr2YULlD5vc2A=; h=Content-type:MIME-version:Subject:From:Date:Message-id:To; b=VabQtiPAfvyy3jwZ7YpvQkpCzPbsmelpYd7xntHLeNFIhnoKgfZ+3W5xaYoLxHRXm WySPiCd8o4N5c8m07qVEo8EmOfu68ARmcpHCHBEpj41zMG8Fociy8auzgra/En3Sg6 rmjqXDmXzTZKV9vmqquqKI60p/qEaFs/pAXGKSxou5SpF6RYtNSDTFQR7TzBs91YYb EMp4/qhHW2u+HRWFDlfM5SLQChGa9rELagJEaiGfbg7ExzC63q0Gww1lZ1xxD9JfYH tR74rS6T+8rqAW3nd2iT9qgqF2sIbR2qcO0QMTX4rBpnZMqEfZSSCKuO47fGhnf8ZT 900np/YuLp1UA== Received: from lbellad-mba.jnpr.net (unknown [66.129.239.12]) by pv33p04im-asmtp002.me.com (Oracle Communications Messaging Server 7.0.5.38.0 64bit (built Feb 26 2016)) with ESMTPSA id <0OEJ003TJKXK4V20@pv33p04im-asmtp002.me.com>; Tue, 04 Oct 2016 21:27:26 +0000 (GMT) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-10-04_08:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 clxscore=1034 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1603290000 definitions=main-1610040368 MIME-version: 1.0 (Mac OS X Mail 8.2 \(2104\)) Subject: Re: svn commit: r306337 - head/sys/kern From: Lohith Bellad In-reply-to: <20161004205945.GA50669@strugglingcoder.info> Date: Tue, 04 Oct 2016 14:27:19 -0700 Cc: lohithbsd@gmail.com Message-id: References: <201609261013.u8QADwrV002892@repo.freebsd.org> <20161004205352.GM23123@FreeBSD.org> <20161004205945.GA50669@strugglingcoder.info> To: Hiren Panchasara , Gleb Smirnoff , svn-src-head@freebsd.org X-Mailer: Apple Mail (2.2104) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 04 Oct 2016 21:27:42 -0000 Hi Gleb and Hiren, > On Oct 4, 2016, at 1:59 PM, Hiren Panchasara = wrote: >=20 > + Lohith >=20 > On 10/04/16 at 01:53P, Gleb Smirnoff wrote: >> Hiren, >>=20 >> 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>=20 >> 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> =20 >> H> Submitted by: Lohith Bellad >> H> Reviewed by: adrian >> H> MFC after: 3 weeks >> H> Differential Revision: https://reviews.freebsd.org/D7910 >>=20 >> 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? >>=20 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=3Drevision&revision=3D306337 = Discussion thread regarding the kernel panic, = https://lists.freebsd.org/pipermail/svn-src-head/2016-September/092110.htm= l = 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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/kern/uipc_syscalls.c (revision 305955) +++ sys/kern/uipc_syscalls.c (working copy) @@ -809,6 +809,9 @@ } if (error =3D=3D 0) td->td_retval[0] =3D len - auio.uio_resid; +=20 + /* call to sosend would have cleared control */ + control =3D NULL; #ifdef KTRACE if (ktruio !=3D NULL) { ktruio->uio_resid =3D td->td_retval[0]; @@ -816,6 +819,8 @@ } #endif bad: + if (control !=3D 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 =3D NULL, = after the call to sosend() will prevent double freeing of control mbuf.=20= 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 !=3D NULL, we will clear the mbuf. This way mbuf leak for EBADF = is also prevented. Cheers, Lohith