2002-10-19 08:04:58

by Andres Salomon

[permalink] [raw]
Subject: [PATCH] 2.5.44 Fix uninitialized device struct w/ add_disk()

This occurs w/ 2.5 device-mapper; add_disk() is called (w/ a possibly
invalid gendisk). add_disk() calls register_disk(), which constructs
the device struct (gendisk::disk_dev). First, the bus_id field is
initialized, and then device_add() is called. However, device_add()
does no initialization of the device struct. So, since the
gendisk::disk_dev::parent list is NULL, the gendisk::disk_dev::node
field is never initialized. Later on in device_add(), if an error has
occurred, list_del_init(&dev->node) is called; dev->node is {NULL,NULL},
and an oops occurs.

Instead of calling device_add(), my patch makes register_disk() call
device_register(). This appear to do the same thing as device_add(),
but initializes the device struct before calling device_add().



--
It's not denial. I'm just selective about the reality I accept.
-- Bill Watterson


Attachments:
(No filename) (862.00 B)
check.c.diff (377.00 B)
Download all attachments

2002-10-19 08:47:59

by Alexander Viro

[permalink] [raw]
Subject: Re: [PATCH] 2.5.44 Fix uninitialized device struct w/ add_disk()



On Sat, 19 Oct 2002, Andres Salomon wrote:

> This occurs w/ 2.5 device-mapper; add_disk() is called (w/ a possibly
> invalid gendisk). add_disk() calls register_disk(), which constructs
> the device struct (gendisk::disk_dev). First, the bus_id field is
> initialized, and then device_add() is called. However, device_add()
> does no initialization of the device struct. So, since the
> gendisk::disk_dev::parent list is NULL, the gendisk::disk_dev::node
> field is never initialized. Later on in device_add(), if an error has
> occurred, list_del_init(&dev->node) is called; dev->node is {NULL,NULL},
> and an oops occurs.
>
> Instead of calling device_add(), my patch makes register_disk() call
> device_register(). This appear to do the same thing as device_add(),
> but initializes the device struct before calling device_add().

Wrong. If device-mapper doesn't create gendisk with alloc_disk() - it's
their bug; if it does - initialization is already done at alloc_disk()
time.

Fix driver instead of breaking generic code.