Re: [PATCH] crypto: Replace strlcpy with strscpy

[Azeem Shaikh]

On Mon, Jun 12, 2023 at 10:37 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
>
> On Tue, Jun 13, 2023 at 12:22:58AM +0000, Azeem Shaikh wrote:
> > diff --git a/crypto/lrw.c b/crypto/lrw.c
> > index 1b0f76ba3eb5..bb8c1575645b 100644
> > --- a/crypto/lrw.c
> > +++ b/crypto/lrw.c
> > @@ -359,8 +359,8 @@ static int lrw_create(struct crypto_template *tmpl, struct rtattr **tb)
> >       if (!strncmp(cipher_name, "ecb(", 4)) {
> >               unsigned len;
> >
> > -             len = strlcpy(ecb_name, cipher_name + 4, sizeof(ecb_name));
> > -             if (len < 2 || len >= sizeof(ecb_name))
> > +             len = strscpy(ecb_name, cipher_name + 4, sizeof(ecb_name));
> > +             if (len < 2)
> >                       goto err_free_inst;
>
> This is wrong because 'len' is unsigned.  So the -EFBIG return value is not
> being checked for, so this patch actually removes the overflow check.
>

Thanks for catching this Eric. I can send over a v2 with "unsigned
len" -> "int len" if that works?

> It looks like you've sent over 60 strscpy conversion patches recently
> (https://lore.kernel.org/linux-hardening/?q=f%3Aazeemshaikh38+s%3Astrscpy).
> That's concerning because this is the only one that I happened to review, and it
> introduces a bug.  There are 60 others that could have this same bug.  Can you
> please review all your patches for this same bug?
>

My previous patches replace strlcpy where the return value is ignored.
So this same bug could not have occurred in those. This is the first
set of patches where I'm attempting to replace strlcpy whose return
value is used by callers. For reference below are all my patches where
the return value of strlcpy is used. I have checked that none of these
have the same bug as here.

https://lore.kernel.org/all/20230613004402.3540432-1-azeemshaikh38@xxxxxxxxx/
   // Uses int
https://lore.kernel.org/all/20230613004341.3540325-1-azeemshaikh38@xxxxxxxxx/
   // Uses int
https://lore.kernel.org/all/20230613004125.3539934-1-azeemshaikh38@xxxxxxxxx/
   // Replaces ">= buflen" with "== -E2BIG"
https://lore.kernel.org/all/20230613004054.3539554-1-azeemshaikh38@xxxxxxxxx/
   // Uses int