linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] netconsole: avoid a crash with multiple sysfs writers
@ 2013-08-07  9:02 Dan Aloni
  2013-08-08  5:50 ` Neil Horman
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Aloni @ 2013-08-07  9:02 UTC (permalink / raw)
  To: linux-kernel; +Cc: David S. Miller, Neil Horman

When my 'ifup eth' script was fired multiple times and ran concurrent on
my laptop, for some obscure /etc scripting reason, it was revealed
that the store_enabled() function in netconsole doesn't handle it nicely,
as recorded by the Oops below (a syslog paste, but not mangled too much
to prevent from discerning the traceback).

On Linux 3.10.4, this patch seeks to remedy the problem, and it has been
running stable on my laptop for a few days.

[52608.609325] BUG: unable to handle kernel NULL pointer dereference at 00000000000003e0
[52608.609331] IP: [<ffffffff81532a17>] __netpoll_cleanup+0x27/0xe0
[52608.609339] PGD 15e51a067 PUD 15433e067 PMD 0
[52608.609343] Oops: 0000 [#1] SMP re firewire_ohci firewire_core crc_itu_t [last unloaded: kvm_intel]
[52608.609347] Modules linked in: kvm_intel tun vfat fat ppdev parport_pc parport fuse ipt_MASQUERADE usb_storage nf_conntrack_netbios_ns nf_connAug  5 09:39:28 inception kernel: [52608.609419] CPU: 1 PID: 13467 Comm: netconsole-osir Tainted: PF          O 3.10.4-alonid #15ntrack_ipv4 bridge nf_defrag_ipv4 xt_conntrack nf_Aug  5 09:39:28 inception kernel: [52608.609422] Hardware name: LENOVO 24474KG/24474KG, BIOS G5ET90WW (2.50 ) 12/21/2012vm snd_hda_codec_hdmi mac80211 snd_hda_codec_realtek vsock Aug  5 09:39:28 inception kernel: [52608.609424] task: ffff88015a4397a0 ti: ffff8801ba446000 task.ti: ffff8801ba446000lloc nvidia(POF) videobuf2_memops snd_hwdep snd_seq videobuf2Aug  5 09:39:28 inception kernel: [52608.609426] RIP: 0010:[<ffffffff81532a17>]  [<ffffffff81532a17>] __netpoll_cleanup+0x27/0xe0imer media snd i2c_i801 ptp lpc_ich mfd_core pps_cAug  5 09:39:28 inception kernel: [52608.609430] RSP: 0018:ffff8801ba447de8  EFLAGS: 00010286e_core crc_itu_t [last unloaded: kvm_intel]
[52608.609433] RAX: 0000000000000000 RBX: ffff880210bbcc68 RCX: 0000000000000000
[52608.609435] RDX: 0000000000000000 RSI: ffff8801ba447da0 RDI: ffff880210bbcc68
[52608.609437] RBP: ffff8801ba447e18 R08: 0000000000000000 R09: 0000000000000001
[52608.609439] R10: 000000000000000a R11: f000000000000000 R12: ffff880210bbcc68
[52608.609441] R13: ffff88020bc41000 R14: 0000000000000002 R15: 000000000000000200000000000
[52608.609443] FS:  00007f38d7bff740(0000) GS:ffff88021dc40000(0000) knlGS:0000000000000000
[52608.609446] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003300000000001427e0
[52608.609448] CR2: 00000000000003e0 CR3: 0000000154103000 CR4: 00000000001427e0
[52608.609450] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[52608.609452] netpoll: netconsole: local port 6665ess 10.0.0.27
[52608.609454] netpoll: netconsole: local IPv4 address 10.0.0.27
[52608.609456] netpoll: netconsole: interface 'em1'
[52608.609457] netpoll: netconsole: remote port 514ress 10.0.0.15
[52608.609459] netpoll: netconsole: remote IPv4 address 10.0.0.15:65:a8:9a:c7
[52608.609461] netpoll: netconsole: remote ethernet address 1c:6f:65:a8:9a:c7400
[52608.609463] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[52608.609464] Stack:801ba447e08 ffff880210bbcc68 ffffffffffffffea ffff88020bc41000
[52608.609466]  ffff8801ba447e08 ffff880210bbcc68 ffffffffffffffea ffff88020bc41000
[52608.609471]  0000000000000002 0000000000000002 ffff8801ba447e38 ffffffff81532af4
[52608.609475]  0000000000000000 ffff880210bbcc00 ffff8801ba447e78 ffffffff81420e7c
[52608.609479] Call Trace:
[52608.609484]  [<ffffffff81532af4>] netpoll_cleanup+0x24/0x50
[52608.609489]  [<ffffffff81420e7c>] store_enabled+0x5c/0xe0store+0x2e/0x40
[52608.609492]  [<ffffffff81420abe>] netconsole_target_attr_store+0x2e/0x40
[52608.609498]  [<ffffffff811ff2a2>] configfs_write_file+0xd2/0x130
[52608.609503]  [<ffffffff81188f95>] vfs_write+0xc5/0x1f0
[52608.609506]  [<ffffffff81189482>] SyS_write+0x52/0xa0/0x10
[52608.609511]  [<ffffffff81628c2e>] ? do_page_fault+0xe/0x10
[52608.609516]  [<ffffffff8162d402>] system_call_fastpath+0x16/0x1b
[52608.609517] Code: 1f 44 00 00 0f 1f 44 00 00 55 48 89 e5 48 83 ec 30 4c 89 65 e0 48 89 5d d8 49 89 fc 4c 89 6d e8 4c 89 75 f0 4c 89 7d f8 48 8
[52608.609559] RIP  [<ffffffff81532a17>] __netpoll_cleanup+0x27/0xe0
[52608.609563]  RSP <ffff8801ba447de8>
[52608.609564] CR2: 00000000000003e0
[52608.609567] ---[ end trace d25ec343349b61d2 ]---

Signed-off-by: Dan Aloni <alonid@postram.com>
CC: David S. Miller <davem@davemloft.net>
CC: Neil Horman <nhorman@tuxdriver.com>
---
 drivers/net/netconsole.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 4822aaf..dcb2134 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -102,6 +102,7 @@ struct netconsole_target {
 	struct config_item	item;
 #endif
 	int			enabled;
+	struct mutex		mutex;
 	struct netpoll		np;
 };
 
@@ -181,6 +182,7 @@ static struct netconsole_target *alloc_param_target(char *target_config)
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
 	nt->np.local_port = 6665;
 	nt->np.remote_port = 6666;
+	mutex_init(&nt->mutex);
 	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
 
 	/* Parse parameters and setup netpoll */
@@ -322,6 +324,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		return -EINVAL;
 	}
 
+	mutex_lock(&nt->mutex);
 	if (enabled) {	/* 1 */
 
 		/*
@@ -331,8 +334,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		netpoll_print_options(&nt->np);
 
 		err = netpoll_setup(&nt->np);
-		if (err)
+		if (err) {
+			mutex_unlock(&nt->mutex);
 			return err;
+		}
 
 		printk(KERN_INFO "netconsole: network logging started\n");
 
@@ -341,6 +346,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 	}
 
 	nt->enabled = enabled;
+	mutex_unlock(&nt->mutex);
 
 	return strnlen(buf, count);
 }
@@ -597,6 +603,7 @@ static struct config_item *make_netconsole_target(struct config_group *group,
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
 	nt->np.local_port = 6665;
 	nt->np.remote_port = 6666;
+	mutex_init(&nt->mutex);
 	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
 
 	/* Initialize the config_item member */
@@ -682,7 +689,11 @@ restart:
 				 * we might sleep in __netpoll_cleanup()
 				 */
 				spin_unlock_irqrestore(&target_list_lock, flags);
+
+				mutex_lock(&nt->mutex);
 				__netpoll_cleanup(&nt->np);
+				mutex_unlock(&nt->mutex);
+
 				spin_lock_irqsave(&target_list_lock, flags);
 				dev_put(nt->np.dev);
 				nt->np.dev = NULL;
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] netconsole: avoid a crash with multiple sysfs writers
  2013-08-07  9:02 [PATCH] netconsole: avoid a crash with multiple sysfs writers Dan Aloni
@ 2013-08-08  5:50 ` Neil Horman
  2013-08-08  6:14   ` Dan Aloni
  0 siblings, 1 reply; 6+ messages in thread
From: Neil Horman @ 2013-08-08  5:50 UTC (permalink / raw)
  To: Dan Aloni; +Cc: linux-kernel, David S. Miller

On Wed, Aug 07, 2013 at 12:02:44PM +0300, Dan Aloni wrote:
> When my 'ifup eth' script was fired multiple times and ran concurrent on
> my laptop, for some obscure /etc scripting reason, it was revealed
> that the store_enabled() function in netconsole doesn't handle it nicely,
> as recorded by the Oops below (a syslog paste, but not mangled too much
> to prevent from discerning the traceback).
> 
> On Linux 3.10.4, this patch seeks to remedy the problem, and it has been
> running stable on my laptop for a few days.
> 
> [52608.609325] BUG: unable to handle kernel NULL pointer dereference at 00000000000003e0
> [52608.609331] IP: [<ffffffff81532a17>] __netpoll_cleanup+0x27/0xe0
> [52608.609339] PGD 15e51a067 PUD 15433e067 PMD 0
> [52608.609343] Oops: 0000 [#1] SMP re firewire_ohci firewire_core crc_itu_t [last unloaded: kvm_intel]
> [52608.609347] Modules linked in: kvm_intel tun vfat fat ppdev parport_pc parport fuse ipt_MASQUERADE usb_storage nf_conntrack_netbios_ns nf_connAug  5 09:39:28 inception kernel: [52608.609419] CPU: 1 PID: 13467 Comm: netconsole-osir Tainted: PF          O 3.10.4-alonid #15ntrack_ipv4 bridge nf_defrag_ipv4 xt_conntrack nf_Aug  5 09:39:28 inception kernel: [52608.609422] Hardware name: LENOVO 24474KG/24474KG, BIOS G5ET90WW (2.50 ) 12/21/2012vm snd_hda_codec_hdmi mac80211 snd_hda_codec_realtek vsock Aug  5 09:39:28 inception kernel: [52608.609424] task: ffff88015a4397a0 ti: ffff8801ba446000 task.ti: ffff8801ba446000lloc nvidia(POF) videobuf2_memops snd_hwdep snd_seq videobuf2Aug  5 09:39:28 inception kernel: [52608.609426] RIP: 0010:[<ffffffff81532a17>]  [<ffffffff81532a17>] __netpoll_cleanup+0x27/0xe0imer media snd i2c_i801 ptp lpc_ich mfd_core pps_cAug  5 09:39:28 inception kernel: [52608.609430] RSP: 0018:ffff8801ba447de8  EFLAGS: 00010286e_core crc_itu_t [last unloaded: kvm_intel]
> [52608.609433] RAX: 0000000000000000 RBX: ffff880210bbcc68 RCX: 0000000000000000
> [52608.609435] RDX: 0000000000000000 RSI: ffff8801ba447da0 RDI: ffff880210bbcc68
> [52608.609437] RBP: ffff8801ba447e18 R08: 0000000000000000 R09: 0000000000000001
> [52608.609439] R10: 000000000000000a R11: f000000000000000 R12: ffff880210bbcc68
> [52608.609441] R13: ffff88020bc41000 R14: 0000000000000002 R15: 000000000000000200000000000
> [52608.609443] FS:  00007f38d7bff740(0000) GS:ffff88021dc40000(0000) knlGS:0000000000000000
> [52608.609446] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003300000000001427e0
> [52608.609448] CR2: 00000000000003e0 CR3: 0000000154103000 CR4: 00000000001427e0
> [52608.609450] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [52608.609452] netpoll: netconsole: local port 6665ess 10.0.0.27
> [52608.609454] netpoll: netconsole: local IPv4 address 10.0.0.27
> [52608.609456] netpoll: netconsole: interface 'em1'
> [52608.609457] netpoll: netconsole: remote port 514ress 10.0.0.15
> [52608.609459] netpoll: netconsole: remote IPv4 address 10.0.0.15:65:a8:9a:c7
> [52608.609461] netpoll: netconsole: remote ethernet address 1c:6f:65:a8:9a:c7400
> [52608.609463] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [52608.609464] Stack:801ba447e08 ffff880210bbcc68 ffffffffffffffea ffff88020bc41000
> [52608.609466]  ffff8801ba447e08 ffff880210bbcc68 ffffffffffffffea ffff88020bc41000
> [52608.609471]  0000000000000002 0000000000000002 ffff8801ba447e38 ffffffff81532af4
> [52608.609475]  0000000000000000 ffff880210bbcc00 ffff8801ba447e78 ffffffff81420e7c
> [52608.609479] Call Trace:
> [52608.609484]  [<ffffffff81532af4>] netpoll_cleanup+0x24/0x50
> [52608.609489]  [<ffffffff81420e7c>] store_enabled+0x5c/0xe0store+0x2e/0x40
> [52608.609492]  [<ffffffff81420abe>] netconsole_target_attr_store+0x2e/0x40
> [52608.609498]  [<ffffffff811ff2a2>] configfs_write_file+0xd2/0x130
> [52608.609503]  [<ffffffff81188f95>] vfs_write+0xc5/0x1f0
> [52608.609506]  [<ffffffff81189482>] SyS_write+0x52/0xa0/0x10
> [52608.609511]  [<ffffffff81628c2e>] ? do_page_fault+0xe/0x10
> [52608.609516]  [<ffffffff8162d402>] system_call_fastpath+0x16/0x1b
> [52608.609517] Code: 1f 44 00 00 0f 1f 44 00 00 55 48 89 e5 48 83 ec 30 4c 89 65 e0 48 89 5d d8 49 89 fc 4c 89 6d e8 4c 89 75 f0 4c 89 7d f8 48 8
> [52608.609559] RIP  [<ffffffff81532a17>] __netpoll_cleanup+0x27/0xe0
> [52608.609563]  RSP <ffff8801ba447de8>
> [52608.609564] CR2: 00000000000003e0
> [52608.609567] ---[ end trace d25ec343349b61d2 ]---
> 
> Signed-off-by: Dan Aloni <alonid@postram.com>
> CC: David S. Miller <davem@davemloft.net>
> CC: Neil Horman <nhorman@tuxdriver.com>
> ---
>  drivers/net/netconsole.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 4822aaf..dcb2134 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -102,6 +102,7 @@ struct netconsole_target {
>  	struct config_item	item;
>  #endif
>  	int			enabled;
> +	struct mutex		mutex;
>  	struct netpoll		np;
>  };
>  
> @@ -181,6 +182,7 @@ static struct netconsole_target *alloc_param_target(char *target_config)
>  	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
>  	nt->np.local_port = 6665;
>  	nt->np.remote_port = 6666;
> +	mutex_init(&nt->mutex);
>  	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
>  
>  	/* Parse parameters and setup netpoll */
> @@ -322,6 +324,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>  		return -EINVAL;
>  	}
>  
> +	mutex_lock(&nt->mutex);
>  	if (enabled) {	/* 1 */
>  
>  		/*
> @@ -331,8 +334,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>  		netpoll_print_options(&nt->np);
>  
>  		err = netpoll_setup(&nt->np);
> -		if (err)
> +		if (err) {
> +			mutex_unlock(&nt->mutex);
>  			return err;
> +		}
>  
>  		printk(KERN_INFO "netconsole: network logging started\n");
>  
> @@ -341,6 +346,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>  	}
>  
>  	nt->enabled = enabled;
> +	mutex_unlock(&nt->mutex);
>  
>  	return strnlen(buf, count);
>  }
> @@ -597,6 +603,7 @@ static struct config_item *make_netconsole_target(struct config_group *group,
>  	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
>  	nt->np.local_port = 6665;
>  	nt->np.remote_port = 6666;
> +	mutex_init(&nt->mutex);
>  	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
>  
>  	/* Initialize the config_item member */
> @@ -682,7 +689,11 @@ restart:
>  				 * we might sleep in __netpoll_cleanup()
>  				 */
>  				spin_unlock_irqrestore(&target_list_lock, flags);
> +
> +				mutex_lock(&nt->mutex);
>  				__netpoll_cleanup(&nt->np);
> +				mutex_unlock(&nt->mutex);
> +
NAK, you can't hold a mutex while calling __netpoll_cleanup.  __netpoll_cleanup
may sleep and its illegal to hold a mutex while doing so.
Neil


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] netconsole: avoid a crash with multiple sysfs writers
  2013-08-08  5:50 ` Neil Horman
