All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [AX25] kernel panic
@ 2009-09-21 20:11 Jarek Poplawski
  2009-09-23 21:17 ` Bernard Pidoux F6BVP
  0 siblings, 1 reply; 20+ messages in thread
From: Jarek Poplawski @ 2009-09-21 20:11 UTC (permalink / raw)
  To: Bernard Pidoux; +Cc: Ralf Baechle DL5RB, Linux Netdev List, linux-hams

<20090910142436.GB10547@linux-mips.org> <4AA9288B.2070205@upmc.fr>
<20090911120557.GA12175@linux-mips.org> <4AB5EAE5.6070605@upmc.fr>
<20090920210242.GA9804@del.dom.local> <4AB73CDE.4030709@upmc.fr>
In-Reply-To: <4AB73CDE.4030709@upmc.fr>

Bernard Pidoux wrote, On 09/21/2009 10:44 AM:

> Hi Jarek,
> 
> Good fishing !
> 
> During the night I catched the following two identical AX25_DBG
> messages with netconsole
> sending already reported message: kernel BUG at kernel/timer.c:913!
> and followed by kernel
> panics and the machine rebooting.
> 
> 
> Sep 21 03:24:06 f6bvp-11 klogd: ------------[ cut here ]------------
> Sep 21 03:24:06 f6bvp-11 klogd: WARNING: at include/net/ax25.h:260
> ax25_kiss_rcv+0x650/0xab0 [ax25]()

Thanks for testing. Alas I don't get how it's possible at this place
(unless I miss the place), especially with a nosmp kernel. So here is
take 2 (to apply after reverting the previous one).

Regards,
Jarek P.
--- (debugging patch, take 2)

 include/net/ax25.h |   36 ++++++++++++++++++++++++++++++++++++
 net/ax25/af_ax25.c |   12 ++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/include/net/ax25.h b/include/net/ax25.h
index 717e219..7fefbb0 100644
--- a/include/net/ax25.h
+++ b/include/net/ax25.h
@@ -252,9 +252,45 @@ typedef struct ax25_cb {
 #define ax25_cb_hold(__ax25) \
 	atomic_inc(&((__ax25)->refcount))
 
+static __inline__ int ax25_timers_warn(ax25_cb *ax25)
+{
+	int err = 0;
+
+	if (del_timer(&ax25->timer)) {
+		WARN_ON_ONCE(1);
+		err = 1;
+	}
+	if (del_timer(&ax25->t1timer)) {
+		WARN_ON_ONCE(1);
+		err += 2;
+	}
+	if (del_timer(&ax25->t2timer)) {
+		WARN_ON_ONCE(1);
+		err += 4;
+	}
+	if (del_timer(&ax25->t3timer)) {
+		WARN_ON_ONCE(1);
+		err += 8;
+	}
+	if (del_timer(&ax25->idletimer)) {
+		WARN_ON_ONCE(1);
+		err += 16;
+	}
+	if (del_timer(&ax25->dtimer)) {
+		WARN_ON_ONCE(1);
+		err += 32;
+	}
+	if (err)
+		printk(KERN_WARNING "AX25_DBG: %d %p %u %s %d\n", err, ax25,
+		       ax25->state, __func__, __LINE__);
+
+	return err;
+}
+
 static __inline__ void ax25_cb_put(ax25_cb *ax25)
 {
 	if (atomic_dec_and_test(&ax25->refcount)) {
+		ax25_timers_warn(ax25);
 		kfree(ax25->digipeat);
 		kfree(ax25);
 	}
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index da0f64f..f1f515c 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -58,6 +58,9 @@ static const struct proto_ops ax25_proto_ops;
 
 static void ax25_free_sock(struct sock *sk)
 {
+	if (ax25_timers_warn(ax25_sk(sk)))
+		printk(KERN_WARNING "AX25_DBG: %p %u %u %u\n", sk,
+		       sk->sk_family, sk->sk_type, sk->sk_protocol);
 	ax25_cb_put(ax25_sk(sk));
 }
 
@@ -222,6 +225,8 @@ ax25_cb *ax25_find_cb(ax25_address *src_addr, ax25_address *dest_addr,
 		if (s->ax25_dev == NULL)
 			continue;
 		if (ax25cmp(&s->source_addr, src_addr) == 0 && ax25cmp(&s->dest_addr, dest_addr) == 0 && s->ax25_dev->dev == dev) {
+			int ref;
+
 			if (digi != NULL && digi->ndigi != 0) {
 				if (s->digipeat == NULL)
 					continue;
@@ -231,6 +236,13 @@ ax25_cb *ax25_find_cb(ax25_address *src_addr, ax25_address *dest_addr,
 				if (s->digipeat != NULL && s->digipeat->ndigi != 0)
 					continue;
 			}
+			ref = atomic_read(&s->refcount);
+			if (ref < 2) {
+				WARN_ON_ONCE(1);
+				printk(KERN_WARNING "AX25_DBG: %d %p %d %s\n",
+				       ref, s, s->state, __func__);
+			}
+
 			ax25_cb_hold(s);
 			spin_unlock_bh(&ax25_list_lock);
 


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

* Re: [AX25] kernel panic
  2009-09-21 20:11 [AX25] kernel panic Jarek Poplawski
@ 2009-09-23 21:17 ` Bernard Pidoux F6BVP
  2009-09-24  8:07   ` Jarek Poplawski
                     ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Bernard Pidoux F6BVP @ 2009-09-23 21:17 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Bernard Pidoux, Ralf Baechle DL5RB, Linux Netdev List, linux-hams

Hi Jarek,

After applying your second patch I had to wait until today before I catched
these kernel messages.
The last three ones where single lines.
There was no kernel panic. 

Sep 23 10:13:53 f6bvp-11 klogd: ------------[ cut here ]------------
Sep 23 10:13:53 f6bvp-11 klogd: WARNING: at net/ax25/af_ax25.c:241 ax25_find_cb+0x170/0x1a0 [ax25]()
Sep 23 10:13:53 f6bvp-11 klogd: Hardware name: MS-7258
Sep 23 10:13:53 f6bvp-11 klogd: Modules linked in: netconsole netrom mkiss rose ax25 nfsd exportfs nfs l
ockd nfs_acl auth_rpcgss sunrpc af_packet ipv6 snd_via82xx snd_ac97_codec ac97_bus snd_mpu401_uart snd_r
awmidi snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_pcm snd_timer
 snd_page_alloc 8139cp snd_mixer_oss 8139too snd i2c_viapro i2c_core soundcore shpchp sr_mod mii pci_hot
plug binfmt_misc ext3 jbd cpufreq_ondemand cpufreq_conservative cpufreq_powersave acpi_cpufreq freq_tabl
e processor floppy sg rtc_cmos evdev button thermal via_agp pata_via ata_generic ide_pci_generic pata_ac
pi sata_via libata sd_mod scsi_mod crc_t10dif
Sep 23 10:13:53 f6bvp-11 klogd: Pid: 5, comm: events/0 Not tainted 2.6.31-nosmp #3
Sep 23 10:13:53 f6bvp-11 klogd: Call Trace:
Sep 23 10:13:53 f6bvp-11 klogd:  <IRQ>  [<ffffffffa03b40d0>] ? ax25_find_cb+0x170/0x1a0 [ax25]
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff81053b90>] warn_slowpath_common+0x80/0xe0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff81053c12>] warn_slowpath_null+0x22/0x40
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffffa03b40d0>] ax25_find_cb+0x170/0x1a0 [ax25]
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffffa03aced5>] ? ax25_listen_mine+0x95/0xb0 [ax25]
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffffa03ad299>] ax25_kiss_rcv+0x199/0xac0 [ax25]
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8132b4bd>] ? sock_def_readable+0x3d/0x80
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8132c96d>] ? sock_queue_rcv_skb+0x12d/0x160
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8133a831>] netif_receive_skb+0x351/0x5f0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff81061699>] ? run_timer_softirq+0x179/0x250
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8133ab50>] process_backlog+0x80/0xe0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8133b4a4>] net_rx_action+0xf4/0x220
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8105b242>] __do_softirq+0xe2/0x1d0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8101414a>] call_softirq+0x1a/0x30
Sep 23 10:13:53 f6bvp-11 klogd:  <EOI>  [<ffffffff810162c5>] do_softirq+0x75/0xc0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8105b094>] local_bh_enable+0xc4/0xd0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffffa03cf798>] mkiss_receive_buf+0x3a8/0x460 [mkiss]
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8104b864>] ? finish_task_switch+0x44/0xe0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff812af800>] flush_to_ldisc+0x110/0x1f0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8106c71c>] ? schedule_delayed_work+0x2c/0x50
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff812af6f0>] ? flush_to_ldisc+0x0/0x1f0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8106c263>] worker_thread+0x173/0x260
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff81071be0>] ? autoremove_wake_function+0x0/0x60
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8106c0f0>] ? worker_thread+0x0/0x260
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8107149e>] kthread+0xae/0xc0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff8101404a>] child_rip+0xa/0x20
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff810713f0>] ? kthread+0x0/0xc0
Sep 23 10:13:53 f6bvp-11 klogd:  [<ffffffff81014040>] ? child_rip+0x0/0x20
Sep 23 10:13:53 f6bvp-11 klogd: ---[ end trace 422da9fe354a7ce3 ]---
Sep 23 10:13:53 f6bvp-11 klogd: AX25_DBG: 1 ffff880003c80000 0 ax25_find_cb
Sep 23 10:13:53 f6bvp-11 last message repeated 2 times

-----

Sep 23 14:26:56 f6bvp-11 klogd: AX25_DBG: 1 ffff88005247b000 0 ax25_find_cb
Sep 23 14:26:56 f6bvp-11 klogd: AX25_DBG: 1 ffff88005247b000 0 ax25_find_cb
Sep 23 14:26:56 f6bvp-11 klogd: AX25_DBG: 1 ffff88005247b000 0 ax25_find_cb

-----

Regards, 

Bernard



Jarek Poplawski a e'crit :
> <20090910142436.GB10547@linux-mips.org> <4AA9288B.2070205@upmc.fr>
> <20090911120557.GA12175@linux-mips.org> <4AB5EAE5.6070605@upmc.fr>
> <20090920210242.GA9804@del.dom.local> <4AB73CDE.4030709@upmc.fr>
> In-Reply-To: <4AB73CDE.4030709@upmc.fr>
> 
> Bernard Pidoux wrote, On 09/21/2009 10:44 AM:
> 
>> Hi Jarek,
>>
>> Good fishing !
>>
>> During the night I catched the following two identical AX25_DBG
>> messages with netconsole
>> sending already reported message: kernel BUG at kernel/timer.c:913!
>> and followed by kernel
>> panics and the machine rebooting.
>>
>>
>> Sep 21 03:24:06 f6bvp-11 klogd: ------------[ cut here ]------------
>> Sep 21 03:24:06 f6bvp-11 klogd: WARNING: at include/net/ax25.h:260
>> ax25_kiss_rcv+0x650/0xab0 [ax25]()
> 
> Thanks for testing. Alas I don't get how it's possible at this place
> (unless I miss the place), especially with a nosmp kernel. So here is
> take 2 (to apply after reverting the previous one).
> 
> Regards,
> Jarek P.
> --- (debugging patch, take 2)
> 
>  include/net/ax25.h |   36 ++++++++++++++++++++++++++++++++++++
>  net/ax25/af_ax25.c |   12 ++++++++++++
>  2 files changed, 48 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/ax25.h b/include/net/ax25.h
> index 717e219..7fefbb0 100644
> --- a/include/net/ax25.h
> +++ b/include/net/ax25.h
> @@ -252,9 +252,45 @@ typedef struct ax25_cb {
>  #define ax25_cb_hold(__ax25) \
>  	atomic_inc(&((__ax25)->refcount))
>  
> +static __inline__ int ax25_timers_warn(ax25_cb *ax25)
> +{
> +	int err = 0;
> +
> +	if (del_timer(&ax25->timer)) {
> +		WARN_ON_ONCE(1);
> +		err = 1;
> +	}
> +	if (del_timer(&ax25->t1timer)) {
> +		WARN_ON_ONCE(1);
> +		err += 2;
> +	}
> +	if (del_timer(&ax25->t2timer)) {
> +		WARN_ON_ONCE(1);
> +		err += 4;
> +	}
> +	if (del_timer(&ax25->t3timer)) {
> +		WARN_ON_ONCE(1);
> +		err += 8;
> +	}
> +	if (del_timer(&ax25->idletimer)) {
> +		WARN_ON_ONCE(1);
> +		err += 16;
> +	}
> +	if (del_timer(&ax25->dtimer)) {
> +		WARN_ON_ONCE(1);
> +		err += 32;
> +	}
> +	if (err)
> +		printk(KERN_WARNING "AX25_DBG: %d %p %u %s %d\n", err, ax25,
> +		       ax25->state, __func__, __LINE__);
> +
> +	return err;
> +}
> +
>  static __inline__ void ax25_cb_put(ax25_cb *ax25)
>  {
>  	if (atomic_dec_and_test(&ax25->refcount)) {
> +		ax25_timers_warn(ax25);
>  		kfree(ax25->digipeat);
>  		kfree(ax25);
>  	}
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index da0f64f..f1f515c 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -58,6 +58,9 @@ static const struct proto_ops ax25_proto_ops;
>  
>  static void ax25_free_sock(struct sock *sk)
>  {
> +	if (ax25_timers_warn(ax25_sk(sk)))
> +		printk(KERN_WARNING "AX25_DBG: %p %u %u %u\n", sk,
> +		       sk->sk_family, sk->sk_type, sk->sk_protocol);
>  	ax25_cb_put(ax25_sk(sk));
>  }
>  
> @@ -222,6 +225,8 @@ ax25_cb *ax25_find_cb(ax25_address *src_addr, ax25_address *dest_addr,
>  		if (s->ax25_dev == NULL)
>  			continue;
>  		if (ax25cmp(&s->source_addr, src_addr) == 0 && ax25cmp(&s->dest_addr, dest_addr) == 0 && s->ax25_dev->dev == dev) {
> +			int ref;
> +
>  			if (digi != NULL && digi->ndigi != 0) {
>  				if (s->digipeat == NULL)
>  					continue;
> @@ -231,6 +236,13 @@ ax25_cb *ax25_find_cb(ax25_address *src_addr, ax25_address *dest_addr,
>  				if (s->digipeat != NULL && s->digipeat->ndigi != 0)
>  					continue;
>  			}
> +			ref = atomic_read(&s->refcount);
> +			if (ref < 2) {
> +				WARN_ON_ONCE(1);
> +				printk(KERN_WARNING "AX25_DBG: %d %p %d %s\n",
> +				       ref, s, s->state, __func__);
> +			}
> +
>  			ax25_cb_hold(s);
>  			spin_unlock_bh(&ax25_list_lock);
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


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

* Re: [AX25] kernel panic
  2009-09-23 21:17 ` Bernard Pidoux F6BVP
