Re: [PATCH v3 next 07/17] tools/nolibc/printf: Move snprintf length check to callback

[David Laight <david.laight.linux@gmail.com>]

On Wed, 25 Feb 2026 23:37:42 +0100
Thomas Weißschuh <linux@weissschuh.net> wrote:

> On 2026-02-23 10:17:25+0000, david.laight.linux@gmail.com wrote:
> 
> (...)
> 
> > @@ -425,18 +430,25 @@ int __nolibc_printf(__nolibc_printf_cb cb, intptr_t state, size_t n, const char
> >  
> >  		/* literal char, just queue it */
> >  	}
> > +
> > +	/* Request a final '\0' be added to the snprintf() output.
> > +	 * This may be the only call of the cb() function.
> > +	 */
> > +	if (cb(state, NULL, 0) != 0)
> > +		return -1;
> > +
> >  	return written;
> >  }  
> 
> (...)
> 
> > +static int __nolibc_sprintf_cb(void *v_state, const char *buf, size_t size)
> >  {
> > -	char **state = (char **)_state;
> > +	struct __nolibc_sprintf_cb_state *state = v_state;
> > +	size_t space = state->space;
> > +	char *tgt;
> > +
> > +	/* Truncate the request to fit in the output buffer space.
> > +	 * The last byte is reserved for the terminating '\0'.
> > +	 * state->space can only be zero for snprintf(NULL, 0, fmt, args)
> > +	 * so this normally lets through calls with 'size == 0'.
> > +	 */
> > +	if (size >= space) {
> > +		if (space <= 1)
> > +			return 0;
> > +		size = space - 1;
> > +	}
> > +	tgt = state->buf;
> > +
> > +	/* __nolibc_printf() ends with cb(state, NULL, 0) to request the output
> > +	 * buffer be '\0' terminated.
> > +	 * That will be the only cb() call for, eg, snprintf(buf, sz, "").
> > +	 * Zero lengths can occur at other times (eg "%s" for an empty string).
> > +	 * Unconditionally write the '\0' byte to reduce code size, it is
> > +	 * normally overwritten by the data being output.
> > +	 * There is no point adding a '\0' after copied data - there is always
> > +	 * another call.
> > +	 */
> > +	*tgt = '\0';
> > +	state->space = space - size;
> > +	state->buf = tgt + size;
> > +	memcpy(tgt, buf, size);  
> 
> This trips UBSAN for me when 'buf == NULL'.
> 
> if (cb(state, NULL, 0) != 0)
> 	return -1;
> 
> It can be fixed by adding a NULL check around memcpy(),
> but I'd rather not do this as a random fixup.

Blame Willy, he made me remove the 'if (size)' check to reduce
the code size.
The '*tgt = 0' line is (only) needed when size is zero, the other lines
are clearly pointless.
So the 'random fixup' is adding 'if (size)' rather than a NULL
pointer check
printf("%s", "") will give a zero size with non-NULL buf.

IIRC the C standard make memcpy(tgt, NULL, 0) UB because some old system
no one has used for 40+ years would trap when NULL was loaded into an
'address register' and they wanted it to be compliant.

	David

> 
> >  
> > -	memcpy(*state, buf, size);
> > -	*state += size;
> >  	return 0;
> >  }  
> 
> (...)