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

[Thomas Weißschuh]

On 2026-02-23 10:17:25+0000, david.laight.linux@xxxxxxxxx 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.

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

(...)