[PATCH] bonding: Fix sysfs attribute handling

[PATCH] bonding: Fix sysfs attribute handling

[Libor Pechacek]

bonding: Fix sysfs attribute handling

For bonding interfaces any attempt to read the sysfs directory contents after
module removal results in an oops.  The fix is to release sysfs attributes
for the interfaces upon module unload.

Signed-off-by: Libor Pechacek <lpechacek@suse.cz>
--
 drivers/net/bonding/bond_main.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0942d82..33767d4 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4528,8 +4528,7 @@ static void bond_free_all(void)
 		netif_tx_unlock_bh(bond_dev);
 		/* Release the bonded slaves */
 		bond_release_all(bond_dev);
-		bond_deinit(bond_dev);
-		unregister_netdevice(bond_dev);
+		bond_destroy(bond);
 	}
 
 #ifdef CONFIG_PROC_FS

Re: [PATCH] bonding: Fix sysfs attribute handling

[Andrew Morton]

On Fri, 21 Mar 2008 14:52:55 +0100 Libor Pechacek <lpechacek@suse.cz> wrote:

> bonding: Fix sysfs attribute handling
> 
> For bonding interfaces any attempt to read the sysfs directory contents after
> module removal results in an oops.  The fix is to release sysfs attributes
> for the interfaces upon module unload.
> 
> Signed-off-by: Libor Pechacek <lpechacek@suse.cz>
> --
>  drivers/net/bonding/bond_main.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 0942d82..33767d4 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4528,8 +4528,7 @@ static void bond_free_all(void)
>  		netif_tx_unlock_bh(bond_dev);
>  		/* Release the bonded slaves */
>  		bond_release_all(bond_dev);
> -		bond_deinit(bond_dev);
> -		unregister_netdevice(bond_dev);
> +		bond_destroy(bond);
>  	}

Is this also needed in 2.6.24.x?

Re: [PATCH] bonding: Fix sysfs attribute handling

[Jay Vosburgh]

Andrew Morton <akpm@linux-foundation.org> wrote:

>On Fri, 21 Mar 2008 14:52:55 +0100 Libor Pechacek <lpechacek@suse.cz> wrote:
>
>> bonding: Fix sysfs attribute handling
>> 
>> For bonding interfaces any attempt to read the sysfs directory contents after
>> module removal results in an oops.  The fix is to release sysfs attributes
>> for the interfaces upon module unload.
>> 
>> Signed-off-by: Libor Pechacek <lpechacek@suse.cz>
>> --
>>  drivers/net/bonding/bond_main.c |    3 +--
>>  1 files changed, 1 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 0942d82..33767d4 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -4528,8 +4528,7 @@ static void bond_free_all(void)
>>  		netif_tx_unlock_bh(bond_dev);
>>  		/* Release the bonded slaves */
>>  		bond_release_all(bond_dev);
>> -		bond_deinit(bond_dev);
>> -		unregister_netdevice(bond_dev);
>> +		bond_destroy(bond);
>>  	}
>
>Is this also needed in 2.6.24.x?

	Yes (I haven't tested it personally, but the affected code is
the same).  I was going to forward this to stable when it was accepted
upstream.  The oops is fairly straightforward to create:

# modprobe bonding
# cd /sys/class/net/bond0/bonding
# rmmod bonding
# ls

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

Re: [PATCH] bonding: Fix sysfs attribute handling

[lpechacek]

> Andrew Morton <akpm@linux-foundation.org> wrote:
>
>>On Fri, 21 Mar 2008 14:52:55 +0100 Libor Pechacek <lpechacek@suse.cz>
>> wrote:
>>
>>> bonding: Fix sysfs attribute handling
>>>
>>> For bonding interfaces any attempt to read the sysfs directory contents
>>> after
>>> module removal results in an oops.  The fix is to release sysfs
>>> attributes
>>> for the interfaces upon module unload.
>>>
>>> Signed-off-by: Libor Pechacek <lpechacek@suse.cz>
>>> --
>>>  drivers/net/bonding/bond_main.c |    3 +--
>>>  1 files changed, 1 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_main.c
>>> b/drivers/net/bonding/bond_main.c
>>> index 0942d82..33767d4 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -4528,8 +4528,7 @@ static void bond_free_all(void)
>>>  		netif_tx_unlock_bh(bond_dev);
>>>  		/* Release the bonded slaves */
>>>  		bond_release_all(bond_dev);
>>> -		bond_deinit(bond_dev);
>>> -		unregister_netdevice(bond_dev);
>>> +		bond_destroy(bond);
>>>  	}
>>
>>Is this also needed in 2.6.24.x?
>
> 	Yes (I haven't tested it personally, but the affected code is
> the same).  I was going to forward this to stable when it was accepted
> upstream.  The oops is fairly straightforward to create:
>
> # modprobe bonding
> # cd /sys/class/net/bond0/bonding
> # rmmod bonding
> # ls

Yes, exactly.

I tested the patch with the latest Linus' tree (GIT commit
49a5ba46c5d1e34bcb07634157b29d7414ce13bd), but I first found the bug in
2.6.16.

Libor