linux-kernel - RE: [PATCH] s390: Fix strrchr() implementation

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <7eeccf08aed44453959763629852d58e@huawei.com>
Date:   Tue, 5 Oct 2021 11:30:02 +0000
From:   Roberto Sassu <roberto.sassu@...wei.com>
To:     Heiko Carstens <hca@...ux.ibm.com>
CC:     "gor@...ux.ibm.com" <gor@...ux.ibm.com>,
        "borntraeger@...ibm.com" <borntraeger@...ibm.com>,
        "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] s390: Fix strrchr() implementation

> From: Heiko Carstens [mailto:hca@...ux.ibm.com]
> Sent: Tuesday, October 5, 2021 1:13 PM
> On Tue, Oct 05, 2021 at 09:26:21AM +0200, Roberto Sassu wrote:
> > Access the string at len - 1 instead of len.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
> > ---
> >  arch/s390/lib/string.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/s390/lib/string.c b/arch/s390/lib/string.c
> > index cfcdf76d6a95..162a391788ad 100644
> > --- a/arch/s390/lib/string.c
> > +++ b/arch/s390/lib/string.c
> > @@ -261,12 +261,12 @@ char *strrchr(const char *s, int c)
> >  {
> >         size_t len = __strend(s) - s;
> >
> > -       if (len)
> > -	       do {
> > -		       if (s[len] == (char) c)
> > -			       return (char *) s + len;
> > -	       } while (--len > 0);
> > -       return NULL;
> > +	if (len)
> > +		do {
> > +			if (s[len - 1] == (char) c)
> > +				return (char *) s + len - 1;
> > +		} while (--len > 0);
> > +	return NULL;
> 
> You missed to tell what this is supposed to fix. The patch however is
> incorrect: the terminating null byte is considered part of the
> string. With your patch strrchr(somestring, 0) would not work
> correctly anymore.

Hi Heiko

yes, sorry. I didn't consider that.

> However our strrchr implementation is indeed broken, since for an
> empty string and searching for the null byte would incorrectly return
> NULL. Luckily there is not a single invocation in the kernel which
> doing that.

Ok. However, the main reason of this patch is that s[0] is not
evaluated, when len > 0. I will provide a new version of the patch.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua