From: "Adam J. Richter" <adam@yggdrasil.com>
To: viro@math.psu.edu, tytso@mit.edu
Cc: linux-kernel@vger.kernel.org, axboe@suse.de
Subject: Patch(2.5.45): Eliminate unchecked kfree(disk->random)
Date: Mon, 4 Nov 2002 13:29:54 -0800 [thread overview]
Message-ID: <20021104132954.A337@baldur.yggdrasil.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]
alloc_disk in linux-2.5.45/drivers/block/genhd.c calls
rand_initialize_disk, which can set disk->random to NULL on kmalloc
failure. disk_release does a kfree(disk->random) without checking.
I could add an "if(disk->random == NULL)" to disk_release,
memory allocation policy for disk->random does not need to be split
between genhd.c and and random.c, and disk->random is so small that
there is really no benefit to kmalloc'ing it.
More importantly, I want to reduce memory allocation error
branches (or, worse, lack of branches where they are needed as this
case as this case illustrates), especially because I'd eventually like
to have a variant of alloc_disk that doesn't do the kmalloc, so I can
embed the struct gendisk and have a gendisk initializer that doesn't
need an error branch (init_disk(disk, partitions_array,...), but
that's another matter).
So, here is a patch to embed disk->random. It is a net
deletion of ten lines. It also moves the declarations for
add_disk_randomness and rand_initialize_disk to <linux/random.h> where
the other add_foo_randomness and rand_initialize_foo routines are
declared.
--
Adam J. Richter __ ______________ 575 Oroville Road
adam@yggdrasil.com \ / Milpitas, California 95035
+1 408 309-6081 | g g d r a s i l United States of America
"Free Software For The Rest Of Us."
[-- Attachment #2: random.diff --]
[-- Type: text/plain, Size: 4939 bytes --]
--- linux-2.5.45/include/linux/random.h 2002-10-30 16:43:45.000000000 -0800
+++ linux/include/linux/random.h 2002-11-04 11:28:09.000000000 -0800
@@ -38,18 +38,36 @@
__u32 buf[0];
};
+/* There is one of these per entropy source. The contents of this
+ structure is private to random.c. It is only declared in random.h
+ so that other data structures can embed it. */
+struct timer_rand_state {
+ __u32 last_time;
+ __s32 last_delta,last_delta2;
+ int dont_count_entropy:1;
+};
+
/* Exported functions */
#ifdef __KERNEL__
+struct gendisk;
+
extern void rand_initialize(void);
extern void rand_initialize_irq(int irq);
+static inline void rand_initialize_disk(struct gendisk *disk)
+{
+ /* gendisk initializes &disk->random to all zeros and that
+ is all that random.c currently wants. */
+}
+
extern void batch_entropy_store(u32 a, u32 b, int num);
extern void add_keyboard_randomness(unsigned char scancode);
extern void add_mouse_randomness(__u32 mouse_data);
extern void add_interrupt_randomness(int irq);
+extern void add_disk_randomness(struct gendisk *disk);
extern void get_random_bytes(void *buf, int nbytes);
void generate_random_uuid(unsigned char uuid_out[16]);
--- linux-2.5.45/include/linux/blk.h 2002-10-30 16:43:39.000000000 -0800
+++ linux/include/linux/blk.h 2002-11-04 11:27:21.000000000 -0800
@@ -9,8 +9,6 @@
extern void set_device_ro(struct block_device *bdev, int flag);
extern void set_disk_ro(struct gendisk *disk, int flag);
-extern void add_disk_randomness(struct gendisk *disk);
-extern void rand_initialize_disk(struct gendisk *disk);
#ifdef CONFIG_BLK_DEV_RAM
--- linux-2.5.45/include/linux/genhd.h 2002-10-30 16:41:57.000000000 -0800
+++ linux/include/linux/genhd.h 2002-11-04 10:50:38.000000000 -0800
@@ -13,6 +13,7 @@
#include <linux/types.h>
#include <linux/major.h>
#include <linux/device.h>
+#include <linux/random.h>
enum {
/* These three have identical behaviour; use the second one if DOS fdisk gets
@@ -95,7 +96,7 @@
struct device *driverfs_dev;
struct device disk_dev;
- struct timer_rand_state *random;
+ struct timer_rand_state random;
int policy;
unsigned sync_io; /* RAID */
--- linux-2.5.45/drivers/block/genhd.c 2002-10-30 16:42:55.000000000 -0800
+++ linux/drivers/block/genhd.c 2002-11-04 11:29:27.000000000 -0800
@@ -27,7 +27,7 @@
#include <linux/kmod.h>
-static rwlock_t gendisk_lock;
+static rwlock_t gendisk_lock = RW_LOCK_UNLOCKED;
/*
* Global kernel list of partitioning information.
@@ -249,9 +249,6 @@
};
#endif
-
-extern int blk_dev_init(void);
-
struct device_class disk_devclass = {
.name = "disk",
};
@@ -272,14 +269,13 @@
{
struct blk_probe *base = kmalloc(sizeof(struct blk_probe), GFP_KERNEL);
int i;
- rwlock_init(&gendisk_lock);
+
memset(base, 0, sizeof(struct blk_probe));
base->dev = MKDEV(1,0);
base->range = MKDEV(MAX_BLKDEV-1, 255) - base->dev + 1;
base->get = base_probe;
for (i = 1; i < MAX_BLKDEV; i++)
probes[i] = base;
- blk_dev_init();
devclass_register(&disk_devclass);
bus_register(&disk_bus);
return 0;
@@ -292,7 +288,6 @@
static void disk_release(struct device *dev)
{
struct gendisk *disk = dev->driver_data;
- kfree(disk->random);
kfree(disk->part);
kfree(disk);
}
@@ -319,8 +314,8 @@
disk->disk_dev.release = disk_release;
disk->disk_dev.driver_data = disk;
device_initialize(&disk->disk_dev);
+ rand_initialize_disk(disk);
}
- rand_initialize_disk(disk);
return disk;
}
--- linux-2.5.45/drivers/char/random.c 2002-10-30 16:41:52.000000000 -0800
+++ linux/drivers/char/random.c 2002-11-04 11:21:37.000000000 -0800
@@ -708,13 +708,6 @@
*
*********************************************************************/
-/* There is one of these per entropy source */
-struct timer_rand_state {
- __u32 last_time;
- __s32 last_delta,last_delta2;
- int dont_count_entropy:1;
-};
-
static struct timer_rand_state keyboard_timer_state;
static struct timer_rand_state mouse_timer_state;
static struct timer_rand_state extract_timer_state;
@@ -814,10 +807,10 @@
void add_disk_randomness(struct gendisk *disk)
{
- if (!disk || !disk->random)
+ if (!disk)
return;
/* first major is 1, so we get >= 0x200 here */
- add_timer_randomness(disk->random, 0x100+MKDEV(disk->major, disk->first_minor));
+ add_timer_randomness(&disk->random, 0x100+MKDEV(disk->major, disk->first_minor));
}
/******************************************************************
@@ -1465,21 +1458,6 @@
}
}
-void rand_initialize_disk(struct gendisk *disk)
-{
- struct timer_rand_state *state;
-
- /*
- * If kmalloc returns null, we just won't use that entropy
- * source.
- */
- state = kmalloc(sizeof(struct timer_rand_state), GFP_KERNEL);
- if (state) {
- memset(state, 0, sizeof(struct timer_rand_state));
- disk->random = state;
- }
-}
-
static ssize_t
random_read(struct file * file, char * buf, size_t nbytes, loff_t *ppos)
{
next reply other threads:[~2002-11-04 21:23 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2002-11-04 21:29 Adam J. Richter [this message]
2002-11-04 21:37 ` Patch(2.5.45): Eliminate unchecked kfree(disk->random) Alexander Viro
2002-11-04 22:02 Adam J. Richter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20021104132954.A337@baldur.yggdrasil.com \
--to=adam@yggdrasil.com \
--cc=axboe@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=tytso@mit.edu \
--cc=viro@math.psu.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).