[PATCH 1/2] Staging: bcm: Fix an invalid dereference to a zero length kmalloc in IOCTL_BCM_BULK_WRM

[Dan Carpenter]

On Fri, Dec 16, 2011 at 09:17:13AM -0500, Kevin McKinney wrote:
> On Fri, Dec 16, 2011 at 4:37 AM, Dan Carpenter <dan.carpenter at oracle.com> wrote:
> > 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 <klmckinney1 at gmail.com>
> >> ---
> >>  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;
> 
> Yeah, that ULONG Values[1] threw me for a loop. Not sure why the array
> of one position is needed.  Why not just ULONG Values?

It's not an really array of one.  The number of elements depends on
the size of IoBuffer.InputLength.  That bit is normal.

The other thing is that longs are 32 bits or 64 bits but
IoBuffer.InputLength is given in chars.  It could be that really the
user does pass unsigned longs...  You never know.

regards,
dan carpenter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/attachments/20111216/b560b4b9/attachment.asc>