Re: Problem in using kobjects for block devices and partitions

[Patrick Mochel]

>    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? 

Thanks for catching this. There are two things wrong: register_disk() is 
calling kobject_register() instead of kobject_add(). This is fixed with a 
patch I posted earlier today. 

Also, though it is now irrelevant in this case, kobject_register() was 
causing the object to be freed if kobject_add() failed. This is a bit 
rude, though kobject_add() really shoudln't fail. It can fail because of 
these reasons: 

- sysfs isn't mounted yet. 
- we couldn't get an inode.
- the file or directory already existed. 

In each case, we probably want to know about it. So, the patch below 
changes the call to kobject_cleanup() to a WARN_ON(error) to flag it, but 
continue going..

Thanks,

	-pat

===== lib/kobject.c 1.8 vs edited =====
--- 1.8/lib/kobject.c	Thu Nov 21 17:01:53 2002
+++ edited/lib/kobject.c	Mon Nov 25 16:01:53 2002
@@ -110,8 +110,7 @@
 	if (kobj) {
 		kobject_init(kobj);
 		error = kobject_add(kobj);
-		if (error)
-			kobject_cleanup(kobj);
+		WARN_ON(error);
 	} else
 		error = -EINVAL;
 	return error;