Re: [PATCH 4/9] VC04_SERVICES: Add compat ioctl handler for "queue message"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2017-01-19 at 12:14 +0300, Dan Carpenter wrote:
> On Wed, Jan 18, 2017 at 07:04:48AM -0800, Michael Zoran wrote:
> > Add compat handler for "queue message" ioctl.
> > 
> > Signed-off-by: Michael Zoran <mzoran@xxxxxxxxxxxx>
> > ---
> >  .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 36
> > ++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git
> > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index e26949247f91..1c0afa318036 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -1271,6 +1271,41 @@ vchiq_ioctl_compat_internal(
> >  		}
> >  	} break;
> >  
> > +
> > +		for (i = 0; i < args32.count; i++) {
> > +			elements[i].data =
> > compat_ptr(elements32[i].data);
> > +			elements[i].size = elements32[i].size;
> > +		}
> > +
> > +		status = vchiq_ioc_queue_message(args32.handle,
> > +						 elements,
> > args32.count);
> 
> Btw, I notice that in vchiq_ioc_queue_message() "total_size +=
> elements[i].size;" can have an integer overflow.  What are the
> security
> implications of that?

I don't think it has much security implications. I did some checking
and I think all an integer overflow would do is truncate the message.
Meaning both the coping and the memory allocation code will both use
the truncated integer. That should be the same as if the application
just legitimately sent a truncated message.

In general, I think the firmware needs to gracefully handle garbage
messages as this driver can't prevent that and it really shouldn't be
into the business of validating garbage messages.

I do think in the case of an integer overflow, it's better to return an
error and possibly log an error message to the kernel log for easier
debugging then just let it go.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux