[PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat

Josh Law posted 1 patch 3 weeks ago
lib/string.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
Posted by Josh Law 3 weeks ago
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
Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
Posted by Greg KH 3 weeks ago
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
Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
Posted by Andy Shevchenko 3 weeks ago
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
Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
Posted by Josh Law 3 weeks ago

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
Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
Posted by Andy Shevchenko 3 weeks ago
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
Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
Posted by Josh Law 3 weeks ago

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
Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
Posted by Andy Shevchenko 3 weeks ago
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
Re: [PATCH] lib/string: replace BUG_ON with WARN_ON_ONCE in strlcat
Posted by Josh Law 3 weeks ago

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