lkml.org 
[lkml]   [2002]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectProblem in using kobjects for block devices and partitions
Hello!

Problem code is in register_disk, and though it was included at the end of
November, but problems started to appear very recently.

At the beginning of register_disk() we see this snippet:

s = strchr(disk->kobj.name, '/');
if (s)
*s = '!';
kobject_register(&disk->kobj);
disk_sysfs_symlinks(disk);

And now kobject_register() frees disk structure if registration was
unsuccesful, so doing anything else with this structure is not very
nice thing. This is especially observable if you turn on SLAB poisoning,
then it will die during disk_sysfs_symlinks (actually two levels deeper).

I think here we should at least check return value of kobject_register
and print something useful. This won't help much against crashing since
accessing /proc/partition afterwards will still kill the box (in case
of SLAB poisoning, otherwise some other unpredictable stuff will happen).

It seems that making register_disk() to return error back to caller
does not makes much sence either, since struct gendisk is already destroyed
and we cannot get it off the lists that easily.

May be it was just a bad decision to destroy whole structure of underlying
object when kobject creation have failed?

Anyway at least below patch is a good thing to easy problem diagnostics
when kobject_register() fails during registering block devices (e.g. because
some of them have same base name).

Bye,
Oleg

===== fs/partitions/check.c 1.88 vs edited =====
--- 1.88/fs/partitions/check.c Thu Nov 21 06:08:39 2002
+++ edited/fs/partitions/check.c Sat Nov 23 15:17:28 2002
@@ -400,13 +400,23 @@
struct block_device *bdev;
char *s;
int j;
+ int error;
+ /* In case of error disk structure will be freed, so we cannot
+ dereference it. Let's save some useful info so that we can
+ print meaningful error message. */
+ int minor=disk->first_minor, major=disk->major;

strncpy(disk->kobj.name,disk->disk_name,KOBJ_NAME_LEN);
/* ewww... some of these buggers have / in name... */
s = strchr(disk->kobj.name, '/');
if (s)
*s = '!';
- kobject_register(&disk->kobj);
+
+ error = kobject_register(&disk->kobj);
+ if ( error ) {
+ printk(KERN_ERR "%s: Failed to create kernel object for disk major %d, first minor %d. Reason: %d\n", __FUNCTION__, major, minor, error );
+ return;
+ }
disk_sysfs_symlinks(disk);

if (disk->flags & GENHD_FL_CD)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2005-03-22 13:31    [W:0.634 / U:0.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site