Date: Thu, 26 Jan 2012 23:53:57 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Gleb Smirnoff <glebius@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r230583 - head/sys/kern Message-ID: <20120126233110.C960@besplex.bde.org> In-Reply-To: <201201261159.q0QBxma2086162@svn.freebsd.org> References: <201201261159.q0QBxma2086162@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120126233110.C960>