Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1628908

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

Show all headers | View raw

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@gmail.com>
> > ---
> >  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...

I'll take this for the next kernel (after 4.12-rc1) and see what shakes
out in linux-next...

thanks,

greg k-h

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] usb: core: Warn if an URB's transfer_buffer is on stack Florian Fainelli <f.fainelli@gmail.com> - 2017-04-22 20:40 +0200
  Re: [PATCH] usb: core: Warn if an URB's transfer_buffer is on stack Alan Stern <stern@rowland.harvard.edu> - 2017-04-22 23:40 +0200
    Re: [PATCH] usb: core: Warn if an URB's transfer_buffer is on stack Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-04-23 08:40 +0200
      Re: [PATCH] usb: core: Warn if an URB's transfer_buffer is on stack Alan Stern <stern@rowland.harvard.edu> - 2017-04-23 18:10 +0200
        Re: [PATCH] usb: core: Warn if an URB's transfer_buffer is on stack Florian Fainelli <f.fainelli@gmail.com> - 2017-04-23 19:00 +0200
          Re: [PATCH] usb: core: Warn if an URB's transfer_buffer is on stack Alan Stern <stern@rowland.harvard.edu> - 2017-04-24 02:10 +0200

csiph-web