lkml.org 
[lkml]   [2023]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] drivers/comedi: copy userspace array safely
From
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 )=-

\
 
 \ /
  Last update: 2023-11-20 13:47    [W:0.044 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site