From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from todd.t-8ch.de (todd.t-8ch.de [159.69.126.157]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2BAFD3033F1 for ; Thu, 26 Feb 2026 21:29:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=159.69.126.157 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772141356; cv=none; b=NiD15ESJq2Cgo4IhOqWMS6AYLfb6Io9iLBi8zQYYghA0dHD0PW7lPZeZ+fBO70REpl4SD5Xd28e1A64eoLPPWUBXoAG28ZxPhVLxw8XA/Q95MJq4JCP/ZYpBC7DZNZ6rCWY3gxwvWkQeyMQ0Xz7mPpeX8Hu9XfNL/Beu6xZzJpo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772141356; c=relaxed/simple; bh=LbJWDGn/lCUE5DVmeG840G7C4tLo3ROHe14tLhoiHfM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=rB/HPmkI1IOB0F+gUeFkVFRL8h27IyLcYtO6G7NRcPvuqnnGUoszXeHlhWKQdf1f/vXR64NrxTQPIGOp1lhEXUKO9RocABK9mFRE1nrxMEF3lY01b9xDzhIaImWy1ryvRkeoGOMHi1JWxhKvV5tQe33tba4vhD34e1vbukw/V/Y= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=weissschuh.net; spf=pass smtp.mailfrom=weissschuh.net; dkim=pass (1024-bit key) header.d=weissschuh.net header.i=@weissschuh.net header.b=ZQ4p2fxv; arc=none smtp.client-ip=159.69.126.157 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=weissschuh.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=weissschuh.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=weissschuh.net header.i=@weissschuh.net header.b="ZQ4p2fxv" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=weissschuh.net; s=mail; t=1772141347; bh=LbJWDGn/lCUE5DVmeG840G7C4tLo3ROHe14tLhoiHfM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZQ4p2fxv+nGN4lP3eXVi36phXewH0lcMnxs/3VNpJPGTHIpvUg+JYxk0iNuMQA81D Ldkn4GqnZ0KwZ9HL925swdLWXUEKUxaApDcmUGjjK63deBo0HFE34GaV+6+N1sWEEV UvjgHS6q9cI1FkFcIpmzMu2OEHDPu+1WWMsSp4Jc= Date: Thu, 26 Feb 2026 22:29:07 +0100 From: Thomas =?utf-8?Q?Wei=C3=9Fschuh?= To: David Laight Cc: Willy Tarreau , linux-kernel@vger.kernel.org, Cheng Li Subject: Re: [PATCH v3 next 07/17] tools/nolibc/printf: Move snprintf length check to callback Message-ID: References: <20260223101735.2922-1-david.laight.linux@gmail.com> <20260223101735.2922-8-david.laight.linux@gmail.com> <20260225231221.70b0531f@pumpkin> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260225231221.70b0531f@pumpkin> On 2026-02-25 23:12:21+0000, David Laight wrote: > On Wed, 25 Feb 2026 23:37:42 +0100 > Thomas Weißschuh 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. Done. But we still can't hard-break our own testsuite. If we can detect UBSAN at build-time, that could work. But I would prefer to just add the check. > 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. Can you roll this into the next revision? > 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. Fair enough. But how would this work for functions where NULL is an allowed argument value like 'free()'? Anyways, we use UBSAN in the testsuite and it actually found a bunch of issues, so I'd like to keep it. > > > > > > - memcpy(*state, buf, size); > > > - *state += size; > > > return 0; > > > } > > > > (...)