linux-kernel - Re: [Fwd: [PATCH 1/2] string: Add stracpy and stracpy

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f909b4b31f123c7d88535db397a04421077ed0ab.camel@perches.com>
Date:   Tue, 23 Jul 2019 21:37:51 -0700
From:   Joe Perches <joe@...ches.com>
To:     Julia Lawall <julia.lawall@...6.fr>
Cc:     cocci <cocci@...teme.lip6.fr>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [Fwd: [PATCH 1/2] string: Add stracpy and stracpy_pad
 mechanisms]

On Tue, 2019-07-23 at 23:27 -0500, Julia Lawall wrote:
> 
> On Tue, 23 Jul 2019, Joe Perches wrote:
> 
> > On Tue, 2019-07-23 at 22:54 -0500, Julia Lawall wrote:
> > > A seantic patch and the resulting output for the case where the third
> > > arugument is a constant is attached.  Likewise the resulting output on a
> > > recent linux-next.
> > > 
> > > julia
> > 
> > Nice.  Thanks Julia
> > 
> > A couple issues:
> > 
> > There is a problem with conversions with assignments
> > of strlcpy() so ideally the cocci script should make sure
> > any return value was not used before conversion.
> > 
> > This is not a provably good conversion:
> > 
> > drivers/s390/char/sclp_ftp.c
> > @@ -114,8 +114,7 @@ static int sclp_ftp_et7(const struct hmc
> >         sccb->evbuf.mdd.ftp.length = ftp->len;
> >         sccb->evbuf.mdd.ftp.bufaddr = virt_to_phys(ftp->buf);
> > 
> > -       len = strlcpy(sccb->evbuf.mdd.ftp.fident, ftp->fname,
> > -                     HMCDRV_FTP_FIDENT_MAX);
> > +       len = stracpy(sccb->evbuf.mdd.ftp.fident, ftp->fname);
> 
> Sorry, I don't understand the issue here.  What specifically should I be
> looking for?

The return value of strlcpy differs from (strscpy or stracpy).

strlcpy		returns the length of the src string
str[sa]cpy	returns the length of the src string if it fits in dest
		or -E2BIG if src is truncated by the size of dest
		or -E2BIG if dest is 0 length

Any use of the strlcpy return value should not be converted
because the logic after an assignment or use of the return value
can not be assured to have the same behavior.

> > And:
> > 
> > I would have expected the bit below to find and convert uses like
> > 	drivers/hwmon/adc128d818.c:     strlcpy(info->type, "adc128d818", I2C_NAME_SIZE);
> > but it seems none of those were converted.
> 
> OK, thanks.  I will check on it.

Thanks again.

> julia
> 
> > I don't know why.
> > 
> > //------------------------------------------
> > @r1@
> > struct i1 *e1;
> > expression e2;
> > identifier f,i1,i2;
> > position p;
> > @@
> > \(strscpy\|strlcpy\)(e1->f, e2, i2)@p
> > 
> > @@
> > identifier r1.i1,r1.i2;
> > type T;
> > @@
> > struct i1 { ... T i1[i2]; ... }
> > 
> > @@
> > identifier f,i2;
> > expression e1,e2;
> > position r1.p;
> > @@
> > (
> > -strscpy
> > +stracpy
> > -strlcpy
> > +stracpy
> > )(e1->f, e2
> > -    , i2
> >   )@p
> > //------------------------------------------
> > 
> > to find
> > 
> >