diff mbox series

[02/20] crypto: Manual replacement of the deprecated strlcpy() with return values

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

Commit Message

Romain Perier Feb. 22, 2021, 3:12 p.m. UTC
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(-)

Comments

Herbert Xu March 4, 2021, 4:37 a.m. UTC | #1
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,
Romain Perier March 4, 2021, 10:08 a.m. UTC | #2
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 mbox series

Patch

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] != ')')