diff options
author | Alexey Dobriyan <adobriyan@gmail.com> | 2009-03-25 15:48:06 -0400 |
---|---|---|
committer | Alexey Dobriyan <adobriyan@gmail.com> | 2009-03-30 17:14:44 -0400 |
commit | 99b76233803beab302123d243eea9e41149804f3 (patch) | |
tree | 398178210fe66845ccd6fa4258ba762a87e023ad /drivers/net/bonding | |
parent | 3dec7f59c370c7b58184d63293c3dc984d475840 (diff) |
proc 2/2: remove struct proc_dir_entry::owner
Setting ->owner as done currently (pde->owner = THIS_MODULE) is racy
as correctly noted at bug #12454. Someone can lookup entry with NULL
->owner, thus not pinning enything, and release it later resulting
in module refcount underflow.
We can keep ->owner and supply it at registration time like ->proc_fops
and ->data.
But this leaves ->owner as easy-manipulative field (just one C assignment)
and somebody will forget to unpin previous/pin current module when
switching ->owner. ->proc_fops is declared as "const" which should give
some thoughts.
->read_proc/->write_proc were just fixed to not require ->owner for
protection.
rmmod'ed directories will be empty and return "." and ".." -- no harm.
And directories with tricky enough readdir and lookup shouldn't be modular.
We definitely don't want such modular code.
Removing ->owner will also make PDE smaller.
So, let's nuke it.
Kudos to Jeff Layton for reminding about this, let's say, oversight.
http://bugzilla.kernel.org/show_bug.cgi?id=12454
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Diffstat (limited to 'drivers/net/bonding')
-rw-r--r-- | drivers/net/bonding/bond_main.c | 35 |
1 files changed, 2 insertions, 33 deletions
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 9c326a50a3ee..99610f358c40 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c | |||
@@ -3444,25 +3444,12 @@ static void bond_remove_proc_entry(struct bonding *bond) | |||
3444 | */ | 3444 | */ |
3445 | static void bond_create_proc_dir(void) | 3445 | static void bond_create_proc_dir(void) |
3446 | { | 3446 | { |
3447 | int len = strlen(DRV_NAME); | ||
3448 | |||
3449 | for (bond_proc_dir = init_net.proc_net->subdir; bond_proc_dir; | ||
3450 | bond_proc_dir = bond_proc_dir->next) { | ||
3451 | if ((bond_proc_dir->namelen == len) && | ||
3452 | !memcmp(bond_proc_dir->name, DRV_NAME, len)) { | ||
3453 | break; | ||
3454 | } | ||
3455 | } | ||
3456 | |||
3457 | if (!bond_proc_dir) { | 3447 | if (!bond_proc_dir) { |
3458 | bond_proc_dir = proc_mkdir(DRV_NAME, init_net.proc_net); | 3448 | bond_proc_dir = proc_mkdir(DRV_NAME, init_net.proc_net); |
3459 | if (bond_proc_dir) { | 3449 | if (!bond_proc_dir) |
3460 | bond_proc_dir->owner = THIS_MODULE; | ||
3461 | } else { | ||
3462 | printk(KERN_WARNING DRV_NAME | 3450 | printk(KERN_WARNING DRV_NAME |
3463 | ": Warning: cannot create /proc/net/%s\n", | 3451 | ": Warning: cannot create /proc/net/%s\n", |
3464 | DRV_NAME); | 3452 | DRV_NAME); |
3465 | } | ||
3466 | } | 3453 | } |
3467 | } | 3454 | } |
3468 | 3455 | ||
@@ -3471,25 +3458,7 @@ static void bond_create_proc_dir(void) | |||
3471 | */ | 3458 | */ |
3472 | static void bond_destroy_proc_dir(void) | 3459 | static void bond_destroy_proc_dir(void) |
3473 | { | 3460 | { |
3474 | struct proc_dir_entry *de; | 3461 | if (bond_proc_dir) { |
3475 | |||
3476 | if (!bond_proc_dir) { | ||
3477 | return; | ||
3478 | } | ||
3479 | |||
3480 | /* verify that the /proc dir is empty */ | ||
3481 | for (de = bond_proc_dir->subdir; de; de = de->next) { | ||
3482 | /* ignore . and .. */ | ||
3483 | if (*(de->name) != '.') { | ||
3484 | break; | ||
3485 | } | ||
3486 | } | ||
3487 | |||
3488 | if (de) { | ||
3489 | if (bond_proc_dir->owner == THIS_MODULE) { | ||
3490 | bond_proc_dir->owner = NULL; | ||
3491 | } | ||
3492 | } else { | ||
3493 | remove_proc_entry(DRV_NAME, init_net.proc_net); | 3462 | remove_proc_entry(DRV_NAME, init_net.proc_net); |
3494 | bond_proc_dir = NULL; | 3463 | bond_proc_dir = NULL; |
3495 | } | 3464 | } |