@ 2009-09-24  8:07   ` Jarek Poplawski
  2009-09-25 13:10   ` [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl Jarek Poplawski
  2009-09-28  7:12   ` [PATCH] ax25: Add missing dev_put in ax25_setsockopt Jarek Poplawski
  2 siblings, 0 replies; 20+ messages in thread
From: Jarek Poplawski @ 2009-09-24  8:07 UTC (permalink / raw)
  To: Bernard Pidoux F6BVP
  Cc: Bernard Pidoux, Ralf Baechle DL5RB, Linux Netdev List, linux-hams

On Wed, Sep 23, 2009 at 11:17:12PM +0200, Bernard Pidoux F6BVP wrote:
> Hi Jarek,
Hi Bernard,

>
> After applying your second patch I had to wait until today before I catched
> these kernel messages.
> The last three ones where single lines.
> There was no kernel panic. 

Probably you didn't hit the some kind of traffic/problems, because
there was only some additional reporting in this patch vs. take 1.

In the meantime I found some strangeness in refcounting around
ax25_send_frame, and below is a patch which IMHO should fix it. I
don't know if it matters in your case (the previous report suggested
there should be something more), but let's try. Please, don't remove
the previous debugging patch yet, while testing this one.

Thanks,
Jarek P.
--- (ax25_send_frame patch take 1 for testing)

 include/net/netrom.h  |    2 ++
 net/ax25/ax25_out.c   |    6 ++++++
 net/netrom/nr_route.c |   11 ++++++-----
 net/rose/rose_link.c  |    8 ++++++++
 net/rose/rose_route.c |    5 +++++
 5 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/include/net/netrom.h b/include/net/netrom.h
index 15696b1..ab170a6 100644
--- a/include/net/netrom.h
+++ b/include/net/netrom.h
@@ -132,6 +132,8 @@ static __inline__ void nr_node_put(struct nr_node *nr_node)
 static __inline__ void nr_neigh_put(struct nr_neigh *nr_neigh)
 {
 	if (atomic_dec_and_test(&nr_neigh->refcount)) {
+		if (nr_neigh->ax25)
+			ax25_cb_put(nr_neigh->ax25);
 		kfree(nr_neigh->digipeat);
 		kfree(nr_neigh);
 	}
diff --git a/net/ax25/ax25_out.c b/net/ax25/ax25_out.c
index bf706f8..1491260 100644
--- a/net/ax25/ax25_out.c
+++ b/net/ax25/ax25_out.c
@@ -92,6 +92,12 @@ ax25_cb *ax25_send_frame(struct sk_buff *skb, int paclen, ax25_address *src, ax2
 #endif
 	}
 
+	/*
+	 * There is one ref for the state machine; a caller needs
+	 * one more to put it back, just like with the existing one.
+	 */
+	ax25_cb_hold(ax25);
+
 	ax25_cb_add(ax25);
 
 	ax25->state = AX25_STATE_1;
diff --git a/net/netrom/nr_route.c b/net/netrom/nr_route.c
index 4eb1ac9..850ffc0 100644
--- a/net/netrom/nr_route.c
+++ b/net/netrom/nr_route.c
@@ -842,12 +842,13 @@ int nr_route_frame(struct sk_buff *skb, ax25_cb *ax25)
 	dptr  = skb_push(skb, 1);
 	*dptr = AX25_P_NETROM;
 
-	ax25s = ax25_send_frame(skb, 256, (ax25_address *)dev->dev_addr, &nr_neigh->callsign, nr_neigh->digipeat, nr_neigh->dev);
-	if (nr_neigh->ax25 && ax25s) {
-		/* We were already holding this ax25_cb */
+	ax25s = nr_neigh->ax25;
+	nr_neigh->ax25 = ax25_send_frame(skb, 256,
+					 (ax25_address *)dev->dev_addr,
+					 &nr_neigh->callsign,
+					 nr_neigh->digipeat, nr_neigh->dev);
+	if (ax25s)
 		ax25_cb_put(ax25s);
-	}
-	nr_neigh->ax25 = ax25s;
 
 	dev_put(dev);
 	ret = (nr_neigh->ax25 != NULL);
diff --git a/net/rose/rose_link.c b/net/rose/rose_link.c
index bd86a63..5ef5f69 100644
--- a/net/rose/rose_link.c
+++ b/net/rose/rose_link.c
@@ -101,13 +101,17 @@ static void rose_t0timer_expiry(unsigned long param)
 static int rose_send_frame(struct sk_buff *skb, struct rose_neigh *neigh)
 {
 	ax25_address *rose_call;
+	ax25_cb *ax25s;
 
 	if (ax25cmp(&rose_callsign, &null_ax25_address) == 0)
 		rose_call = (ax25_address *)neigh->dev->dev_addr;
 	else
 		rose_call = &rose_callsign;
 
+	ax25s = neigh->ax25;
 	neigh->ax25 = ax25_send_frame(skb, 260, rose_call, &neigh->callsign, neigh->digipeat, neigh->dev);
+	if (ax25s)
+		ax25_cb_put(ax25s);
 
 	return (neigh->ax25 != NULL);
 }
@@ -120,13 +124,17 @@ static int rose_send_frame(struct sk_buff *skb, struct rose_neigh *neigh)
 static int rose_link_up(struct rose_neigh *neigh)
 {
 	ax25_address *rose_call;
+	ax25_cb *ax25s;
 
 	if (ax25cmp(&rose_callsign, &null_ax25_address) == 0)
 		rose_call = (ax25_address *)neigh->dev->dev_addr;
 	else
 		rose_call = &rose_callsign;
 
+	ax25s = neigh->ax25;
 	neigh->ax25 = ax25_find_cb(rose_call, &neigh->callsign, neigh->digipeat, neigh->dev);
+	if (ax25s)
+		ax25_cb_put(ax25s);
 
 	return (neigh->ax25 != NULL);
 }
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 9478d9b..8e5d5ac 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -234,6 +234,8 @@ static void rose_remove_neigh(struct rose_neigh *rose_neigh)
 
 	if ((s = rose_neigh_list) == rose_neigh) {
 		rose_neigh_list = rose_neigh->next;
+		if (rose_neigh->ax25)
+			ax25_cb_put(rose_neigh->ax25);
 		kfree(rose_neigh->digipeat);
 		kfree(rose_neigh);
 		return;
@@ -242,6 +244,8 @@ static void rose_remove_neigh(struct rose_neigh *rose_neigh)
 	while (s != NULL && s->next != NULL) {
 		if (s->next == rose_neigh) {
 			s->next = rose_neigh->next;
+			if (rose_neigh->ax25)
+				ax25_cb_put(rose_neigh->ax25);
 			kfree(rose_neigh->digipeat);
 			kfree(rose_neigh);
 			return;
@@ -814,6 +818,7 @@ void rose_link_failed(ax25_cb *ax25, int reason)
 
 	if (rose_neigh != NULL) {
 		rose_neigh->ax25 = NULL;
+		ax25_cb_put(ax25);
 
 		rose_del_route_by_neigh(rose_neigh);
 		rose_kill_by_neigh(rose_neigh);

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

* [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
  2009-09-23 21:17 ` Bernard Pidoux F6BVP
  2009-09-24  8:07   ` Jarek Poplawski
@ 2009-09-25 13:10   ` Jarek Poplawski
  2009-09-25 13:40     ` Ralf Baechle DL5RB
  2009-09-28  7:12   ` [PATCH] ax25: Add missing dev_put in ax25_setsockopt Jarek Poplawski
  2 siblings, 1 reply; 20+ messages in thread
From: Jarek Poplawski @ 2009-09-25 13:10 UTC (permalink / raw)
  To: David Miller
  Cc: Bernard Pidoux F6BVP, Bernard Pidoux, Ralf Baechle DL5RB,
	Linux Netdev List, linux-hams

This bug isn't responsible for these oopses here, but looks quite
obviously. (I'm not sure if it's easy to test/hit with the common
tools.)

Jarek P.
------------>
[PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl

Use ax25_cb_put after ax25_find_cb in ax25_ctl_ioctl.

Reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---

 net/ax25/af_ax25.c |   27 +++++++++++++++++----------
 1 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index d6b1b05..fbcac76 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -358,6 +358,7 @@ static int ax25_ctl_ioctl(const unsigned int cmd, void __user *arg)
 	ax25_dev *ax25_dev;
 	ax25_cb *ax25;
 	unsigned int k;
+	int ret = 0;
 
 	if (copy_from_user(&ax25_ctl, arg, sizeof(ax25_ctl)))
 		return -EFAULT;
@@ -388,57 +389,63 @@ static int ax25_ctl_ioctl(const unsigned int cmd, void __user *arg)
 	case AX25_WINDOW:
 		if (ax25->modulus == AX25_MODULUS) {
 			if (ax25_ctl.arg < 1 || ax25_ctl.arg > 7)
-				return -EINVAL;
+				goto einval_put;
 		} else {
 			if (ax25_ctl.arg < 1 || ax25_ctl.arg > 63)
-				return -EINVAL;
+				goto einval_put;
 		}
 		ax25->window = ax25_ctl.arg;
 		break;
 
 	case AX25_T1:
 		if (ax25_ctl.arg < 1)
-			return -EINVAL;
+			goto einval_put;
 		ax25->rtt = (ax25_ctl.arg * HZ) / 2;
 		ax25->t1  = ax25_ctl.arg * HZ;
 		break;
 
 	case AX25_T2:
 		if (ax25_ctl.arg < 1)
-			return -EINVAL;
+			goto einval_put;
 		ax25->t2 = ax25_ctl.arg * HZ;
 		break;
 
 	case AX25_N2:
 		if (ax25_ctl.arg < 1 || ax25_ctl.arg > 31)
-			return -EINVAL;
+			goto einval_put;
 		ax25->n2count = 0;
 		ax25->n2 = ax25_ctl.arg;
 		break;
 
 	case AX25_T3:
 		if (ax25_ctl.arg < 0)
-			return -EINVAL;
+			goto einval_put;
 		ax25->t3 = ax25_ctl.arg * HZ;
 		break;
 
 	case AX25_IDLE:
 		if (ax25_ctl.arg < 0)
-			return -EINVAL;
+			goto einval_put;
 		ax25->idle = ax25_ctl.arg * 60 * HZ;
 		break;
 
 	case AX25_PACLEN:
 		if (ax25_ctl.arg < 16 || ax25_ctl.arg > 65535)
-			return -EINVAL;
+			goto einval_put;
 		ax25->paclen = ax25_ctl.arg;
 		break;
 
 	default:
-		return -EINVAL;
+		goto einval_put;
 	  }
 
-	return 0;
+out_put:
+	ax25_cb_put(ax25);
+	return ret;
+
+einval_put:
+	ret = -EINVAL;
+	goto out_put;
 }
 
 static void ax25_fillin_cb_from_dev(ax25_cb *ax25, ax25_dev *ax25_dev)

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

* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
  2009-09-25 13:10   ` [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl Jarek Poplawski
@ 2009-09-25 13:40     ` Ralf Baechle DL5RB
  2009-09-25 18:35       ` Jarek Poplawski
  2009-09-27 20:57       ` [PATCH] ax25: Fix possible oops in ax25_make_new Jarek Poplawski
  0 siblings, 2 replies; 20+ messages in thread
From: Ralf Baechle DL5RB @ 2009-09-25 13:40 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
	Linux Netdev List, linux-hams

On Fri, Sep 25, 2009 at 01:10:38PM +0000, Jarek Poplawski wrote:

> This bug isn't responsible for these oopses here, but looks quite
> obviously. (I'm not sure if it's easy to test/hit with the common
> tools.)

The issue your patch fixes is obvious enough.

> Jarek P.
> ------------>
> [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
> 
> Use ax25_cb_put after ax25_find_cb in ax25_ctl_ioctl.
> 
> Reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

Reviewed-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf

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

* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
  2009-09-25 13:40     ` Ralf Baechle DL5RB
@ 2009-09-25 18:35       ` Jarek Poplawski
  2009-09-25 19:10         ` David Miller
                           ` (2 more replies)
  2009-09-27 20:57       ` [PATCH] ax25: Fix possible oops in ax25_make_new Jarek Poplawski
  1 sibling, 3 replies; 20+ messages in thread
From: Jarek Poplawski @ 2009-09-25 18:35 UTC (permalink / raw)
  To: Ralf Baechle DL5RB
  Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
	Linux Netdev List, linux-hams

On Fri, Sep 25, 2009 at 02:40:52PM +0100, Ralf Baechle DL5RB wrote:
> On Fri, Sep 25, 2009 at 01:10:38PM +0000, Jarek Poplawski wrote:
> 
> > This bug isn't responsible for these oopses here, but looks quite
> > obviously. (I'm not sure if it's easy to test/hit with the common
> > tools.)
> 
> The issue your patch fixes is obvious enough.

Yes, with new code there would be no doubt. But here, if you know it's
worked for some time, you wonder if you're not blind. |-)
> 
> > Jarek P.
> > ------------>
> > [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
> > 
> > Use ax25_cb_put after ax25_find_cb in ax25_ctl_ioctl.
> > 
> > Reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
> > Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
> 
> Reviewed-by: Ralf Baechle <ralf@linux-mips.org>
> 
Thanks for reviewing,
Jarek P.

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

* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
  2009-09-25 18:35       ` Jarek Poplawski
@ 2009-09-25 19:10         ` David Miller
  2009-09-25 19:50         ` Bernard Pidoux
  2009-09-27  7:23         ` Ralf Baechle DL5RB
  2 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2009-09-25 19:10 UTC (permalink / raw)
  To: jarkao2; +Cc: ralf, f6bvp, bernard.pidoux, netdev, linux-hams

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Fri, 25 Sep 2009 20:35:04 +0200

> On Fri, Sep 25, 2009 at 02:40:52PM +0100, Ralf Baechle DL5RB wrote:
>> > [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
>> > 
>> > Use ax25_cb_put after ax25_find_cb in ax25_ctl_ioctl.
>> > 
>> > Reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
>> > Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
>> 
>> Reviewed-by: Ralf Baechle <ralf@linux-mips.org>
>> 
> Thanks for reviewing,

Applied.

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

* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
  2009-09-25 18:35       ` Jarek Poplawski
  2009-09-25 19:10         ` David Miller
@ 2009-09-25 19:50         ` Bernard Pidoux
  2009-09-25 20:29           ` Jarek Poplawski
  2009-09-27  7:23         ` Ralf Baechle DL5RB
  2 siblings, 1 reply; 20+ messages in thread
From: Bernard Pidoux @ 2009-09-25 19:50 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Ralf Baechle DL5RB, David Miller, Bernard Pidoux F6BVP,
	Linux Netdev List, linux-hams

Hi Jarek,

patch applied to 2.6.31.1 kernel keeping your other AX25 debugging patches.

So far, so good !

Thanks.

Bernard Pidoux


Jarek Poplawski a écrit :
> On Fri, Sep 25, 2009 at 02:40:52PM +0100, Ralf Baechle DL5RB wrote:
>> On Fri, Sep 25, 2009 at 01:10:38PM +0000, Jarek Poplawski wrote:
>>
>>> This bug isn't responsible for these oopses here, but looks quite
>>> obviously. (I'm not sure if it's easy to test/hit with the common
>>> tools.)
>> The issue your patch fixes is obvious enough.
> 
> Yes, with new code there would be no doubt. But here, if you know it's
> worked for some time, you wonder if you're not blind. |-)
>>> Jarek P.
>>> ------------>
>>> [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
>>>
>>> Use ax25_cb_put after ax25_find_cb in ax25_ctl_ioctl.
>>>
>>> Reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
>>> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
>> Reviewed-by: Ralf Baechle <ralf@linux-mips.org>
>>
> Thanks for reviewing,
> Jarek P.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-hams" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
  2009-09-25 19:50         ` Bernard Pidoux
@ 2009-09-25 20:29           ` Jarek Poplawski
  0 siblings, 0 replies; 20+ messages in thread
From: Jarek Poplawski @ 2009-09-25 20:29 UTC (permalink / raw)
  To: Bernard Pidoux
  Cc: Ralf Baechle DL5RB, David Miller, Bernard Pidoux F6BVP,
	Linux Netdev List, linux-hams

On Fri, Sep 25, 2009 at 09:50:11PM +0200, Bernard Pidoux wrote:
> Hi Jarek,
> 
> patch applied to 2.6.31.1 kernel keeping your other AX25 debugging patches.

Actually, only one of them is for debugging.
> 
> So far, so good !

Very strange...;-) It seems I might have been wrong and the previous
patch could actually fix those oopses. Then it would point at some
problems while starting netrom connections as the main suspect.

Thanks,
Jarek P.

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

* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
  2009-09-25 18:35       ` Jarek Poplawski
  2009-09-25 19:10         ` David Miller
  2009-09-25 19:50         ` Bernard Pidoux
@ 2009-09-27  7:23         ` Ralf Baechle DL5RB
  2009-09-27 17:10           ` Jarek Poplawski
  2 siblings, 1 reply; 20+ messages in thread
From: Ralf Baechle DL5RB @ 2009-09-27  7:23 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
	Linux Netdev List, linux-hams

On Fri, Sep 25, 2009 at 08:35:04PM +0200, Jarek Poplawski wrote:

> > > This bug isn't responsible for these oopses here, but looks quite
> > > obviously. (I'm not sure if it's easy to test/hit with the common
> > > tools.)
> > 
> > The issue your patch fixes is obvious enough.
> 
> Yes, with new code there would be no doubt. But here, if you know it's
> worked for some time, you wonder if you're not blind. |-)

Most of of the ioctls are used by AX.25 userland which does error
checking on user supplied values so userland will never attempt invalid
ioctl calls.  So no surprise this went unnoticed.

  Ralf

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

* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
  2009-09-27  7:23         ` Ralf Baechle DL5RB
@ 2009-09-27 17:10           ` Jarek Poplawski
  2009-09-27 19:02             ` Jarek Poplawski
  0 siblings, 1 reply; 20+ messages in thread
From: Jarek Poplawski @ 2009-09-27 17:10 UTC (permalink / raw)
  To: Ralf Baechle DL5RB
  Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
	Linux Netdev List, linux-hams

On Sun, Sep 27, 2009 at 08:23:19AM +0100, Ralf Baechle DL5RB wrote:
> On Fri, Sep 25, 2009 at 08:35:04PM +0200, Jarek Poplawski wrote:
> 
> > > > This bug isn't responsible for these oopses here, but looks quite
> > > > obviously. (I'm not sure if it's easy to test/hit with the common
> > > > tools.)
> > > 
> > > The issue your patch fixes is obvious enough.
> > 
> > Yes, with new code there would be no doubt. But here, if you know it's
> > worked for some time, you wonder if you're not blind. |-)
> 
> Most of of the ioctls are used by AX.25 userland which does error
> checking on user supplied values so userland will never attempt invalid
> ioctl calls.  So no surprise this went unnoticed.

In this case valid calls (return 0) were affected too, so I guess the
whole thing is rarely used.

Jarek P.

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

* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
  2009-09-27 17:10           ` Jarek Poplawski
@ 2009-09-27 19:02             ` Jarek Poplawski
  0 siblings, 0 replies; 20+ messages in thread
From: Jarek Poplawski @ 2009-09-27 19:02 UTC (permalink / raw)
  To: Ralf Baechle DL5RB
  Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
	Linux Netdev List, linux-hams

On Sun, Sep 27, 2009 at 07:10:29PM +0200, Jarek Poplawski wrote:
...
> In this case valid calls (return 0) were affected too, so I guess the
> whole thing is rarely used.

And, after all, there would be only a little, invisible memory leak.

Jarek P.

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

* [PATCH] ax25: Fix possible oops in ax25_make_new
  2009-09-25 13:40     ` Ralf Baechle DL5RB
  2009-09-25 18:35       ` Jarek Poplawski
@ 2009-09-27 20:57       ` Jarek Poplawski
  2009-09-28 10:47         ` Bernard Pidoux F6BVP
  2009-09-30 23:44         ` David Miller
  1 sibling, 2 replies; 20+ messages in thread
From: Jarek Poplawski @ 2009-09-27 20:57 UTC (permalink / raw)
  To: Ralf Baechle DL5RB
  Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
	Linux Netdev List, linux-hams

In ax25_make_new, if kmemdup of digipeat returns an error, there would
be an oops in sk_free while calling sk_destruct, because sk_protinfo
is NULL at the moment; move sk->sk_destruct initialization after this.

BTW of reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>

Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---

 net/ax25/af_ax25.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index fbcac76..9884639 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -900,7 +900,6 @@ struct sock *ax25_make_new(struct sock *osk, struct ax25_dev *ax25_dev)
 
 	sock_init_data(NULL, sk);
 
-	sk->sk_destruct = ax25_free_sock;
 	sk->sk_type     = osk->sk_type;
 	sk->sk_priority = osk->sk_priority;
 	sk->sk_protocol = osk->sk_protocol;
@@ -938,6 +937,7 @@ struct sock *ax25_make_new(struct sock *osk, struct ax25_dev *ax25_dev)
 	}
 
 	sk->sk_protinfo = ax25;
+	sk->sk_destruct = ax25_free_sock;
 	ax25->sk    = sk;
 
 	return sk;

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

* [PATCH] ax25: Add missing dev_put in ax25_setsockopt
  2009-09-23 21:17 ` Bernard Pidoux F6BVP
  2009-09-24  8:07   ` Jarek Poplawski
  2009-09-25 13:10   ` [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl Jarek Poplawski
@ 2009-09-28  7:12   ` Jarek Poplawski
  2009-09-28 10:48     ` Bernard Pidoux F6BVP
  2009-09-28 12:53     ` Ralf Baechle
  2 siblings, 2 replies; 20+ messages in thread
From: Jarek Poplawski @ 2009-09-28  7:12 UTC (permalink / raw)
  To: David Miller
  Cc: Bernard Pidoux F6BVP, Bernard Pidoux, Ralf Baechle DL5RB,
	Linux Netdev List, linux-hams

There is no dev_put ending positive SO_BINDTODEVICE call in
ax25_setsockopt and no matching dev_put later. This ref isn't used by
ax25_cb's because it's handled with up and down device events.

BTW of reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>

Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---

 net/ax25/af_ax25.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index fbcac76..3eee8eb 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -663,6 +663,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
 
 		ax25->ax25_dev = ax25_dev_ax25dev(dev);
 		ax25_fillin_cb(ax25, ax25->ax25_dev);
+		dev_put(dev);
 		break;
 
 	default:

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

* Re: [PATCH] ax25: Fix possible oops in ax25_make_new
  2009-09-27 20:57       ` [PATCH] ax25: Fix possible oops in ax25_make_new Jarek Poplawski
@ 2009-09-28 10:47         ` Bernard Pidoux F6BVP
  2009-09-30 23:44         ` David Miller
  1 sibling, 0 replies; 20+ messages in thread
From: Bernard Pidoux F6BVP @ 2009-09-28 10:47 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Ralf Baechle DL5RB, David Miller, Bernard Pidoux,
	Linux Netdev List, linux-hams

Applied.
Thanks.

Bernard

Jarek Poplawski a écrit :
> In ax25_make_new, if kmemdup of digipeat returns an error, there would
> be an oops in sk_free while calling sk_destruct, because sk_protinfo
> is NULL at the moment; move sk->sk_destruct initialization after this.
> 
> BTW of reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
> 
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
> ---
> 
>  net/ax25/af_ax25.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index fbcac76..9884639 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -900,7 +900,6 @@ struct sock *ax25_make_new(struct sock *osk, struct ax25_dev *ax25_dev)
>  
>  	sock_init_data(NULL, sk);
>  
> -	sk->sk_destruct = ax25_free_sock;
>  	sk->sk_type     = osk->sk_type;
>  	sk->sk_priority = osk->sk_priority;
>  	sk->sk_protocol = osk->sk_protocol;
> @@ -938,6 +937,7 @@ struct sock *ax25_make_new(struct sock *osk, struct ax25_dev *ax25_dev)
>  	}
>  
>  	sk->sk_protinfo = ax25;
> +	sk->sk_destruct = ax25_free_sock;
>  	ax25->sk    = sk;
>  
>  	return sk;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-hams" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] ax25: Add missing dev_put in ax25_setsockopt
  2009-09-28  7:12   ` [PATCH] ax25: Add missing dev_put in ax25_setsockopt Jarek Poplawski
@ 2009-09-28 10:48     ` Bernard Pidoux F6BVP
  2009-09-28 12:53     ` Ralf Baechle
  1 sibling, 0 replies; 20+ messages in thread
From: Bernard Pidoux F6BVP @ 2009-09-28 10:48 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: David Miller, Bernard Pidoux, Ralf Baechle DL5RB,
	Linux Netdev List, linux-hams

I had noticed dev_put(dev) was missing but never found where to insert it !
Patch applied.
Thank you.

Bernard


Jarek Poplawski a écrit :
> There is no dev_put ending positive SO_BINDTODEVICE call in
> ax25_setsockopt and no matching dev_put later. This ref isn't used by
> ax25_cb's because it's handled with up and down device events.
> 
> BTW of reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
> 
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
> ---
> 
>  net/ax25/af_ax25.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index fbcac76..3eee8eb 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -663,6 +663,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
>  
>  		ax25->ax25_dev = ax25_dev_ax25dev(dev);
>  		ax25_fillin_cb(ax25, ax25->ax25_dev);
> +		dev_put(dev);
>  		break;
>  
>  	default:
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


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

* [PATCH] ax25: Add missing dev_put in ax25_setsockopt
  2009-09-28  7:12   ` [PATCH] ax25: Add missing dev_put in ax25_setsockopt Jarek Poplawski
  2009-09-28 10:48     ` Bernard Pidoux F6BVP
@ 2009-09-28 12:53     ` Ralf Baechle
  2009-09-28 17:46       ` Bernard Pidoux
  2009-09-28 19:26       ` David Miller
  1 sibling, 2 replies; 20+ messages in thread
From: Ralf Baechle @ 2009-09-28 12:53 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
	Linux Netdev List, linux-hams

ax25_setsockopt SO_BINDTODEVICE is missing a dev_put call in case of
success.  Re-order code to fix this bug.  While at it also reformat two
lines of code to comply with the Linux coding style.

Initial patch by Jarek Poplawski <jarkao2@gmail.com>.

Reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

---
Counter-proposal.  Reordering all the code avoids the need for a 2nd dev_put
and is more readable.

   Ralf

 net/ax25/af_ax25.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index fbcac76..4102de1 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -641,15 +641,10 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
 
 	case SO_BINDTODEVICE:
 		if (optlen > IFNAMSIZ)
-			optlen=IFNAMSIZ;
-		if (copy_from_user(devname, optval, optlen)) {
-		res = -EFAULT;
-			break;
-		}
+			optlen = IFNAMSIZ;
 
-		dev = dev_get_by_name(&init_net, devname);
-		if (dev == NULL) {
-			res = -ENODEV;
+		if (copy_from_user(devname, optval, optlen)) {
+			res = -EFAULT;
 			break;
 		}
 
@@ -657,12 +652,18 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
 		   (sock->state != SS_UNCONNECTED ||
 		    sk->sk_state == TCP_LISTEN)) {
 			res = -EADDRNOTAVAIL;
-			dev_put(dev);
+			break;
+		}
+
+		dev = dev_get_by_name(&init_net, devname);
+		if (!dev) {
+			res = -ENODEV;
 			break;
 		}
 
 		ax25->ax25_dev = ax25_dev_ax25dev(dev);
 		ax25_fillin_cb(ax25, ax25->ax25_dev);
+		dev_put(dev);
 		break;
 
 	default:

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

* Re: [PATCH] ax25: Add missing dev_put in ax25_setsockopt
  2009-09-28 12:53     ` Ralf Baechle
@ 2009-09-28 17:46       ` Bernard Pidoux
  2009-09-28 19:26       ` David Miller
  1 sibling, 0 replies; 20+ messages in thread
From: Bernard Pidoux @ 2009-09-28 17:46 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Jarek Poplawski, David Miller, Bernard Pidoux F6BVP,
	Linux Netdev List, linux-hams

Hi take this one too. Hi.
Applied.

Thanks Ralf.

Bernard

Ralf Baechle a écrit :
> ax25_setsockopt SO_BINDTODEVICE is missing a dev_put call in case of
> success.  Re-order code to fix this bug.  While at it also reformat two
> lines of code to comply with the Linux coding style.
> 
> Initial patch by Jarek Poplawski <jarkao2@gmail.com>.
> 
> Reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
> 
> ---
> Counter-proposal.  Reordering all the code avoids the need for a 2nd dev_put
> and is more readable.
> 
>    Ralf
> 
>  net/ax25/af_ax25.c |   19 ++++++++++---------
>  1 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index fbcac76..4102de1 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -641,15 +641,10 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
>  
>  	case SO_BINDTODEVICE:
>  		if (optlen > IFNAMSIZ)
> -			optlen=IFNAMSIZ;
> -		if (copy_from_user(devname, optval, optlen)) {
> -		res = -EFAULT;
> -			break;
> -		}
> +			optlen = IFNAMSIZ;
>  
> -		dev = dev_get_by_name(&init_net, devname);
> -		if (dev == NULL) {
> -			res = -ENODEV;
> +		if (copy_from_user(devname, optval, optlen)) {
> +			res = -EFAULT;
>  			break;
>  		}
>  
> @@ -657,12 +652,18 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
>  		   (sock->state != SS_UNCONNECTED ||
>  		    sk->sk_state == TCP_LISTEN)) {
>  			res = -EADDRNOTAVAIL;
> -			dev_put(dev);
> +			break;
> +		}
> +
> +		dev = dev_get_by_name(&init_net, devname);
> +		if (!dev) {
> +			res = -ENODEV;
>  			break;
>  		}
>  
>  		ax25->ax25_dev = ax25_dev_ax25dev(dev);
>  		ax25_fillin_cb(ax25, ax25->ax25_dev);
> +		dev_put(dev);
>  		break;
>  
>  	default:
> 


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

* Re: [PATCH] ax25: Add missing dev_put in ax25_setsockopt
  2009-09-28 12:53     ` Ralf Baechle
  2009-09-28 17:46       ` Bernard Pidoux
@ 2009-09-28 19:26       ` David Miller
  1 sibling, 0 replies; 20+ messages in thread
From: David Miller @ 2009-09-28 19:26 UTC (permalink / raw)
  To: ralf; +Cc: jarkao2, f6bvp, bernard.pidoux, netdev, linux-hams

From: Ralf Baechle <ralf@linux-mips.org>
Date: Mon, 28 Sep 2009 13:53:13 +0100

> ax25_setsockopt SO_BINDTODEVICE is missing a dev_put call in case of
> success.  Re-order code to fix this bug.  While at it also reformat two
> lines of code to comply with the Linux coding style.
> 
> Initial patch by Jarek Poplawski <jarkao2@gmail.com>.
> 
> Reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
> 
> ---
> Counter-proposal.  Reordering all the code avoids the need for a 2nd dev_put
> and is more readable.

Looks good to me, applied.

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

* Re: [PATCH] ax25: Fix possible oops in ax25_make_new
  2009-09-27 20:57       ` [PATCH] ax25: Fix possible oops in ax25_make_new Jarek Poplawski
  2009-09-28 10:47         ` Bernard Pidoux F6BVP
@ 2009-09-30 23:44         ` David Miller
  1 sibling, 0 replies; 20+ messages in thread
From: David Miller @ 2009-09-30 23:44 UTC (permalink / raw)
  To: jarkao2; +Cc: ralf, f6bvp, bernard.pidoux, netdev, linux-hams

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Sun, 27 Sep 2009 22:57:02 +0200

> In ax25_make_new, if kmemdup of digipeat returns an error, there would
> be an oops in sk_free while calling sk_destruct, because sk_protinfo
> is NULL at the moment; move sk->sk_destruct initialization after this.
> 
> BTW of reported-by: Bernard Pidoux F6BVP <f6bvp@free.fr>
> 
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

Applied and queued up for -stable, thanks!

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

end of thread, other threads:[~2009-09-30 23:44 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-21 20:11 [AX25] kernel panic Jarek Poplawski
2009-09-23 21:17 ` Bernard Pidoux F6BVP
2009-09-24  8:07   ` Jarek Poplawski
2009-09-25 13:10   ` [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl Jarek Poplawski
2009-09-25 13:40     ` Ralf Baechle DL5RB
2009-09-25 18:35       ` Jarek Poplawski
2009-09-25 19:10         ` David Miller
2009-09-25 19:50         ` Bernard Pidoux
2009-09-25 20:29           ` Jarek Poplawski
2009-09-27  7:23         ` Ralf Baechle DL5RB
2009-09-27 17:10           ` Jarek Poplawski
2009-09-27 19:02             ` Jarek Poplawski
2009-09-27 20:57       ` [PATCH] ax25: Fix possible oops in ax25_make_new Jarek Poplawski
2009-09-28 10:47         ` Bernard Pidoux F6BVP
2009-09-30 23:44         ` David Miller
2009-09-28  7:12   ` [PATCH] ax25: Add missing dev_put in ax25_setsockopt Jarek Poplawski
2009-09-28 10:48     ` Bernard Pidoux F6BVP
2009-09-28 12:53     ` Ralf Baechle
2009-09-28 17:46       ` Bernard Pidoux
2009-09-28 19:26       ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.