Fix NULL pointer deref. on no default_rng
diff mbox

Message ID 20171112142432.20186-1-pinaraf@pinaraf.info
State Accepted
Delegated to: Herbert Xu
Headers show

Commit Message

Pierre Nov. 12, 2017, 2:24 p.m. UTC
If crypto_get_default_rng returns an error, the
function ecc_gen_privkey should return an error.
Instead, it currently tries to use the default_rng
nevertheless, thus creating a kernel panic with a
NULL pointer dereference.
Returning the error directly, as was supposedly
intended when looking at the code, fixes this.

Signed-off-by: Pierre Ducroquet <pinaraf@pinaraf.info>
---
 crypto/ecc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

PrasannaKumar Muralidharan Nov. 12, 2017, 2:46 p.m. UTC | #1
Hi Pierre,

On 12 November 2017 at 19:54, Pierre Ducroquet <pinaraf@pinaraf.info> wrote:
> If crypto_get_default_rng returns an error, the
> function ecc_gen_privkey should return an error.
> Instead, it currently tries to use the default_rng
> nevertheless, thus creating a kernel panic with a
> NULL pointer dereference.
> Returning the error directly, as was supposedly
> intended when looking at the code, fixes this.
>
> Signed-off-by: Pierre Ducroquet <pinaraf@pinaraf.info>
> ---
>  crypto/ecc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/crypto/ecc.c b/crypto/ecc.c
> index 633a9bcdc574..18f32f2a5e1c 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -964,7 +964,7 @@ int ecc_gen_privkey(unsigned int curve_id, unsigned int ndigits, u64 *privkey)
>          * DRBG with a security strength of 256.
>          */
>         if (crypto_get_default_rng())
> -               err = -EFAULT;
> +               return -EFAULT;
>
>         err = crypto_rng_get_bytes(crypto_default_rng, (u8 *)priv, nbytes);
>         crypto_put_default_rng();
> --
> 2.15.0
>

Looks good to me.

Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>

Regards,
PrasannaKumar
Pierre Nov. 18, 2017, 5:18 p.m. UTC | #2
On Sunday, November 12, 2017 8:16:27 PM CET PrasannaKumar Muralidharan wrote:
> Hi Pierre,
> 
> On 12 November 2017 at 19:54, Pierre Ducroquet <pinaraf@pinaraf.info> wrote:
> > If crypto_get_default_rng returns an error, the
> > function ecc_gen_privkey should return an error.
> > Instead, it currently tries to use the default_rng
> > nevertheless, thus creating a kernel panic with a
> > NULL pointer dereference.
> > Returning the error directly, as was supposedly
> > intended when looking at the code, fixes this.
> > 
> > Signed-off-by: Pierre Ducroquet <pinaraf@pinaraf.info>
> > ---
> > 
> >  crypto/ecc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/crypto/ecc.c b/crypto/ecc.c
> > index 633a9bcdc574..18f32f2a5e1c 100644
> > --- a/crypto/ecc.c
> > +++ b/crypto/ecc.c
> > @@ -964,7 +964,7 @@ int ecc_gen_privkey(unsigned int curve_id, unsigned
> > int ndigits, u64 *privkey)> 
> >          * DRBG with a security strength of 256.
> >          */
> >         
> >         if (crypto_get_default_rng())
> > 
> > -               err = -EFAULT;
> > +               return -EFAULT;
> > 
> >         err = crypto_rng_get_bytes(crypto_default_rng, (u8 *)priv,
> >         nbytes);
> >         crypto_put_default_rng();
> > 
> > --
> > 2.15.0
> 
> Looks good to me.
> 
> Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> 
> Regards,
> PrasannaKumar

Hi

Thanks for your review.
Is there anything I should do to have this merged ? It fixes a kernel panic at 
boot on my computer, so I suppose other people may have that too.

Regards
 Pierre
Herbert Xu Nov. 29, 2017, 6:34 a.m. UTC | #3
On Sun, Nov 12, 2017 at 03:24:32PM +0100, Pierre Ducroquet wrote:
> If crypto_get_default_rng returns an error, the
> function ecc_gen_privkey should return an error.
> Instead, it currently tries to use the default_rng
> nevertheless, thus creating a kernel panic with a
> NULL pointer dereference.
> Returning the error directly, as was supposedly
> intended when looking at the code, fixes this.
> 
> Signed-off-by: Pierre Ducroquet <pinaraf@pinaraf.info>

Patch applied.  Thanks.
Pierre Dec. 7, 2017, 10:32 a.m. UTC | #4
On Wednesday, November 29, 2017 5:34:30 PM CET you wrote:
> On Sun, Nov 12, 2017 at 03:24:32PM +0100, Pierre Ducroquet wrote:
> > If crypto_get_default_rng returns an error, the
> > function ecc_gen_privkey should return an error.
> > Instead, it currently tries to use the default_rng
> > nevertheless, thus creating a kernel panic with a
> > NULL pointer dereference.
> > Returning the error directly, as was supposedly
> > intended when looking at the code, fixes this.
> > 
> > Signed-off-by: Pierre Ducroquet <pinaraf@pinaraf.info>
> 
> Patch applied.  Thanks.

Thanks to you.

Will it be backported to 4.14 and the LTS releases ? Currently, no recent 
debian kernel can boot on my system without that patch (I don't know why 
nobody else noticed that before).

 Pierre
Pierre Dec. 20, 2017, 7:49 p.m. UTC | #5
On Wednesday, November 29, 2017 5:34:30 PM CET Herbert Xu wrote:
> On Sun, Nov 12, 2017 at 03:24:32PM +0100, Pierre Ducroquet wrote:
> > If crypto_get_default_rng returns an error, the
> > function ecc_gen_privkey should return an error.
> > Instead, it currently tries to use the default_rng
> > nevertheless, thus creating a kernel panic with a
> > NULL pointer dereference.
> > Returning the error directly, as was supposedly
> > intended when looking at the code, fixes this.
> > 
> > Signed-off-by: Pierre Ducroquet <pinaraf@pinaraf.info>
> 
> Patch applied.  Thanks.

Hi

How long will it take for the patch to be merged in an official release ?
This simple patch fixes a kernel panic at boot, it would be great for it to be 
merged in 4.15.

Thanks
 Pierre

Patch
diff mbox

diff --git a/crypto/ecc.c b/crypto/ecc.c
index 633a9bcdc574..18f32f2a5e1c 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -964,7 +964,7 @@  int ecc_gen_privkey(unsigned int curve_id, unsigned int ndigits, u64 *privkey)
 	 * DRBG with a security strength of 256.
 	 */
 	if (crypto_get_default_rng())
-		err = -EFAULT;
+		return -EFAULT;
 
 	err = crypto_rng_get_bytes(crypto_default_rng, (u8 *)priv, nbytes);
 	crypto_put_default_rng();