Re: [PATCH] drivers/comedi: copy userspace array safely

[Ian Abbott]

On 2023-11-03 09:11, Philipp Stanner wrote:
> On Fri, 2023-11-03 at 06:53 +0100, Greg Kroah-Hartman wrote:
> > On Thu, Nov 02, 2023 at 08:08:49PM +0100, Philipp Stanner wrote:
> > > comedi_fops.c utilizes memdup_user() to copy a userspace array. This
> > > does not check for an overflow.
> >
> > Is there potential for an overflow today?
>
> None that I'm aware of, no. This is more about establishing the new
> function as the standard for array-copying, thereby improving
> readability and maybe robustness in case of future changes.

I agree there is no potential for overflow.  The chanlist_len in the 
command is bound checked against the len_chanlist in the comedi 
subdevice in __comedi_get_user_cmd(), and the len_chanlist value is set 
by driver code with no user input.  So it should be fine barring some 
rogue comedi driver.

> > > Use the new wrapper memdup_array_user() to copy the array more safely.
> >
> > How about saying something like:
> >          "Use the new function memdup_array_user() in case things change
> >          in the future which would prevent overflows if something were to
> >          change in the size of the structures".
> >
> > Or something to the affect of "all is good today, but make it easy to be
> > correct in the future as well".
>
> Yes, good idea. I'll send a better wording

Feel free to add my reviewed by line:

Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>

--
-=( Ian Abbott <abbotti@xxxxxxxxx> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-