On Sun, 23 Apr 2017, Greg Kroah-Hartman wrote: > On Sat, Apr 22, 2017 at 05:31:27PM -0400, Alan Stern wrote: > > On Sat, 22 Apr 2017, Florian Fainelli wrote: > > > > > We see a large number of fixes to several drivers to remove the usage of > > > on-stack buffers feeding into USB transfer functions. Make it easier to spot > > > the offenders by adding a warning in usb_start_wait_urb() for > > > urb->transfer_buffer to be located on the stack. > > > > > > Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx> > > > --- > > > drivers/usb/core/message.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c > > > index 2184ef40a82a..abefddbe9243 100644 > > > --- a/drivers/usb/core/message.c > > > +++ b/drivers/usb/core/message.c > > > @@ -8,6 +8,7 @@ > > > #include <linux/pci.h> /* for scatterlist macros */ > > > #include <linux/usb.h> > > > #include <linux/module.h> > > > +#include <linux/sched/task_stack.h> /* for object_is_on_stack */ > > > #include <linux/slab.h> > > > #include <linux/mm.h> > > > #include <linux/timer.h> > > > @@ -50,6 +51,8 @@ static int usb_start_wait_urb(struct urb *urb, int timeout, int *actual_length) > > > unsigned long expire; > > > int retval; > > > > > > + WARN_ON(object_is_on_stack(urb->transfer_buffer)); > > > + > > > init_completion(&ctx.done); > > > urb->context = &ctx; > > > urb->actual_length = 0; > > > > Does this really help? I mean, don't we already get warnings when > > the host controller drivers try to map on-stack buffers for DMA? > > > > The only difference is that one wouldn't have to read as far back into > > the stack trace. But that hardly seems like an important > > consideration. > > I don't think this will show up if you don't have the VMALLOC_STACKS > option enabled (or whatever it is), so this warning is good to have. I > didn't know we had that macro, as the USB stack has always required this > due to some platforms needing it, just not the "mainstream" ones... In that case, it would be better to move the warning to a central place where it will always get triggered, such as map_urb_for_dma(). As it is, the patch will only issue a warning for callers of usb_bulk_msg(), usb_interrupt_msg(), or usb_control_msg(). Alan Stern > I'll take this for the next kernel (after 4.12-rc1) and see what shakes > out in linux-next... > > thanks, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