@ 2013-08-08  6:14   ` Dan Aloni
  2013-08-09  6:24     ` Neil Horman
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Aloni @ 2013-08-08  6:14 UTC (permalink / raw)
  To: Neil Horman; +Cc: linux-kernel, David S. Miller

On Thu, Aug 8, 2013 at 8:50 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Wed, Aug 07, 2013 at 12:02:44PM +0300, Dan Aloni wrote:
[..]
>
> > When my 'ifup eth' script was fired multiple times and ran concurrent o> @@ -682,7 +689,11 @@ restart:
> >                                * we might sleep in __netpoll_cleanup()
> >                                */
> >                               spin_unlock_irqrestore(&target_list_lock, flags);
> > +
> > +                             mutex_lock(&nt->mutex);
> >                               __netpoll_cleanup(&nt->np);
> > +                             mutex_unlock(&nt->mutex);
> > +
> NAK, you can't hold a mutex while calling __netpoll_cleanup.  __netpoll_cleanup
> may sleep and its illegal to hold a mutex while doing so.
> Neil
>

To my understanding, it mostly depends on locking order, and having
sleeplocks in the outer order and spinlocks in the inner order is
valid as long the locking order is not reversed.

Also, drivers/net/team/team.c - another netpoll user, already does the
same thing I intended in this patch - it locks the outer team->lock
mutex in team_uninit() while calling team_port_del() and then
team_port_disable_netpoll() calls __netpoll_cleanup().

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] netconsole: avoid a crash with multiple sysfs writers
  2013-08-08  6:14   ` Dan Aloni
@ 2013-08-09  6:24     ` Neil Horman
  2013-08-30 10:59       ` [PATCH net-next] " Dan Aloni
  0 siblings, 1 reply; 6+ messages in thread
From: Neil Horman @ 2013-08-09  6:24 UTC (permalink / raw)
  To: Dan Aloni; +Cc: linux-kernel, David S. Miller

On Thu, Aug 08, 2013 at 09:14:31AM +0300, Dan Aloni wrote:
> On Thu, Aug 8, 2013 at 8:50 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Wed, Aug 07, 2013 at 12:02:44PM +0300, Dan Aloni wrote:
> [..]
> >
> > > When my 'ifup eth' script was fired multiple times and ran concurrent o> @@ -682,7 +689,11 @@ restart:
> > >                                * we might sleep in __netpoll_cleanup()
> > >                                */
> > >                               spin_unlock_irqrestore(&target_list_lock, flags);
> > > +
> > > +                             mutex_lock(&nt->mutex);
> > >                               __netpoll_cleanup(&nt->np);
> > > +                             mutex_unlock(&nt->mutex);
> > > +
> > NAK, you can't hold a mutex while calling __netpoll_cleanup.  __netpoll_cleanup
> > may sleep and its illegal to hold a mutex while doing so.
> > Neil
> >
> 
> To my understanding, it mostly depends on locking order, and having
> sleeplocks in the outer order and spinlocks in the inner order is
> valid as long the locking order is not reversed.
> 
> Also, drivers/net/team/team.c - another netpoll user, already does the
> same thing I intended in this patch - it locks the outer team->lock
> mutex in team_uninit() while calling team_port_del() and then
> team_port_disable_netpoll() calls __netpoll_cleanup().
> 
Sorry, you're right, I was under the impression that you were already in atomic
context, but as long as you don't have preempt disabled when you try to take the
mutex in any path, you should be ok.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net-next] netconsole: avoid a crash with multiple sysfs writers
  2013-08-09  6:24     ` Neil Horman
