* [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file
@ 2010-10-04 20:21 Neil Horman
2010-10-05 0:57 ` [Bonding-devel] " Stephen Hemminger
2010-10-05 13:39 ` [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file (v2) Neil Horman
0 siblings, 2 replies; 5+ messages in thread
From: Neil Horman @ 2010-10-04 20:21 UTC (permalink / raw)
To: netdev; +Cc: bonding-devel, fubar, davem, nhorman
Fix a WARN_ON failure in bond_masters sysfs file
Got a report of this warning recently
bonding: bond0 is being created...
------------[ cut here ]------------
WARNING: at fs/proc/generic.c:590 proc_register+0x14d/0x185()
Hardware name: ProLiant BL465c G1
proc_dir_entry 'bonding/bond0' already registered
Modules linked in: bonding ipv6 tg3 bnx2 shpchp amd64_edac_mod edac_core
ipmi_si
ipmi_msghandler serio_raw i2c_piix4 k8temp edac_mce_amd hpwdt microcode hpsa
cc
iss radeon ttm drm_kms_helper drm i2c_algo_bit i2c_core [last unloaded:
scsi_wai
t_scan]
Pid: 935, comm: ifup-eth Not tainted 2.6.33.5-124.fc13.x86_64 #1
Call Trace:
[<ffffffff8104b54c>] warn_slowpath_common+0x77/0x8f
[<ffffffff8104b5b1>] warn_slowpath_fmt+0x3c/0x3e
[<ffffffff8114bf0b>] proc_register+0x14d/0x185
[<ffffffff8114c20c>] proc_create_data+0x87/0xa1
[<ffffffffa0211e9b>] bond_create_proc_entry+0x55/0x95 [bonding]
[<ffffffffa0215e5d>] bond_init+0x95/0xd0 [bonding]
[<ffffffff8138cd97>] register_netdevice+0xdd/0x29e
[<ffffffffa021240b>] bond_create+0x8e/0xb8 [bonding]
[<ffffffffa021c4be>] bonding_store_bonds+0xb3/0x1c1 [bonding]
[<ffffffff812aec85>] class_attr_store+0x27/0x29
[<ffffffff8115423d>] sysfs_write_file+0x10f/0x14b
[<ffffffff81101acf>] vfs_write+0xa9/0x106
[<ffffffff81101be2>] sys_write+0x45/0x69
[<ffffffff81009b02>] system_call_fastpath+0x16/0x1b
---[ end trace a677c3f7f8b16b1e ]---
bonding: Bond creation failed.
It happens because a user space writer to bond_master can try to register and
already existing bond interface name. Fix it by teaching bond_create to check
for the existance of devices with that name first in cases where a non-NULL name
parameter has been passed in
Signed-off-by: Neil Horman <nhorman@tuxdriver•com>
bond_main.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index fb70c3e..10e4ffe 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5148,7 +5148,7 @@ static struct rtnl_link_ops bond_link_ops __read_mostly = {
*/
int bond_create(struct net *net, const char *name)
{
- struct net_device *bond_dev;
+ struct net_device *bond_dev, *check_dev;
int res;
rtnl_lock();
@@ -5168,6 +5168,20 @@ int bond_create(struct net *net, const char *name)
res = dev_alloc_name(bond_dev, "bond%d");
if (res < 0)
goto out;
+ } else {
+ /*
+ * If we're given a name to register
+ * we need to ensure that its not already
+ * registered
+ */
+ check_dev = dev_get_by_name(net, name);
+
+ res = (check_dev) ? 0 : -EEXIST;
+
+ dev_put(check_dev);
+
+ if (res)
+ goto out;
}
res = register_netdevice(bond_dev);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Bonding-devel] [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file
2010-10-04 20:21 [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file Neil Horman
@ 2010-10-05 0:57 ` Stephen Hemminger
2010-10-05 2:26 ` Neil Horman
2010-10-05 13:39 ` [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file (v2) Neil Horman
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2010-10-05 0:57 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, fubar, bonding-devel, davem
On Mon, 4 Oct 2010 16:21:12 -0400
Neil Horman <nhorman@tuxdriver•com> wrote:
> Fix a WARN_ON failure in bond_masters sysfs file
>
> Got a report of this warning recently
>
> bonding: bond0 is being created...
> ------------[ cut here ]------------
> WARNING: at fs/proc/generic.c:590 proc_register+0x14d/0x185()
> Hardware name: ProLiant BL465c G1
> proc_dir_entry 'bonding/bond0' already registered
> Modules linked in: bonding ipv6 tg3 bnx2 shpchp amd64_edac_mod edac_core
> ipmi_si
> ipmi_msghandler serio_raw i2c_piix4 k8temp edac_mce_amd hpwdt microcode hpsa
> cc
> iss radeon ttm drm_kms_helper drm i2c_algo_bit i2c_core [last unloaded:
> scsi_wai
> t_scan]
> Pid: 935, comm: ifup-eth Not tainted 2.6.33.5-124.fc13.x86_64 #1
> Call Trace:
> [<ffffffff8104b54c>] warn_slowpath_common+0x77/0x8f
> [<ffffffff8104b5b1>] warn_slowpath_fmt+0x3c/0x3e
> [<ffffffff8114bf0b>] proc_register+0x14d/0x185
> [<ffffffff8114c20c>] proc_create_data+0x87/0xa1
> [<ffffffffa0211e9b>] bond_create_proc_entry+0x55/0x95 [bonding]
> [<ffffffffa0215e5d>] bond_init+0x95/0xd0 [bonding]
> [<ffffffff8138cd97>] register_netdevice+0xdd/0x29e
> [<ffffffffa021240b>] bond_create+0x8e/0xb8 [bonding]
> [<ffffffffa021c4be>] bonding_store_bonds+0xb3/0x1c1 [bonding]
> [<ffffffff812aec85>] class_attr_store+0x27/0x29
> [<ffffffff8115423d>] sysfs_write_file+0x10f/0x14b
> [<ffffffff81101acf>] vfs_write+0xa9/0x106
> [<ffffffff81101be2>] sys_write+0x45/0x69
> [<ffffffff81009b02>] system_call_fastpath+0x16/0x1b
> ---[ end trace a677c3f7f8b16b1e ]---
> bonding: Bond creation failed.
>
> It happens because a user space writer to bond_master can try to register and
> already existing bond interface name. Fix it by teaching bond_create to check
> for the existance of devices with that name first in cases where a non-NULL name
> parameter has been passed in
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver•com>
>
> bond_main.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index fb70c3e..10e4ffe 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -5148,7 +5148,7 @@ static struct rtnl_link_ops bond_link_ops __read_mostly = {
> */
> int bond_create(struct net *net, const char *name)
> {
> - struct net_device *bond_dev;
> + struct net_device *bond_dev, *check_dev;
> int res;
>
> rtnl_lock();
> @@ -5168,6 +5168,20 @@ int bond_create(struct net *net, const char *name)
> res = dev_alloc_name(bond_dev, "bond%d");
> if (res < 0)
> goto out;
> + } else {
> + /*
> + * If we're given a name to register
> + * we need to ensure that its not already
> + * registered
> + */
> + check_dev = dev_get_by_name(net, name);
> +
> + res = (check_dev) ? 0 : -EEXIST;
> +
> + dev_put(check_dev);
> +
> + if (res)
> + goto out;
> }
>
> res = register_netdevice(bond_dev);
Why is this necessary?
register_netdevice does already check for duplicate name so please
use it's return value instead
If that doesn't work then use __dev_get_by_name to avoid
ref count (ie dev_put).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bonding-devel] [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file
2010-10-05 0:57 ` [Bonding-devel] " Stephen Hemminger
@ 2010-10-05 2:26 ` Neil Horman
0 siblings, 0 replies; 5+ messages in thread
From: Neil Horman @ 2010-10-05 2:26 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, fubar, bonding-devel, davem
On Tue, Oct 05, 2010 at 09:57:13AM +0900, Stephen Hemminger wrote:
> On Mon, 4 Oct 2010 16:21:12 -0400
> Neil Horman <nhorman@tuxdriver•com> wrote:
>
> > Fix a WARN_ON failure in bond_masters sysfs file
> >
> > Got a report of this warning recently
> >
> > bonding: bond0 is being created...
> > ------------[ cut here ]------------
> > WARNING: at fs/proc/generic.c:590 proc_register+0x14d/0x185()
> > Hardware name: ProLiant BL465c G1
> > proc_dir_entry 'bonding/bond0' already registered
> > Modules linked in: bonding ipv6 tg3 bnx2 shpchp amd64_edac_mod edac_core
> > ipmi_si
> > ipmi_msghandler serio_raw i2c_piix4 k8temp edac_mce_amd hpwdt microcode hpsa
> > cc
> > iss radeon ttm drm_kms_helper drm i2c_algo_bit i2c_core [last unloaded:
> > scsi_wai
> > t_scan]
> > Pid: 935, comm: ifup-eth Not tainted 2.6.33.5-124.fc13.x86_64 #1
> > Call Trace:
> > [<ffffffff8104b54c>] warn_slowpath_common+0x77/0x8f
> > [<ffffffff8104b5b1>] warn_slowpath_fmt+0x3c/0x3e
> > [<ffffffff8114bf0b>] proc_register+0x14d/0x185
> > [<ffffffff8114c20c>] proc_create_data+0x87/0xa1
> > [<ffffffffa0211e9b>] bond_create_proc_entry+0x55/0x95 [bonding]
> > [<ffffffffa0215e5d>] bond_init+0x95/0xd0 [bonding]
> > [<ffffffff8138cd97>] register_netdevice+0xdd/0x29e
> > [<ffffffffa021240b>] bond_create+0x8e/0xb8 [bonding]
> > [<ffffffffa021c4be>] bonding_store_bonds+0xb3/0x1c1 [bonding]
> > [<ffffffff812aec85>] class_attr_store+0x27/0x29
> > [<ffffffff8115423d>] sysfs_write_file+0x10f/0x14b
> > [<ffffffff81101acf>] vfs_write+0xa9/0x106
> > [<ffffffff81101be2>] sys_write+0x45/0x69
> > [<ffffffff81009b02>] system_call_fastpath+0x16/0x1b
> > ---[ end trace a677c3f7f8b16b1e ]---
> > bonding: Bond creation failed.
> >
> > It happens because a user space writer to bond_master can try to register and
> > already existing bond interface name. Fix it by teaching bond_create to check
> > for the existance of devices with that name first in cases where a non-NULL name
> > parameter has been passed in
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver•com>
> >
> > bond_main.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > index fb70c3e..10e4ffe 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -5148,7 +5148,7 @@ static struct rtnl_link_ops bond_link_ops __read_mostly = {
> > */
> > int bond_create(struct net *net, const char *name)
> > {
> > - struct net_device *bond_dev;
> > + struct net_device *bond_dev, *check_dev;
> > int res;
> >
> > rtnl_lock();
> > @@ -5168,6 +5168,20 @@ int bond_create(struct net *net, const char *name)
> > res = dev_alloc_name(bond_dev, "bond%d");
> > if (res < 0)
> > goto out;
> > + } else {
> > + /*
> > + * If we're given a name to register
> > + * we need to ensure that its not already
> > + * registered
> > + */
> > + check_dev = dev_get_by_name(net, name);
> > +
> > + res = (check_dev) ? 0 : -EEXIST;
> > +
> > + dev_put(check_dev);
> > +
> > + if (res)
> > + goto out;
> > }
> >
> > res = register_netdevice(bond_dev);
>
> Why is this necessary?
> register_netdevice does already check for duplicate name so please
> use it's return value instead
>
No, its the call to register_netdev that causes the WARN_ON. Check the stack
trace, register_netdev call bond_init which in turn registers a proc interface
by an existing name, which causes the WARN_ON
> If that doesn't work then use __dev_get_by_name to avoid
> ref count (ie dev_put).
>
Yeah, thats a good idea. I'll respin this in the AM. Thanks!
Regards
Neil
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file (v2)
2010-10-04 20:21 [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file Neil Horman
2010-10-05 0:57 ` [Bonding-devel] " Stephen Hemminger
@ 2010-10-05 13:39 ` Neil Horman
2010-10-06 3:06 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Neil Horman @ 2010-10-05 13:39 UTC (permalink / raw)
To: netdev; +Cc: bonding-devel, fubar, davem, shemminger
Ok, V2 of this patch, taking Stephens notes into account. Switched to using
__dev_get_by_name to avoid reference count inc/dec.
Fix a WARN_ON failure in bond_masters sysfs file
Got a report of this warning recently
bonding: bond0 is being created...
------------[ cut here ]------------
WARNING: at fs/proc/generic.c:590 proc_register+0x14d/0x185()
Hardware name: ProLiant BL465c G1
proc_dir_entry 'bonding/bond0' already registered
Modules linked in: bonding ipv6 tg3 bnx2 shpchp amd64_edac_mod edac_core
ipmi_si
ipmi_msghandler serio_raw i2c_piix4 k8temp edac_mce_amd hpwdt microcode hpsa
cc
iss radeon ttm drm_kms_helper drm i2c_algo_bit i2c_core [last unloaded:
scsi_wai
t_scan]
Pid: 935, comm: ifup-eth Not tainted 2.6.33.5-124.fc13.x86_64 #1
Call Trace:
[<ffffffff8104b54c>] warn_slowpath_common+0x77/0x8f
[<ffffffff8104b5b1>] warn_slowpath_fmt+0x3c/0x3e
[<ffffffff8114bf0b>] proc_register+0x14d/0x185
[<ffffffff8114c20c>] proc_create_data+0x87/0xa1
[<ffffffffa0211e9b>] bond_create_proc_entry+0x55/0x95 [bonding]
[<ffffffffa0215e5d>] bond_init+0x95/0xd0 [bonding]
[<ffffffff8138cd97>] register_netdevice+0xdd/0x29e
[<ffffffffa021240b>] bond_create+0x8e/0xb8 [bonding]
[<ffffffffa021c4be>] bonding_store_bonds+0xb3/0x1c1 [bonding]
[<ffffffff812aec85>] class_attr_store+0x27/0x29
[<ffffffff8115423d>] sysfs_write_file+0x10f/0x14b
[<ffffffff81101acf>] vfs_write+0xa9/0x106
[<ffffffff81101be2>] sys_write+0x45/0x69
[<ffffffff81009b02>] system_call_fastpath+0x16/0x1b
---[ end trace a677c3f7f8b16b1e ]---
bonding: Bond creation failed.
It happens because a user space writer to bond_master can try to register and
already existing bond interface name. Fix it by teaching bond_create to check
for the existance of devices with that name first in cases where a non-NULL name
parameter has been passed in
Signed-off-by: Neil Horman <nhorman@tuxdriver•com>
bond_main.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index fb70c3e..985cbc1 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5168,6 +5168,15 @@ int bond_create(struct net *net, const char *name)
res = dev_alloc_name(bond_dev, "bond%d");
if (res < 0)
goto out;
+ } else {
+ /*
+ * If we're given a name to register
+ * we need to ensure that its not already
+ * registered
+ */
+ res = -EEXIST;
+ if (__dev_get_by_name(net, name) != NULL)
+ goto out;
}
res = register_netdevice(bond_dev);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file (v2)
2010-10-05 13:39 ` [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file (v2) Neil Horman
@ 2010-10-06 3:06 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2010-10-06 3:06 UTC (permalink / raw)
To: nhorman; +Cc: netdev, bonding-devel, fubar, shemminger
From: Neil Horman <nhorman@tuxdriver•com>
Date: Tue, 5 Oct 2010 09:39:21 -0400
> Ok, V2 of this patch, taking Stephens notes into account. Switched to using
> __dev_get_by_name to avoid reference count inc/dec.
Applied, thanks Neil.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-10-06 3:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-04 20:21 [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file Neil Horman
2010-10-05 0:57 ` [Bonding-devel] " Stephen Hemminger
2010-10-05 2:26 ` Neil Horman
2010-10-05 13:39 ` [PATCH] bonding: fix WARN_ON when writing to bond_master sysfs file (v2) Neil Horman
2010-10-06 3:06 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox