Re: [PATCH] usb: core: fix quirks_param_set() writing to a const pointer

Greg Kroah-Hartman
2020-07-06T10:34:10Z

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