linux-kernel - Re: [PATCH 4/4] tools/nolibc: add missing memchr() to string.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250621084234.GA26934@1wt.eu>
Date: Sat, 21 Jun 2025 10:42:34 +0200
From: Willy Tarreau <w@....eu>
To: Thomas Weißschuh <thomas@...ch.de>
Cc: Thomas Weißschuh <thomas.weissschuh@...utronix.de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] tools/nolibc: add missing memchr() to string.h

On Sat, Jun 21, 2025 at 10:27:11AM +0200, Thomas Weißschuh wrote:
> On 2025-06-20 12:02:51+0200, Willy Tarreau wrote:
> > Surprisingly we forgot to add this common one. It was added with a
> > per-arch guard allowing to later implement it in arch-specific asm
> > code like was done for a few other ones.
> > 
> > The test verifies that we don't search past the indicated length.
> > 
> > Signed-off-by: Willy Tarreau <w@....eu>
> > ---
> >  tools/include/nolibc/string.h                | 15 +++++++++++++++
> >  tools/testing/selftests/nolibc/nolibc-test.c |  2 ++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
> > index 163a17e7dd38b..4000926f44ac4 100644
> > --- a/tools/include/nolibc/string.h
> > +++ b/tools/include/nolibc/string.h
> > @@ -93,6 +93,21 @@ void *memset(void *dst, int b, size_t len)
> >  }
> >  #endif /* #ifndef NOLIBC_ARCH_HAS_MEMSET */
> >  
> > +#ifndef NOLIBC_ARCH_HAS_MEMCHR
> 
> So far we only have added these guards when necessary,
> which they aren't here. Can we drop them?

I intentionally placed them so that we can easily override them,
as we did for the other ones on x86 where string operations are
super short (repnz scasb is two bytes once you have the registers
already loaded).

> > +static __attribute__((unused))
> > +void *memchr(const void *s, int c, size_t len)
> > +{
> > +	char *p = (char *)s;
> 
> The docs say that they are interpreted as "unsigned char".

It does not change anything here, except adding an extra
modifier (since we'll then also have to do it in the loop
when comparing against c), thus IMHO it's extra noise.

> Also, can we keep the const?

It's memchr()'s definition which requires to return a void* so the
const needs to be dropped somewhere. Here I found visually cleaner to
have a single cast during the variable assignment rather than have a
second one on the return statement. But it's a matter of taste. I
tend to hate casts as they confuse the reader and remove the ability
of the compiler to produce relevant warnings, so for me the less the
better.

Thanks,
Willy