2009-09-18 22:56:42

by Roel Kluin

[permalink] [raw]
Subject: [PATCH] random: kmalloc failure ignored in init_std_data()

Clean up and error out if kmalloc() fails.

Signed-off-by: Roel Kluin <[email protected]>
---
Found with sed: http://kernelnewbies.org/roelkluin

Build tested. Please review

diff --git a/drivers/char/random.c b/drivers/char/random.c
index d8a9255..8a68be8 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -939,7 +939,7 @@ EXPORT_SYMBOL(get_random_bytes);
* data into the pool to prepare it for use. The pool is not cleared
* as that can only decrease the entropy in the pool.
*/
-static void init_std_data(struct entropy_store *r)
+static int init_std_data(struct entropy_store *r)
{
ktime_t now;
unsigned long flags;
@@ -952,16 +952,35 @@ static void init_std_data(struct entropy_store *r)
mix_pool_bytes(r, &now, sizeof(now));
mix_pool_bytes(r, utsname(), sizeof(*(utsname())));
/* Enable continuous test in fips mode */
- if (fips_enabled)
+ if (fips_enabled) {
r->last_data = kmalloc(EXTRACT_SIZE, GFP_KERNEL);
+ if (r->last_data == NULL)
+ return -ENOMEM;
+ }
+ return 0;
}

static int rand_initialize(void)
{
- init_std_data(&input_pool);
- init_std_data(&blocking_pool);
- init_std_data(&nonblocking_pool);
+ int ret;
+ ret = init_std_data(&input_pool);
+ if (ret != 0)
+ return ret;
+
+ ret = init_std_data(&blocking_pool);
+ if (ret != 0)
+ goto free_ip_ld;
+
+ ret = init_std_data(&nonblocking_pool);
+ if (ret != 0)
+ goto free_bp_ld;
+
return 0;
+free_bp_ld:
+ kfree(blocking_pool.last_data);
+free_ip_ld:
+ kfree(input_pool.last_data);
+ return ret;
}
module_init(rand_initialize);

@@ -1160,8 +1179,8 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
/* Clear the entropy pool counters. */
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- rand_initialize();
- return 0;
+ retval = rand_initialize();
+ return retval;
default:
return -EINVAL;
}


2009-09-19 00:11:09

by Matt Mackall

[permalink] [raw]
Subject: Re: [PATCH] random: kmalloc failure ignored in init_std_data()

On Sat, 2009-09-19 at 01:03 +0200, Roel Kluin wrote:
> Clean up and error out if kmalloc() fails.

No thanks. Let's instead make it so it can't fail by building the array
into the statically allocated pool structures.

> Signed-off-by: Roel Kluin <[email protected]>
> ---
> Found with sed: http://kernelnewbies.org/roelkluin
>
> Build tested. Please review
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index d8a9255..8a68be8 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -939,7 +939,7 @@ EXPORT_SYMBOL(get_random_bytes);
> * data into the pool to prepare it for use. The pool is not cleared
> * as that can only decrease the entropy in the pool.
> */
> -static void init_std_data(struct entropy_store *r)
> +static int init_std_data(struct entropy_store *r)
> {
> ktime_t now;
> unsigned long flags;
> @@ -952,16 +952,35 @@ static void init_std_data(struct entropy_store *r)
> mix_pool_bytes(r, &now, sizeof(now));
> mix_pool_bytes(r, utsname(), sizeof(*(utsname())));
> /* Enable continuous test in fips mode */
> - if (fips_enabled)
> + if (fips_enabled) {
> r->last_data = kmalloc(EXTRACT_SIZE, GFP_KERNEL);
> + if (r->last_data == NULL)
> + return -ENOMEM;
> + }
> + return 0;
> }
>
> static int rand_initialize(void)
> {
> - init_std_data(&input_pool);
> - init_std_data(&blocking_pool);
> - init_std_data(&nonblocking_pool);
> + int ret;
> + ret = init_std_data(&input_pool);
> + if (ret != 0)
> + return ret;
> +
> + ret = init_std_data(&blocking_pool);
> + if (ret != 0)
> + goto free_ip_ld;
> +
> + ret = init_std_data(&nonblocking_pool);
> + if (ret != 0)
> + goto free_bp_ld;
> +
> return 0;
> +free_bp_ld:
> + kfree(blocking_pool.last_data);
> +free_ip_ld:
> + kfree(input_pool.last_data);
> + return ret;
> }
> module_init(rand_initialize);
>
> @@ -1160,8 +1179,8 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> /* Clear the entropy pool counters. */
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> - rand_initialize();
> - return 0;
> + retval = rand_initialize();
> + return retval;
> default:
> return -EINVAL;
> }