@ 2013-08-30 10:59       ` Dan Aloni
  2013-09-04  2:11         ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Aloni @ 2013-08-30 10:59 UTC (permalink / raw)
  To: davem; +Cc: linux-kernel, netdev, Neil Horman

When my 'ifup eth' script was fired multiple times and ran concurrent on
my laptop, for some obscure /etc scripting reason, it was revealed
that the store_enabled() function in netconsole doesn't handle it nicely,
as recorded by the Oops below (a syslog paste, but not mangled too much
to prevent from discerning the traceback).

On Linux 3.10.4, this patch seeks to remedy the problem, and it has been
running stable on my laptop for a few days.

[52608.609325] BUG: unable to handle kernel NULL pointer dereference at 00000000000003e0
[52608.609331] IP: [<ffffffff81532a17>] __netpoll_cleanup+0x27/0xe0
[52608.609339] PGD 15e51a067 PUD 15433e067 PMD 0
[52608.609343] Oops: 0000 [#1] SMP re firewire_ohci firewire_core crc_itu_t [last unloaded: kvm_intel]
[52608.609347] Modules linked in: kvm_intel tun vfat fat ppdev parport_pc parport fuse ipt_MASQUERADE usb_storage nf_conntrack_netbios_ns nf_conn [..garbled..]
[52608.609433] RAX: 0000000000000000 RBX: ffff880210bbcc68 RCX: 0000000000000000
[52608.609435] RDX: 0000000000000000 RSI: ffff8801ba447da0 RDI: ffff880210bbcc68
[52608.609437] RBP: ffff8801ba447e18 R08: 0000000000000000 R09: 0000000000000001
[52608.609439] R10: 000000000000000a R11: f000000000000000 R12: ffff880210bbcc68
[52608.609441] R13: ffff88020bc41000 R14: 0000000000000002 R15: 000000000000000200000000000
[52608.609443] FS:  00007f38d7bff740(0000) GS:ffff88021dc40000(0000) knlGS:0000000000000000
[52608.609446] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003300000000001427e0
[52608.609448] CR2: 00000000000003e0 CR3: 0000000154103000 CR4: 00000000001427e0
[52608.609450] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[52608.609452] netpoll: netconsole: local port 6665ess 10.0.0.27
[52608.609454] netpoll: netconsole: local IPv4 address 10.0.0.27
[52608.609456] netpoll: netconsole: interface 'em1'
[52608.609457] netpoll: netconsole: remote port 514ress 10.0.0.15
[52608.609459] netpoll: netconsole: remote IPv4 address 10.0.0.15:65:a8:9a:c7
[52608.609461] netpoll: netconsole: remote ethernet address 1c:6f:65:a8:9a:c7
[52608.609463] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[52608.609464] Stack:801ba447e08 ffff880210bbcc68 ffffffffffffffea ffff88020bc41000
[52608.609466]  ffff8801ba447e08 ffff880210bbcc68 ffffffffffffffea ffff88020bc41000
[52608.609471]  0000000000000002 0000000000000002 ffff8801ba447e38 ffffffff81532af4
[52608.609475]  0000000000000000 ffff880210bbcc00 ffff8801ba447e78 ffffffff81420e7c
[52608.609479] Call Trace:
[52608.609484]  [<ffffffff81532af4>] netpoll_cleanup+0x24/0x50
[52608.609489]  [<ffffffff81420e7c>] store_enabled+0x5c/0xe0
[52608.609492]  [<ffffffff81420abe>] netconsole_target_attr_store+0x2e/0x40
[52608.609498]  [<ffffffff811ff2a2>] configfs_write_file+0xd2/0x130
[52608.609503]  [<ffffffff81188f95>] vfs_write+0xc5/0x1f0
[52608.609506]  [<ffffffff81189482>] SyS_write+0x52/0xa0/0x10
[52608.609511]  [<ffffffff81628c2e>] ? do_page_fault+0xe/0x10
[52608.609516]  [<ffffffff8162d402>] system_call_fastpath+0x16/0x1b
[52608.609517] Code: 1f 44 00 00 0f 1f 44 00 00 55 48 89 e5 48 83 ec 30 4c 89 65 e0 48 89 5d d8 49 89 fc 4c 89 6d e8 4c 89 75 f0 4c 89 7d f8 48 8 [..garbled..]
[52608.609559] RIP  [<ffffffff81532a17>] __netpoll_cleanup+0x27/0xe0
[52608.609563]  RSP <ffff8801ba447de8>
[52608.609564] CR2: 00000000000003e0
[52608.609567] ---[ end trace d25ec343349b61d2 ]---

Signed-off-by: Dan Aloni <alonid@postram.com>
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: David S. Miller <davem@davemloft.net>
---
 drivers/net/netconsole.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 4822aaf..dcb2134 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -102,6 +102,7 @@ struct netconsole_target {
 	struct config_item	item;
 #endif
 	int			enabled;
+	struct mutex		mutex;
 	struct netpoll		np;
 };
 
@@ -181,6 +182,7 @@ static struct netconsole_target *alloc_param_target(char *target_config)
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
 	nt->np.local_port = 6665;
 	nt->np.remote_port = 6666;
+	mutex_init(&nt->mutex);
 	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
 
 	/* Parse parameters and setup netpoll */
@@ -322,6 +324,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		return -EINVAL;
 	}
 
+	mutex_lock(&nt->mutex);
 	if (enabled) {	/* 1 */
 
 		/*
@@ -331,8 +334,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		netpoll_print_options(&nt->np);
 
 		err = netpoll_setup(&nt->np);
-		if (err)
+		if (err) {
+			mutex_unlock(&nt->mutex);
 			return err;
+		}
 
 		printk(KERN_INFO "netconsole: network logging started\n");
 
@@ -341,6 +346,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 	}
 
 	nt->enabled = enabled;
+	mutex_unlock(&nt->mutex);
 
 	return strnlen(buf, count);
 }
@@ -597,6 +603,7 @@ static struct config_item *make_netconsole_target(struct config_group *group,
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
 	nt->np.local_port = 6665;
 	nt->np.remote_port = 6666;
+	mutex_init(&nt->mutex);
 	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
 
 	/* Initialize the config_item member */
@@ -682,7 +689,11 @@ restart:
 				 * we might sleep in __netpoll_cleanup()
 				 */
 				spin_unlock_irqrestore(&target_list_lock, flags);
+
+				mutex_lock(&nt->mutex);
 				__netpoll_cleanup(&nt->np);
+				mutex_unlock(&nt->mutex);
+
 				spin_lock_irqsave(&target_list_lock, flags);
 				dev_put(nt->np.dev);
 				nt->np.dev = NULL;
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] netconsole: avoid a crash with multiple sysfs writers
  2013-08-30 10:59       ` [PATCH net-next] " Dan Aloni
@ 2013-09-04  2:11         ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-09-04  2:11 UTC (permalink / raw)
  To: alonid; +Cc: linux-kernel, netdev, nhorman

From: Dan Aloni <alonid@postram.com>
Date: Fri, 30 Aug 2013 13:59:46 +0300

> When my 'ifup eth' script was fired multiple times and ran concurrent on
> my laptop, for some obscure /etc scripting reason, it was revealed
> that the store_enabled() function in netconsole doesn't handle it nicely,
> as recorded by the Oops below (a syslog paste, but not mangled too much
> to prevent from discerning the traceback).
> 
> On Linux 3.10.4, this patch seeks to remedy the problem, and it has been
> running stable on my laptop for a few days.
 ...
> Signed-off-by: Dan Aloni <alonid@postram.com>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

Applied, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-09-04  2:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-07  9:02 [PATCH] netconsole: avoid a crash with multiple sysfs writers Dan Aloni
2013-08-08  5:50 ` Neil Horman
2013-08-08  6:14   ` Dan Aloni
2013-08-09  6:24     ` Neil Horman
2013-08-30 10:59       ` [PATCH net-next] " Dan Aloni
2013-09-04  2:11         ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).