lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 22 Apr 2017 17:31:27 -0400 (EDT)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Florian Fainelli <f.fainelli@...il.com>
cc:     linux-kernel@...r.kernel.org, <arnd@...db.de>,
        <maksim.salau@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        David Mosberger <davidm@...uge.net>,
        Roger Quadros <rogerq@...com>,
        Wolfram Sang <wsa-dev@...g-engineering.com>,
        Oliver Neukum <oneukum@...e.com>,
        Jaejoong Kim <climbbb.kim@...il.com>,
        "open list:USB SUBSYSTEM" <linux-usb@...r.kernel.org>
Subject: Re: [PATCH] usb: core: Warn if an URB's transfer_buffer is on stack

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@...il.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.

Alan Stern

Powered by blists - more mailing lists