On Thu, Dec 15, 2011 at 11:14:53PM -0500, Kevin McKinney wrote:
> Variable IoBuffer.InputLength is chosen from userspace,
> and can therefore be zero. If so, then we will get a kernel
> Oops. To resolve this issue, this patch checks to confirm
> IoBuffer.InputLength is not equal to zero before invoking
> the kmalloc call.
> 
> Signed-off-by: Kevin McKinney <[email protected]>
> ---
>  drivers/staging/bcm/Bcmchar.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c
> index fa4a854..47d6818 100644
> --- a/drivers/staging/bcm/Bcmchar.c
> +++ b/drivers/staging/bcm/Bcmchar.c
> @@ -1137,7 +1137,9 @@ cntrlEnd:
>               if (copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER)))
>                       return -EFAULT;
>  
> -             /* FIXME: restrict length */
> +             if (IoBuffer.InputLength == 0)
> +                     return -EINVAL;

The beceem code is not super pretty.  We're copying one of these:

typedef struct bulkwrmbuffer
{
        ULONG   Register;
        ULONG   SwapEndian;
        ULONG   Values[1];
}BULKWRM_BUFFER,*PBULKWRM_BUFFER;

Values[] is a variable length array.  It's not clear to me that
Values[1] actually stores whole longs.  It could be that it's really
just a buffer of chars.  So probably to be conservative we should
say that we have to at least have the first two members of the
struct:

                if (IoBuffer.InputLength < sizeof(ULONG) * 2)
                        return -EINVAL;

regards,
dan carpenter


Attachment: signature.asc
Description: Digital signature

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to