| Message ID | 20210222151231.22572-3-romain.perier@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Manual replacement of all strlcpy in favor of strscpy | expand |
On Mon, Feb 22, 2021 at 04:12:13PM +0100, Romain Perier wrote: > > diff --git a/crypto/lrw.c b/crypto/lrw.c > index bcf09fbc750a..4d35f4439012 100644 > --- a/crypto/lrw.c > +++ b/crypto/lrw.c > @@ -357,10 +357,10 @@ static int lrw_create(struct crypto_template *tmpl, struct rtattr **tb) > * cipher name. > */ > if (!strncmp(cipher_name, "ecb(", 4)) { > - unsigned len; > + ssize_t 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 == -E2BIG || len < 2) len == -E2BIG is superfluous as len < 2 will catch it anyway. Thanks,
Le jeu. 4 mars 2021 à 05:37, Herbert Xu <herbert@gondor.apana.org.au> a écrit : > On Mon, Feb 22, 2021 at 04:12:13PM +0100, Romain Perier wrote: > > > > diff --git a/crypto/lrw.c b/crypto/lrw.c > > index bcf09fbc750a..4d35f4439012 100644 > > --- a/crypto/lrw.c > > +++ b/crypto/lrw.c > > @@ -357,10 +357,10 @@ static int lrw_create(struct crypto_template > *tmpl, struct rtattr **tb) > > * cipher name. > > */ > > if (!strncmp(cipher_name, "ecb(", 4)) { > > - unsigned len; > > + ssize_t 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 == -E2BIG || len < 2) > > len == -E2BIG is superfluous as len < 2 will catch it anyway. > > Thanks, > > Hello, Yeah that's fixed in v2. Thanks, Romain
diff --git a/crypto/lrw.c b/crypto/lrw.c index bcf09fbc750a..4d35f4439012 100644 --- a/crypto/lrw.c +++ b/crypto/lrw.c @@ -357,10 +357,10 @@ static int lrw_create(struct crypto_template *tmpl, struct rtattr **tb) * cipher name. */ if (!strncmp(cipher_name, "ecb(", 4)) { - unsigned len; + ssize_t 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 == -E2BIG || len < 2) goto err_free_inst; if (ecb_name[len - 1] != ')') diff --git a/crypto/xts.c b/crypto/xts.c index 6c12f30dbdd6..1dfe39d61418 100644 --- a/crypto/xts.c +++ b/crypto/xts.c @@ -396,10 +396,10 @@ static int xts_create(struct crypto_template *tmpl, struct rtattr **tb) * cipher name. */ if (!strncmp(cipher_name, "ecb(", 4)) { - unsigned len; + ssize_t len; - len = strlcpy(ctx->name, cipher_name + 4, sizeof(ctx->name)); - if (len < 2 || len >= sizeof(ctx->name)) + len = strscpy(ctx->name, cipher_name + 4, sizeof(ctx->name)); + if (len == -E2BIG || len < 2) goto err_free_inst; if (ctx->name[len - 1] != ')')
The strlcpy() reads the entire source buffer first, it is dangerous if the source buffer lenght is unbounded or possibility non NULL-terminated. It can lead to linear read overflows, crashes, etc... As recommended in the deprecated interfaces [1], it should be replaced by strscpy. This commit replaces all calls to strlcpy that handle the return values by the corresponding strscpy calls with new handling of the return values (as it is quite different between the two functions). [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy Signed-off-by: Romain Perier <romain.perier@gmail.com> --- crypto/lrw.c | 6 +++--- crypto/xts.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)