svn commit: r230583 - head/sys/kern

[Bruce Evans]

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