| Message ID | 1527572683-26098-1-git-send-email-nick.desaulniers@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On Mon, May 28, 2018 at 10:44:43PM -0700, Nick Desaulniers wrote: > Fixes stringop-truncation warnings from gcc-8. > > Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> > --- > crypto/ablkcipher.c | 8 ++++---- > crypto/blkcipher.c | 4 ++-- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c > index d880a48..e38867f 100644 > --- a/crypto/ablkcipher.c > +++ b/crypto/ablkcipher.c > @@ -370,8 +370,8 @@ static int crypto_ablkcipher_report(struct sk_buff *skb, struct crypto_alg *alg) > { > struct crypto_report_blkcipher rblkcipher; > > - strncpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type)); > - strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<default>", > + strlcpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type)); > + strlcpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<default>", > sizeof(rblkcipher.geniv)); > > rblkcipher.blocksize = alg->cra_blocksize; > @@ -444,8 +444,8 @@ static int crypto_givcipher_report(struct sk_buff *skb, struct crypto_alg *alg) > { > struct crypto_report_blkcipher rblkcipher; > > - strncpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type)); > - strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<built-in>", > + strlcpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type)); > + strlcpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<built-in>", > sizeof(rblkcipher.geniv)); > > rblkcipher.blocksize = alg->cra_blocksize; > diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c > index 01c0d4a..ee88e48 100644 > --- a/crypto/blkcipher.c > +++ b/crypto/blkcipher.c > @@ -509,8 +509,8 @@ static int crypto_blkcipher_report(struct sk_buff *skb, struct crypto_alg *alg) > { > struct crypto_report_blkcipher rblkcipher; > > - strncpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type)); > - strncpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "<default>", > + strlcpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type)); > + strlcpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "<default>", > sizeof(rblkcipher.geniv)); > > rblkcipher.blocksize = alg->cra_blocksize; > -- > 2.7.4 > Hi Nick, this patch is wrong. The 'struct crypto_report_blkcipher' is being copied to userspace via netlink, so all bytes of it must be initialized. strncpy() does this but strlcpy() does not. I noticed that you're sending out some other patches replacing strncpy() with strlcpy() too. Can you please double check them for this same bug? Thanks, - Eric
On Tue, May 29, 2018 at 9:34 AM, Eric Biggers <ebiggers3@gmail.com> wrote: > On Mon, May 28, 2018 at 10:44:43PM -0700, Nick Desaulniers wrote: >> Fixes stringop-truncation warnings from gcc-8. >> >> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> >> --- >> crypto/ablkcipher.c | 8 ++++---- >> crypto/blkcipher.c | 4 ++-- >> 2 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c >> index d880a48..e38867f 100644 >> --- a/crypto/ablkcipher.c >> +++ b/crypto/ablkcipher.c >> @@ -370,8 +370,8 @@ static int crypto_ablkcipher_report(struct sk_buff *skb, struct crypto_alg *alg) >> { >> struct crypto_report_blkcipher rblkcipher; >> >> - strncpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type)); >> - strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<default>", >> + strlcpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type)); >> + strlcpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<default>", >> sizeof(rblkcipher.geniv)); >> >> rblkcipher.blocksize = alg->cra_blocksize; >> @@ -444,8 +444,8 @@ static int crypto_givcipher_report(struct sk_buff *skb, struct crypto_alg *alg) >> { >> struct crypto_report_blkcipher rblkcipher; >> >> - strncpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type)); >> - strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<built-in>", >> + strlcpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type)); >> + strlcpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<built-in>", >> sizeof(rblkcipher.geniv)); >> >> rblkcipher.blocksize = alg->cra_blocksize; >> diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c >> index 01c0d4a..ee88e48 100644 >> --- a/crypto/blkcipher.c >> +++ b/crypto/blkcipher.c >> @@ -509,8 +509,8 @@ static int crypto_blkcipher_report(struct sk_buff *skb, struct crypto_alg *alg) >> { >> struct crypto_report_blkcipher rblkcipher; >> >> - strncpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type)); >> - strncpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "<default>", >> + strlcpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type)); >> + strlcpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "<default>", >> sizeof(rblkcipher.geniv)); >> >> rblkcipher.blocksize = alg->cra_blocksize; >> -- >> 2.7.4 >> > > Hi Nick, this patch is wrong. The 'struct crypto_report_blkcipher' is being > copied to userspace via netlink, so all bytes of it must be initialized. > strncpy() does this but strlcpy() does not. > > I noticed that you're sending out some other patches replacing strncpy() with > strlcpy() too. Can you please double check them for this same bug? > > Thanks, > > - Eric Thanks for pointing out this subtlety, I've retracted the other patches.
diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c index d880a48..e38867f 100644 --- a/crypto/ablkcipher.c +++ b/crypto/ablkcipher.c @@ -370,8 +370,8 @@ static int crypto_ablkcipher_report(struct sk_buff *skb, struct crypto_alg *alg) { struct crypto_report_blkcipher rblkcipher; - strncpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type)); - strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<default>", + strlcpy(rblkcipher.type, "ablkcipher", sizeof(rblkcipher.type)); + strlcpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<default>", sizeof(rblkcipher.geniv)); rblkcipher.blocksize = alg->cra_blocksize; @@ -444,8 +444,8 @@ static int crypto_givcipher_report(struct sk_buff *skb, struct crypto_alg *alg) { struct crypto_report_blkcipher rblkcipher; - strncpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type)); - strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<built-in>", + strlcpy(rblkcipher.type, "givcipher", sizeof(rblkcipher.type)); + strlcpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<built-in>", sizeof(rblkcipher.geniv)); rblkcipher.blocksize = alg->cra_blocksize; diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c index 01c0d4a..ee88e48 100644 --- a/crypto/blkcipher.c +++ b/crypto/blkcipher.c @@ -509,8 +509,8 @@ static int crypto_blkcipher_report(struct sk_buff *skb, struct crypto_alg *alg) { struct crypto_report_blkcipher rblkcipher; - strncpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type)); - strncpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "<default>", + strlcpy(rblkcipher.type, "blkcipher", sizeof(rblkcipher.type)); + strlcpy(rblkcipher.geniv, alg->cra_blkcipher.geniv ?: "<default>", sizeof(rblkcipher.geniv)); rblkcipher.blocksize = alg->cra_blocksize;
Fixes stringop-truncation warnings from gcc-8. Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> --- crypto/ablkcipher.c | 8 ++++---- crypto/blkcipher.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-)