| Message ID | 1495033238-26016-9-git-send-email-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On Wed, May 17, 2017 at 06:00:35PM +0300, Tudor Ambarus wrote: > crypto_akcipher_maxsize() returns minimum length for output buffer > or error code if key hasn't been set. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > crypto/testmgr.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/crypto/testmgr.c b/crypto/testmgr.c > index 6f5f3ed..87a4abd 100644 > --- a/crypto/testmgr.c > +++ b/crypto/testmgr.c > @@ -2150,6 +2150,11 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, > > err = -ENOMEM; > out_len_max = crypto_akcipher_maxsize(tfm); Can this call be reached without a setkey or with a failed setkey? If not we should not check for errors. Cheers,
On 23.05.2017 07:08, Herbert Xu wrote: > On Wed, May 17, 2017 at 06:00:35PM +0300, Tudor Ambarus wrote: >> crypto_akcipher_maxsize() returns minimum length for output buffer >> or error code if key hasn't been set. >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >> --- >> crypto/testmgr.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/crypto/testmgr.c b/crypto/testmgr.c >> index 6f5f3ed..87a4abd 100644 >> --- a/crypto/testmgr.c >> +++ b/crypto/testmgr.c >> @@ -2150,6 +2150,11 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, >> >> err = -ENOMEM; >> out_len_max = crypto_akcipher_maxsize(tfm); > > Can this call be reached without a setkey or with a failed setkey? As of now, this call is reached only after a successful setkey. If some user call it before setkey we will end up in a NULL dereference. I tend to keep the error checking. Thanks, ta
On Tue, May 23, 2017 at 12:18:23PM +0300, Tudor Ambarus wrote: > > As of now, this call is reached only after a successful setkey. > If some user call it before setkey we will end up in a NULL dereference. > I tend to keep the error checking. I fail to see how this could be an issue in testmgr. Please just kill these unnecessary checks. Thanks,
diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 6f5f3ed..87a4abd 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -2150,6 +2150,11 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, err = -ENOMEM; out_len_max = crypto_akcipher_maxsize(tfm); + if (out_len_max < 0) { + err = out_len_max; + goto free_req; + } + outbuf_enc = kzalloc(out_len_max, GFP_KERNEL); if (!outbuf_enc) goto free_req;
crypto_akcipher_maxsize() returns minimum length for output buffer or error code if key hasn't been set. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- crypto/testmgr.c | 5 +++++ 1 file changed, 5 insertions(+)