RE: [Fwd: [PATCH 1/2] string: Add stracpy and stracpy_pad mechanisms]

[David Laight <David.Laight@ACULAB.COM>]

From: Joe Perches
> Sent: 24 July 2019 05:38
> 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.

Most of the code probably expects the length to be that copied
(so is broken if the string is truncated).

OTOH the code almost certainly doesn't expect -E2BIG either
so will go wrong in that case as well.

The return value from str[as]cpy() is its own set of bugs
just waiting to happen.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)