Re: [PATCH] vsprintf: fix possible NULL pointer deref in vsnprintf()

From: Steven Rostedt
Date: Fri Jan 06 2023 - 12:15:12 EST


On Fri, 6 Jan 2023 16:49:46 +0100
Petr Mladek <pmladek@xxxxxxxx> wrote:

> > Index: linux/lib/vsprintf.c
> > ===================================================================
> > --- linux.orig/lib/vsprintf.c
> > +++ linux/lib/vsprintf.c
> > @@ -2738,6 +2738,15 @@ int vsnprintf(char *buf, size_t size, co
> > if (WARN_ON_ONCE(size > INT_MAX))
> > return 0;
> >
> > + /*
> > + * C99 allows @buf to be NULL when @size is 0. We treat such NULL as if
> > + * @buf pointed to 0-sized buffer, so we can both avoid a NULL pointer
> > + * dereference and still return # of characters that would be written
> > + * if @buf pointed to a valid buffer...
> > + */
> > + if (!buf)
> > + size = 0;
>
> It makes sense except that it would hide bugs. It should print a
> warning, for example:

I agree. This is a bug, and should not be quietly ignored.

>
> char *err_msg;
>
> err_msg = check_pointer_msg(buf);
> if (err_msg) {
> WARN_ONCE(1, "Invalid buffer passed to vsnprintf(). Trying to continue with 0 length limit\n");
> size = 0;
> }

if (WARN_ONCE(err_msg, "Invalid buffer passed to vsnprintf(). Trying to continue with 0 length limit\n"))
size = 0;

;-)

-- Steve

>
> check_pointer_msg() allows to catch even more buggy pointers. WARN()
> helps to locate the caller. WARN_ONCE() variant is used to prevent
> a potential infinite loop.
>
> > +
> > str = buf;
> > end = buf + size;
> >