Messages in this thread | | | Date | Fri, 3 Nov 2023 10:25:45 +0000 | Subject | Re: [PATCH] drivers/comedi: copy userspace array safely | From | 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@mev.co.uk>
-- -=( Ian Abbott <abbotti@mev.co.uk> || 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 )=-
| |