When vbin_printf() is called with size==0, end equals bin_buf and
the else branch writes end[-1], which is one byte before the buffer.
Guard the write so it only happens when the buffer is non-empty.
Signed-off-by: Josh Law <objecting@objecting.org>
---
lib/vsprintf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7898fb998b21..72fbfe181076 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3234,7 +3234,7 @@ int vbin_printf(u32 *bin_buf, size_t size, const char *fmt_str, va_list args)
spec);
if (str + 1 < end)
*str++ = '\0';
- else
+ else if (end > (char *)bin_buf)
end[-1] = '\0'; /* Must be nul terminated */
}
/* skip all alphanumeric pointer suffixes */
--
2.34.1On Tue, 24 Mar 2026 22:49:38 +0000 Josh Law <objecting@objecting.org> wrote: > When vbin_printf() is called with size==0, end equals bin_buf and > the else branch writes end[-1], which is one byte before the buffer. > > Guard the write so it only happens when the buffer is non-empty. > > Signed-off-by: Josh Law <objecting@objecting.org> > --- > lib/vsprintf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index 7898fb998b21..72fbfe181076 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -3234,7 +3234,7 @@ int vbin_printf(u32 *bin_buf, size_t size, const char *fmt_str, va_list args) > spec); > if (str + 1 < end) > *str++ = '\0'; > - else > + else if (end > (char *)bin_buf) > end[-1] = '\0'; /* Must be nul terminated */ instead of doing a comparison, couldn't we just check size? else if (size) /* do nothing if size is zero */ I'm guessing this would be slightly more efficient in the assembly. -- Steve > } > /* skip all alphanumeric pointer suffixes */
---- On Mon, 30 Mar 2026 17:30:19 +0100 rostedt@goodmis.org wrote ---- > On Tue, 24 Mar 2026 22:49:38 +0000 > Josh Law wrote: > > > When vbin_printf() is called with size==0, end equals bin_buf and > > the else branch writes end[-1], which is one byte before the buffer. > > > > Guard the write so it only happens when the buffer is non-empty. > > > > Signed-off-by: Josh Law > > > --- > > lib/vsprintf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > > index 7898fb998b21..72fbfe181076 100644 > > --- a/lib/vsprintf.c > > +++ b/lib/vsprintf.c > > @@ -3234,7 +3234,7 @@ int vbin_printf(u32 *bin_buf, size_t size, const char > *fmt_str, va_list args) > > spec); > > if (str + 1 < end) > > *str++ = '\0'; > > - else > > + else if (end > (char *)bin_buf) > > end[-1] = '\0'; /* Must be nul terminated */ > > instead of doing a comparison, couldn't we just check size? > > else if (size) /* do nothing if size is zero */ Good call. I'll make a V2 soon on that patch. > > I'm guessing this would be slightly more efficient in the assembly. > > -- Steve > > > } > > /* skip all alphanumeric pointer suffixes */
---- On Mon, 30 Mar 2026 17:30:19 +0100 rostedt@goodmis.org wrote ---- > On Tue, 24 Mar 2026 22:49:38 +0000 > Josh Law wrote: > > > When vbin_printf() is called with size==0, end equals bin_buf and > > the else branch writes end[-1], which is one byte before the buffer. > > > > Guard the write so it only happens when the buffer is non-empty. > > > > Signed-off-by: Josh Law > > > --- > > lib/vsprintf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > > index 7898fb998b21..72fbfe181076 100644 > > --- a/lib/vsprintf.c > > +++ b/lib/vsprintf.c > > @@ -3234,7 +3234,7 @@ int vbin_printf(u32 *bin_buf, size_t size, const char > *fmt_str, va_list args) > > spec); > > if (str + 1 < end) > > *str++ = '\0'; > > - else > > + else if (end > (char *)bin_buf) > > end[-1] = '\0'; /* Must be nul terminated */ > > instead of doing a comparison, couldn't we just check size? > > else if (size) /* do nothing if size is zero */ > > I'm guessing this would be slightly more efficient in the assembly. > > -- Steve > > > } > > /* skip all alphanumeric pointer suffixes */
---- On Mon, 30 Mar 2026 17:30:19 +0100 rostedt@goodmis.org wrote ---- > On Tue, 24 Mar 2026 22:49:38 +0000 > Josh Law wrote: > > > When vbin_printf() is called with size==0, end equals bin_buf and > > the else branch writes end[-1], which is one byte before the buffer. > > > > Guard the write so it only happens when the buffer is non-empty. > > > > Signed-off-by: Josh Law > > > --- > > lib/vsprintf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > > index 7898fb998b21..72fbfe181076 100644 > > --- a/lib/vsprintf.c > > +++ b/lib/vsprintf.c > > @@ -3234,7 +3234,7 @@ int vbin_printf(u32 *bin_buf, size_t size, const char > *fmt_str, va_list args) > > spec); > > if (str + 1 < end) > > *str++ = '\0'; > > - else > > + else if (end > (char *)bin_buf) > > end[-1] = '\0'; /* Must be nul terminated */ > > instead of doing a comparison, couldn't we just check size? > > else if (size) /* do nothing if size is zero */ > > I'm guessing this would be slightly more efficient in the assembly. > > -- Steve > > > } > > /* skip all alphanumeric pointer suffixes */
---- On Mon, 30 Mar 2026 17:30:19 +0100 rostedt@goodmis.org wrote ---- > On Tue, 24 Mar 2026 22:49:38 +0000 > Josh Law wrote: > > > When vbin_printf() is called with size==0, end equals bin_buf and > > the else branch writes end[-1], which is one byte before the buffer. > > > > Guard the write so it only happens when the buffer is non-empty. > > > > Signed-off-by: Josh Law > > > --- > > lib/vsprintf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > > index 7898fb998b21..72fbfe181076 100644 > > --- a/lib/vsprintf.c > > +++ b/lib/vsprintf.c > > @@ -3234,7 +3234,7 @@ int vbin_printf(u32 *bin_buf, size_t size, const char > *fmt_str, va_list args) > > spec); > > if (str + 1 < end) > > *str++ = '\0'; > > - else > > + else if (end > (char *)bin_buf) > > end[-1] = '\0'; /* Must be nul terminated */ > > instead of doing a comparison, couldn't we just check size? > > else if (size) /* do nothing if size is zero */ > > I'm guessing this would be slightly more efficient in the assembly. > > -- Steve > > > } > > /* skip all alphanumeric pointer suffixes */
On Tue 2026-03-24 22:49:38, Josh Law wrote: > When vbin_printf() is called with size==0, end equals bin_buf and > the else branch writes end[-1], which is one byte before the buffer. > > Guard the write so it only happens when the buffer is non-empty. > > Signed-off-by: Josh Law <objecting@objecting.org> Great catch! There is only one in-tree user and it never passes size=0 so there was no overflow in the reality. But better be on the safe side. Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr
© 2016 - 2026 Red Hat, Inc.