Messages in this thread | | | Date | Sat, 6 Jan 2018 10:09:07 +0100 | From | Greg KH <> | Subject | Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution |
| |
On Fri, Jan 05, 2018 at 05:10:32PM -0800, Dan Williams wrote: > Static analysis reports that 'index' may be a user controlled value that > is used as a data dependency to read 'pin' from the > 'selector->baSourceID' array. In order to avoid potential leaks of > kernel memory values, block speculative execution of the instruction > stream that could issue reads based on an invalid value of 'pin'. > > Based on an original patch by Elena Reshetova. > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Cc: linux-media@vger.kernel.org > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/media/usb/uvc/uvc_v4l2.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 3e7e283a44a8..7442626dc20e 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -22,6 +22,7 @@ > #include <linux/mm.h> > #include <linux/wait.h> > #include <linux/atomic.h> > +#include <linux/compiler.h> > > #include <media/v4l2-common.h> > #include <media/v4l2-ctrls.h> > @@ -810,6 +811,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh, > struct uvc_entity *iterm = NULL; > u32 index = input->index; > int pin = 0; > + __u8 *elem; > > if (selector == NULL || > (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) { > @@ -820,8 +822,9 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh, > break; > } > pin = iterm->id; > - } else if (index < selector->bNrInPins) { > - pin = selector->baSourceID[index]; > + } else if ((elem = nospec_array_ptr(selector->baSourceID, index, > + selector->bNrInPins))) { > + pin = *elem;
I dug through this before, and I couldn't find where index came from userspace, I think seeing the coverity rule would be nice.
And if this value really is user controlled, then why is this the only v4l driver that is affected? This is a common callback.
thanks,
greg k-h
| |