public inbox for linux-hardening@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
@ 2026-03-16 16:17 Josh Law
  2026-03-16 16:29 ` Greg KH
  2026-03-16 16:30 ` Andy Shevchenko
  0 siblings, 2 replies; 8+ messages in thread
From: Josh Law @ 2026-03-16 16:17 UTC (permalink / raw)
  To: kees, akpm; +Cc: andy, linux-hardening, linux-kernel, Josh Law

BUG_ON() in a library function is too harsh -- it panics the kernel
when a caller passes a dest string whose length already meets or
exceeds count. This is a caller bug, not a reason to bring down the
entire system.

Replace with WARN_ON_ONCE() and a safe early return. The return value
of count signals truncation to the caller, consistent with strlcat
semantics (return >= count means the output was truncated).

This follows the guidance in include/asm-generic/bug.h which
explicitly discourages BUG_ON: "Don't use BUG() or BUG_ON() unless
there's really no way out."

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/string.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index b632c71df1a5..ae3eb192534d 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -255,8 +255,9 @@ size_t strlcat(char *dest, const char *src, size_t count)
 	size_t len = strlen(src);
 	size_t res = dsize + len;
 
-	/* This would be a bug */
-	BUG_ON(dsize >= count);
+	/* This would be a caller bug */
+	if (WARN_ON_ONCE(dsize >= count))
+		return count;
 
 	dest += dsize;
 	count -= dsize;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
  2026-03-16 16:17 [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat Josh Law
@ 2026-03-16 16:29 ` Greg KH
  2026-03-16 16:30 ` Andy Shevchenko
  1 sibling, 0 replies; 8+ messages in thread
From: Greg KH @ 2026-03-16 16:29 UTC (permalink / raw)
  To: Josh Law; +Cc: kees, akpm, andy, linux-hardening, linux-kernel

On Mon, Mar 16, 2026 at 04:17:28PM +0000, Josh Law wrote:
> BUG_ON() in a library function is too harsh -- it panics the kernel
> when a caller passes a dest string whose length already meets or
> exceeds count. This is a caller bug, not a reason to bring down the
> entire system.
> 
> Replace with WARN_ON_ONCE() and a safe early return. The return value
> of count signals truncation to the caller, consistent with strlcat
> semantics (return >= count means the output was truncated).
> 
> This follows the guidance in include/asm-generic/bug.h which
> explicitly discourages BUG_ON: "Don't use BUG() or BUG_ON() unless
> there's really no way out."
> 
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  lib/string.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/string.c b/lib/string.c
> index b632c71df1a5..ae3eb192534d 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -255,8 +255,9 @@ size_t strlcat(char *dest, const char *src, size_t count)
>  	size_t len = strlen(src);
>  	size_t res = dsize + len;
>  
> -	/* This would be a bug */
> -	BUG_ON(dsize >= count);
> +	/* This would be a caller bug */
> +	if (WARN_ON_ONCE(dsize >= count))
> +		return count;

You still just crashed the system for the few billion or so Linux
systems out there with panic-on-warn enabled :(

And is returning 'count' really the correct thing to do here when you
didn't actually copy any data?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
  2026-03-16 16:17 [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat Josh Law
  2026-03-16 16:29 ` Greg KH
@ 2026-03-16 16:30 ` Andy Shevchenko
  2026-03-16 16:36   ` Josh Law
  1 sibling, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2026-03-16 16:30 UTC (permalink / raw)
  To: Josh Law; +Cc: kees, akpm, andy, linux-hardening, linux-kernel

On Mon, Mar 16, 2026 at 04:17:28PM +0000, Josh Law wrote:
> BUG_ON() in a library function is too harsh -- it panics the kernel
> when a caller passes a dest string whose length already meets or
> exceeds count. This is a caller bug, not a reason to bring down the
> entire system.
> 
> Replace with WARN_ON_ONCE() and a safe early return. The return value
> of count signals truncation to the caller, consistent with strlcat
> semantics (return >= count means the output was truncated).
> 
> This follows the guidance in include/asm-generic/bug.h which
> explicitly discourages BUG_ON: "Don't use BUG() or BUG_ON() unless
> there's really no way out."

First of all, this doesn't really change much, especially if one uses
panic_on_oops.

Second, the entire function is kinda deprecated, it's better just to drop it.

$ git grep -lw strlcat | wc -l
142

(In reality it's less as tools and implementation are not users of it)

Third, if the caller is that problematic this may lead to the serious
(security) issues.

Based on that, NAK.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
  2026-03-16 16:30 ` Andy Shevchenko
@ 2026-03-16 16:36   ` Josh Law
  2026-03-16 16:43     ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Josh Law @ 2026-03-16 16:36 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: kees, akpm, andy, linux-hardening, linux-kernel



On 16 March 2026 16:30:15 GMT, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>On Mon, Mar 16, 2026 at 04:17:28PM +0000, Josh Law wrote:
>> BUG_ON() in a library function is too harsh -- it panics the kernel
>> when a caller passes a dest string whose length already meets or
>> exceeds count. This is a caller bug, not a reason to bring down the
>> entire system.
>> 
>> Replace with WARN_ON_ONCE() and a safe early return. The return value
>> of count signals truncation to the caller, consistent with strlcat
>> semantics (return >= count means the output was truncated).
>> 
>> This follows the guidance in include/asm-generic/bug.h which
>> explicitly discourages BUG_ON: "Don't use BUG() or BUG_ON() unless
>> there's really no way out."
>
>First of all, this doesn't really change much, especially if one uses
>panic_on_oops.
>
>Second, the entire function is kinda deprecated, it's better just to drop it.
>
>$ git grep -lw strlcat | wc -l
>142
>
>(In reality it's less as tools and implementation are not users of it)
>
>Third, if the caller is that problematic this may lead to the serious
>(security) issues.
>
>Based on that, NAK.
>


Hmm, good call, but how would I implement that? BUG_ON seems a bit too harsh for a library function.

V/R


Josh Law

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
  2026-03-16 16:36   ` Josh Law
@ 2026-03-16 16:43     ` Andy Shevchenko
  2026-03-16 16:46       ` Josh Law
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2026-03-16 16:43 UTC (permalink / raw)
  To: Josh Law; +Cc: kees, akpm, andy, linux-hardening, linux-kernel

On Mon, Mar 16, 2026 at 04:36:40PM +0000, Josh Law wrote:
> On 16 March 2026 16:30:15 GMT, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> >On Mon, Mar 16, 2026 at 04:17:28PM +0000, Josh Law wrote:
> >> BUG_ON() in a library function is too harsh -- it panics the kernel
> >> when a caller passes a dest string whose length already meets or
> >> exceeds count. This is a caller bug, not a reason to bring down the
> >> entire system.
> >> 
> >> Replace with WARN_ON_ONCE() and a safe early return. The return value
> >> of count signals truncation to the caller, consistent with strlcat
> >> semantics (return >= count means the output was truncated).
> >> 
> >> This follows the guidance in include/asm-generic/bug.h which
> >> explicitly discourages BUG_ON: "Don't use BUG() or BUG_ON() unless
> >> there's really no way out."
> >
> >First of all, this doesn't really change much, especially if one uses
> >panic_on_oops.
> >
> >Second, the entire function is kinda deprecated, it's better just to drop it.
> >
> >$ git grep -lw strlcat | wc -l
> >142
> >
> >(In reality it's less as tools and implementation are not users of it)
> >
> >Third, if the caller is that problematic this may lead to the serious
> >(security) issues.
> >
> >Based on that, NAK.
> 
> Hmm, good call, but how would I implement that? BUG_ON seems a bit too harsh
> for a library function.

Drop the function. Start from converting users to strscpy() or similar
(we have a few for different cases) and then drop strlcat() all for good.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
  2026-03-16 16:43     ` Andy Shevchenko
@ 2026-03-16 16:46       ` Josh Law
  2026-03-16 16:51         ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Josh Law @ 2026-03-16 16:46 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: kees, akpm, andy, linux-hardening, linux-kernel



On 16 March 2026 16:43:12 GMT, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>On Mon, Mar 16, 2026 at 04:36:40PM +0000, Josh Law wrote:
>> On 16 March 2026 16:30:15 GMT, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>> >On Mon, Mar 16, 2026 at 04:17:28PM +0000, Josh Law wrote:
>> >> BUG_ON() in a library function is too harsh -- it panics the kernel
>> >> when a caller passes a dest string whose length already meets or
>> >> exceeds count. This is a caller bug, not a reason to bring down the
>> >> entire system.
>> >> 
>> >> Replace with WARN_ON_ONCE() and a safe early return. The return value
>> >> of count signals truncation to the caller, consistent with strlcat
>> >> semantics (return >= count means the output was truncated).
>> >> 
>> >> This follows the guidance in include/asm-generic/bug.h which
>> >> explicitly discourages BUG_ON: "Don't use BUG() or BUG_ON() unless
>> >> there's really no way out."
>> >
>> >First of all, this doesn't really change much, especially if one uses
>> >panic_on_oops.
>> >
>> >Second, the entire function is kinda deprecated, it's better just to drop it.
>> >
>> >$ git grep -lw strlcat | wc -l
>> >142
>> >
>> >(In reality it's less as tools and implementation are not users of it)
>> >
>> >Third, if the caller is that problematic this may lead to the serious
>> >(security) issues.
>> >
>> >Based on that, NAK.
>> 
>> Hmm, good call, but how would I implement that? BUG_ON seems a bit too harsh
>> for a library function.
>
>Drop the function. Start from converting users to strscpy() or similar
>(we have a few for different cases) and then drop strlcat() all for good.
>


Okie dokie. I'm doing a patch on that right now, keep your eyes peeled on your emails, thanks for the suggestion 


V/R


Josh Law

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
  2026-03-16 16:46       ` Josh Law
@ 2026-03-16 16:51         ` Andy Shevchenko
  2026-03-16 16:52           ` Josh Law
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2026-03-16 16:51 UTC (permalink / raw)
  To: Josh Law; +Cc: kees, akpm, andy, linux-hardening, linux-kernel

On Mon, Mar 16, 2026 at 04:46:47PM +0000, Josh Law wrote:
> On 16 March 2026 16:43:12 GMT, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> >On Mon, Mar 16, 2026 at 04:36:40PM +0000, Josh Law wrote:

...

> >Drop the function. Start from converting users to strscpy() or similar
> >(we have a few for different cases) and then drop strlcat() all for good.
> 
> Okie dokie. I'm doing a patch on that right now, keep your eyes peeled on
> your emails, thanks for the suggestion 

I'm taking my breath!

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
  2026-03-16 16:51         ` Andy Shevchenko
@ 2026-03-16 16:52           ` Josh Law
  0 siblings, 0 replies; 8+ messages in thread
From: Josh Law @ 2026-03-16 16:52 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: kees, akpm, andy, linux-hardening, linux-kernel



On 16 March 2026 16:51:39 GMT, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>On Mon, Mar 16, 2026 at 04:46:47PM +0000, Josh Law wrote:
>> On 16 March 2026 16:43:12 GMT, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>> >On Mon, Mar 16, 2026 at 04:36:40PM +0000, Josh Law wrote:
>
>...
>
>> >Drop the function. Start from converting users to strscpy() or similar
>> >(we have a few for different cases) and then drop strlcat() all for good.
>> 
>> Okie dokie. I'm doing a patch on that right now, keep your eyes peeled on
>> your emails, thanks for the suggestion 
>
>I'm taking my breath!
>


It's been sent!

V/R


Josh Law

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-03-16 16:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-16 16:17 [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat Josh Law
2026-03-16 16:29 ` Greg KH
2026-03-16 16:30 ` Andy Shevchenko
2026-03-16 16:36   ` Josh Law
2026-03-16 16:43     ` Andy Shevchenko
2026-03-16 16:46       ` Josh Law
2026-03-16 16:51         ` Andy Shevchenko
2026-03-16 16:52           ` Josh Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox