Re: [PATCH 2.6.32-4.0] sg_start_req(): make sure that there's not too many elements in iovec

[Luis Henriques <luis.henriques@canonical.com>]

On Sat, Aug 01, 2015 at 06:25:59PM +0100, Ben Hutchings wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> commit 451a2886b6bf90e2fb378f7c46c655450fb96e81 upstream.
> 
> unfortunately, allowing an arbitrary 16bit value means a possibility of
> overflow in the calculation of total number of pages in bio_map_user_iov() -
> we rely on there being no more than PAGE_SIZE members of sum in the
> first loop there.  If that sum wraps around, we end up allocating
> too small array of pointers to pages and it's easy to overflow it in
> the second loop.
> 
> X-Coverup: TINC (and there's no lumber cartel either)
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> [bwh: s/MAX_UIOVEC/UIO_MAXIOV/. This was fixed upstream by commit
>  fdc81f45e9f5 ("sg_start_req(): use import_iovec()"), but we don't have
>  that function.]
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> It looks like this bug was introduced in 2.6.28 by commit 10db10d144c0
> ("sg: convert the indirect IO path to use the block layer"), so the fix
> is needed for all stable branches.
> 
> Ben.

Thanks Ben, queuing it for the 3.16 kernel.

Cheers,
--
Lu�s

> 
>  drivers/scsi/sg.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -1687,6 +1687,9 @@ static int sg_start_req(Sg_request *srp,
>  			md->from_user = 0;
>  	}
>  
> +	if (unlikely(iov_count > UIO_MAXIOV))
> +		return -EINVAL;
> +
>  	if (iov_count) {
>  		int len, size = sizeof(struct sg_iovec) * iov_count;
>  		struct iovec *iov;
> -- 
> Ben Hutchings
> One of the nice things about standards is that there are so many of them.
>