linux-kernel - Re: [PATCH] usb: core: fix quirks_param

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200706103405.GA11622@kroah.com>
Date:   Mon, 6 Jul 2020 12:34:05 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Kars Mulder <kerneldev@...smulder.nl>
Cc:     linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        David Laight <David.Laight@...lab.com>,
        Kai-Heng Feng <kai.heng.feng@...onical.com>,
        Pavel Machek <pavel@...x.de>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Oliver Neukum <oneukum@...e.com>
Subject: Re: [PATCH] usb: core: fix quirks_param_set() writing to a const
 pointer

On Sun, Jul 05, 2020 at 11:53:27PM +0200, Kars Mulder wrote:
> The function quirks_param_set() takes as argument a const char* pointer
> to the new value of the usbcore.quirks parameter. It then casts this
> pointer to a non-const char* pointer and passes it to the strsep()
> function, which overwrites the value.
> 
> Fix this by copying the value to a local buffer on the stack and 
> letting that buffer be written to by strsep().
> 
> Fixes: 027bd6cafd9a ("usb: core: Add "quirks" parameter for usbcore")
> Signed-off-by: Kars Mulder <kerneldev@...smulder.nl>
> 
> ---
>  drivers/usb/core/quirks.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index e0b77674869c..86b1a6739b4e 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -12,6 +12,8 @@
>  #include <linux/usb/hcd.h>
>  #include "usb.h"
>  
> +#define QUIRKS_PARAM_SIZE 128
> +
>  struct quirk_entry {
>  	u16 vid;
>  	u16 pid;
> @@ -23,19 +25,21 @@ static DEFINE_MUTEX(quirk_mutex);
>  static struct quirk_entry *quirk_list;
>  static unsigned int quirk_count;
>  
> -static char quirks_param[128];
> +static char quirks_param[QUIRKS_PARAM_SIZE];
>  
> -static int quirks_param_set(const char *val, const struct kernel_param *kp)
> +static int quirks_param_set(const char *value, const struct kernel_param *kp)
>  {
> +	char val[QUIRKS_PARAM_SIZE];

That's a lot of stack space, is it really needed?  Can we just use a
static variable instead, or dynamically allocate this?

thanks,

greg k-h