--
http://selenic.com : development and support for Mercurial and Linux

2009-09-19 00:46:58

by Roel Kluin

[permalink] [raw]
Subject: Re: [PATCH] random: kmalloc failure ignored in init_std_data()

kmalloc may fail so build the array into the statically allocated
pool structures instead.

Signed-off-by: Roel Kluin <[email protected]>
---
Is this what you had in mind? (it builds)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index d8a9255..9012540 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -400,6 +400,8 @@ module_param(debug, bool, 0644);
*
**********************************************************************/

+#define EXTRACT_SIZE 10
+
struct entropy_store;
struct entropy_store {
/* read-only data: */
@@ -414,7 +416,9 @@ struct entropy_store {
unsigned add_ptr;
int entropy_count;
int input_rotate;
- __u8 *last_data;
+#ifdef CONFIG_CRYPTO_FIPS
+ __u8 last_data[EXTRACT_SIZE];
+#endif
};

static __u32 input_pool_data[INPUT_POOL_WORDS];
@@ -714,8 +718,6 @@ void add_disk_randomness(struct gendisk *disk)
}
#endif

-#define EXTRACT_SIZE 10
-
/*********************************************************************
*
* Entropy extraction routines
@@ -951,9 +953,6 @@ static void init_std_data(struct entropy_store *r)
now = ktime_get_real();
mix_pool_bytes(r, &now, sizeof(now));
mix_pool_bytes(r, utsname(), sizeof(*(utsname())));
- /* Enable continuous test in fips mode */
- if (fips_enabled)
- r->last_data = kmalloc(EXTRACT_SIZE, GFP_KERNEL);
}

static int rand_initialize(void)

2009-09-19 07:19:49

by Matt Mackall

[permalink] [raw]
Subject: Re: [PATCH] random: kmalloc failure ignored in init_std_data()

On Sat, 2009-09-19 at 02:54 +0200, Roel Kluin wrote:
> kmalloc may fail so build the array into the statically allocated
> pool structures instead.
>
> Signed-off-by: Roel Kluin <[email protected]>
> ---
> Is this what you had in mind? (it builds)

Yes, looks good.

Acked-by: Matt Mackall <[email protected]>

> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index d8a9255..9012540 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -400,6 +400,8 @@ module_param(debug, bool, 0644);
> *
> **********************************************************************/
>
> +#define EXTRACT_SIZE 10
> +
> struct entropy_store;
> struct entropy_store {
> /* read-only data: */
> @@ -414,7 +416,9 @@ struct entropy_store {
> unsigned add_ptr;
> int entropy_count;
> int input_rotate;
> - __u8 *last_data;
> +#ifdef CONFIG_CRYPTO_FIPS
> + __u8 last_data[EXTRACT_SIZE];
> +#endif
> };
>
> static __u32 input_pool_data[INPUT_POOL_WORDS];
> @@ -714,8 +718,6 @@ void add_disk_randomness(struct gendisk *disk)
> }
> #endif
>
> -#define EXTRACT_SIZE 10
> -
> /*********************************************************************
> *
> * Entropy extraction routines
> @@ -951,9 +953,6 @@ static void init_std_data(struct entropy_store *r)
> now = ktime_get_real();
> mix_pool_bytes(r, &now, sizeof(now));
> mix_pool_bytes(r, utsname(), sizeof(*(utsname())));
> - /* Enable continuous test in fips mode */
> - if (fips_enabled)
> - r->last_data = kmalloc(EXTRACT_SIZE, GFP_KERNEL);
> }
>
> static int rand_initialize(void)

--
http://selenic.com : development and support for Mercurial and Linux

2009-10-09 07:36:03

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] random: kmalloc failure ignored in init_std_data()

On Sat, 19 Sep 2009 02:54:07 +0200 Roel Kluin <[email protected]> wrote:

> kmalloc may fail so build the array into the statically allocated
> pool structures instead.

x86_64 allnoconfig:

drivers/char/random.c: In function 'extract_entropy':
drivers/char/random.c:867: error: 'struct entropy_store' has no member named 'last_data'
drivers/char/random.c:869: error: 'struct entropy_store' has no member named 'last_data'
drivers/char/random.c:871: error: 'struct entropy_store' has no member named 'last_data'
drivers/char/random.c:871: error: 'struct entropy_store' has no member named 'last_data'