[lvc-project] [PATCH] of: module: prevent NULL pointer dereference in vsnprintf()

[Saravana Kannan]

On Wed, Mar 27, 2024 at 9:52 AM Sergey Shtylyov <s.shtylyov at omp.ru> wrote:
>
> In of_modalias(), we can get passed the str and len parameters which would
> cause a kernel oops in vsnprintf() since it only allows passing a NULL ptr
> when the length is also 0. Also, we need to filter out the negative values
> of the len parameter as these will result in a really huge buffer since
> snprintf() takes size_t parameter while ours is ssize_t...
>
> Found by Linux Verification Center (linuxtesting.org) with the Svace static
> analysis tool.
>
> Signed-off-by: Sergey Shtylyov <s.shtylyov at omp.ru>
>
> ---
> The patch is against the for-next branch of Rob Herring's linux-git repo...
>
>  drivers/of/module.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
>
> Index: linux/drivers/of/module.c
> ===================================================================
> --- linux.orig/drivers/of/module.c
> +++ linux/drivers/of/module.c
> @@ -16,6 +16,14 @@ ssize_t of_modalias(const struct device_
>         ssize_t csize;
>         ssize_t tsize;
>
> +       /*
> +        * Prevent a kernel oops in vsnprintf() -- it only allows passing a
> +        * NULL ptr when the length is also 0. Also filter out the negative
> +        * lengths...
> +        */
> +       if ((len > 0 && !str) || len < 0)
> +               return -EINVAL;
> +

Even if vsnprintf() didn't oops, the input doesn't make sense. So, I
think the comment should just say that. Or maybe just delete the
comment since it'd be redundant at that point.

Not a strong opinion.

Acked-by: Saravana Kannan <saravanak at google.com>

-Saravana

>         /* Name & Type */
>         /* %p eats all alphanum characters, so %c must be used here */
>         csize = snprintf(str, len, "of:N%pOFn%c%s", np, 'T',