From owner-svn-src-head@FreeBSD.ORG Thu Jan 26 12:54:00 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DC701106566C; Thu, 26 Jan 2012 12:54:00 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail06.syd.optusnet.com.au (mail06.syd.optusnet.com.au [211.29.132.187]) by mx1.freebsd.org (Postfix) with ESMTP id 71C548FC14; Thu, 26 Jan 2012 12:54:00 +0000 (UTC) Received: from c211-30-171-136.carlnfd1.nsw.optusnet.com.au (c211-30-171-136.carlnfd1.nsw.optusnet.com.au [211.30.171.136]) by mail06.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q0QCrv3v030889 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 26 Jan 2012 23:53:58 +1100 Date: Thu, 26 Jan 2012 23:53:57 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Gleb Smirnoff In-Reply-To: <201201261159.q0QBxma2086162@svn.freebsd.org> Message-ID: <20120126233110.C960@besplex.bde.org> References: <201201261159.q0QBxma2086162@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed 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-head@freebsd.org X-Mailman-Version: 2.1.5 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: Thu, 26 Jan 2012 12:54:01 -0000 On Thu, 26 Jan 2012, Gleb Smirnoff wrote: > Log: > Although aio_nbytes is size_t, later is is signed to > casted types: to ssize_t in filesystem code and to > int in buf code, thus supplying a negative argument > leads to kernel panic later. And supplying a large positive argument leads to undefined behaviour later in the buf case. > To fix that check user > supplied argument in the beginning of syscall. Now, supplying a large positive argument gives implementation- defined behaviour immediately. Sometimes the implementation- defined behaviour is to have no effect immediately and allow a panic later. > Modified: head/sys/kern/vfs_aio.c > ============================================================================== > --- head/sys/kern/vfs_aio.c Thu Jan 26 11:15:12 2012 (r230582) > +++ head/sys/kern/vfs_aio.c Thu Jan 26 11:59:48 2012 (r230583) > @@ -1552,6 +1552,12 @@ aio_aqueue(struct thread *td, struct aio > return (error); > } > > + /* XXX: aio_nbytes is later casted to signed types. */ > + if ((int)aiocbe->uaiocb.aio_nbytes < 0) { This should avoid implementation-defined behaviour by checking if (uncast)aiocbe->uaiocb.aio_nbytes > INT_MAX. It isn't clear what the effects of the implementation-defined behaviour are even when you know what it is. It certainly results in the check passing values that exceed INT_MAX. For example, with 32-bit int and 64-bit size_t, and everything 2's complement, and no perverse implementation-defined behaviour like "the result of converting to int when the argment exceeds INT_MAX is always 42", then (int)0x10000000 is 0. > + uma_zfree(aiocb_zone, aiocbe); > + return (EINVAL); > + } > + > if (aiocbe->uaiocb.aio_sigevent.sigev_notify != SIGEV_KEVENT && > aiocbe->uaiocb.aio_sigevent.sigev_notify != SIGEV_SIGNAL && > aiocbe->uaiocb.aio_sigevent.sigev_notify != SIGEV_THREAD_ID && Bruce