Re: [PATCH v2] usb: core: Warn if an URB's transfer_buffer is on stack

From: Felipe Balbi
Date: Tue Apr 25 2017 - 09:28:25 EST



Hi,

Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
> On Tue, Apr 25, 2017 at 01:35:33PM +0300, Maksim Salau wrote:
>> > + } else if (object_is_on_stack(urb->transfer_buffer)) {
>> > + WARN_ONCE(1, "transfer buffer is on stack\n");
>> > + ret = -EAGAIN;
>> > } else {
>>
>> Hi,
>>
>> Has anyone considered a fail-safe mode? I.e.: if a buffer is on stack,
>> kmemdup it and continue with a warning. This will give us both: functional
>> drivers (with possibly decreased efficiency in speed and memory footprint)
>> and warnings for developers that a particular driver requires attention.
>
> No, I do not want that, let's fix the drivers.
>
>> This mode will not affect drivers which obey the rules, but will make
>> offenders at least functional. My main concern is that not every user is able
>> to detect and report a problem, which prevents drivers from functioning.
>> Especially this is a problem for not wide spread devices.
>> Due to this users a seeing unusable equipment, but developers are not
>> aware of those, even if fixes are trivial.
>>
>> Such mode has a also a negative effect: if a developer has a device
>> with an offending driver, he can miss the warning message, since the driver
>> just works.
>
> Exactly, let's fix the bugs. These have been bugs for 10+ years now,
> they should get fixed, it's not complex :)

We should probably have a similar patch on
drivers/usb/gadget/udc/core.c::usb_gadget_map_request_by_dev()

--
balbi