From owner-svn-src-all@FreeBSD.ORG Thu Jan 26 15:36:43 2012 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8E6BD1065672; Thu, 26 Jan 2012 15:36:43 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.64.117]) by mx1.freebsd.org (Postfix) with ESMTP id DDECD8FC18; Thu, 26 Jan 2012 15:36:42 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.5/8.14.5) with ESMTP id q0QFaf6o068488; Thu, 26 Jan 2012 19:36:41 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.5/8.14.5/Submit) id q0QFafwX068487; Thu, 26 Jan 2012 19:36:41 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Thu, 26 Jan 2012 19:36:41 +0400 From: Gleb Smirnoff To: Bruce Evans Message-ID: <20120126153641.GA68112@FreeBSD.org> References: <201201261159.q0QBxma2086162@svn.freebsd.org> <20120126233110.C960@besplex.bde.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="Nq2Wo0NMKNjxTN9z" Content-Disposition: inline In-Reply-To: <20120126233110.C960@besplex.bde.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r230583 - head/sys/kern X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Jan 2012 15:36:43 -0000 --Nq2Wo0NMKNjxTN9z Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Bruce, On Thu, Jan 26, 2012 at 11:53:57PM +1100, Bruce Evans wrote: B> > Log: B> > Although aio_nbytes is size_t, later is is signed to B> > casted types: to ssize_t in filesystem code and to B> > int in buf code, thus supplying a negative argument B> > leads to kernel panic later. B> B> And supplying a large positive argument leads to undefined B> behaviour later in the buf case. B> B> > To fix that check user B> > supplied argument in the beginning of syscall. B> B> Now, supplying a large positive argument gives implementation- B> defined behaviour immediately. Sometimes the implementation- B> defined behaviour is to have no effect immediately and allow B> a panic later. B> B> > Modified: head/sys/kern/vfs_aio.c B> > ============================================================================== B> > --- head/sys/kern/vfs_aio.c Thu Jan 26 11:15:12 2012 (r230582) B> > +++ head/sys/kern/vfs_aio.c Thu Jan 26 11:59:48 2012 (r230583) B> > @@ -1552,6 +1552,12 @@ aio_aqueue(struct thread *td, struct aio B> > return (error); B> > } B> > B> > + /* XXX: aio_nbytes is later casted to signed types. */ B> > + if ((int)aiocbe->uaiocb.aio_nbytes < 0) { B> B> This should avoid implementation-defined behaviour by checking if B> B> (uncast)aiocbe->uaiocb.aio_nbytes > INT_MAX. B> B> It isn't clear what the effects of the implementation-defined behaviour B> are even when you know what it is. It certainly results in the B> check passing values that exceed INT_MAX. For example, with 32-bit B> int and 64-bit size_t, and everything 2's complement, and no perverse B> implementation-defined behaviour like "the result of converting to B> int when the argment exceeds INT_MAX is always 42", then B> (int)0x10000000 is 0. Is the attached patch okay? -- Totus tuus, Glebius. --Nq2Wo0NMKNjxTN9z Content-Type: text/x-diff; charset=koi8-r Content-Disposition: attachment; filename="vfs_aio.c.diff" Index: vfs_aio.c =================================================================== --- vfs_aio.c (revision 230585) +++ vfs_aio.c (working copy) @@ -1553,7 +1553,7 @@ } /* XXX: aio_nbytes is later casted to signed types. */ - if ((int)aiocbe->uaiocb.aio_nbytes < 0) { + if (aiocbe->uaiocb.aio_nbytes > INT_MAX) { uma_zfree(aiocb_zone, aiocbe); return (EINVAL); } --Nq2Wo0NMKNjxTN9z--