diff mbox

[1/1] ax25: Fix segfault after sock connection timeout

Message ID 20170114121855.62254455@brox.localnet
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Basil Gunn Jan. 14, 2017, 8:18 p.m. UTC
The ax.25 socket connection timed out & the sock struct has been
previously taken down ie. sock struct is now a NULL pointer. Checking
the sock_flag causes the segfault.  Check if the socket struct pointer
is NULL before checking sock_flag. This segfault is seen in
timed out netrom connections.

Please submit to -stable.

Signed-off-by: Basil Gunn <basil@pacabunga.com>
---

Comments

David Miller Jan. 16, 2017, 7:40 p.m. UTC | #1
From: Basil Gunn <basil@pacabunga.com>
Date: Sat, 14 Jan 2017 12:18:55 -0800

> The ax.25 socket connection timed out & the sock struct has been
> previously taken down ie. sock struct is now a NULL pointer. Checking
> the sock_flag causes the segfault.  Check if the socket struct pointer
> is NULL before checking sock_flag. This segfault is seen in
> timed out netrom connections.
> 
> Please submit to -stable.
> 
> Signed-off-by: Basil Gunn <basil@pacabunga.com>

This is consistent with the ax25->sk NULL check later in this function.

Applied and queued up for -stable, thanks.
Dmitry Vyukov Jan. 2, 2019, 11:52 a.m. UTC | #2
On Wed, Jan 2, 2019 at 12:12 AM Bernard Pidoux <f6bvp@free.fr> wrote:
>
> Hi David,
>
> In my previous message I should have reported the following patch rather than the one I reported.
>
> The reason is that the bug is better explained here :
>
> https://marc.info/?l=linux-hams&m=154478673812818&w=2
>
> and  I hope the new proposed patch is more convenient.
>
>
> Bernard
>
>
> Le 01/01/2019 à 23:39, Bernard Pidoux a écrit :
>
> Hi David,
>
> As you already know I am still looking for the simplest way to configure a kernel rose failure situation when rose_route_frame is called with a NULL pointer.
>
> Could you explain with full details how to have "TCP/IP over AX.25 fully configured" ?
>
> More specifically how can we configure rose device without NOARP ? This is not the case when performing Dmitry Vyukov :
>
> # ip link set dev rose0 address 11:22:33:44:55
> # ip link set dev rose0 up
>
> 73 de Bernard, f6bvp
>
>
> Le 08/12/2018 à 17:23, David Ranch a écrit :
>
> Hello Bernard, Everyone,
>
> Yes, I've seen a similar behavior with another program I have here that broadcasts on all live TCP/IP interfaces when it loads.  That all depends if you have TCP/IP over AX.25 fully configured on your machine.  If you do, this cp,,amd should key up your radio to send out an ARP:
>
>     ping -b -c 1 <broadcast IP on your ROSE or AX.25 interface>
>    --
>    d710: fm KI6ZHD to QST ctl UI pid=CC(IP) len 84
>    IP: len 84 44.4.10.39->44.4.10.127 ihl 20 ttl 64 DF prot ICMP
>    ICMP: type Echo Request id 50814 seq 1
>    P�.\
>    �~.
>    ................ !"#$%&'()*+,-./01234567
>    --
>
> Btw, I've been aware of this ROSE panic issue for some time and I'm pretty sure I forwarded those details on to you but that was many years ago.  Another way to reproduce a ROSE panic is, if I remember correctly, you remove the backing AX.25 interface's connection (say killing kisssattach for ax0) on a ROSE interface that has an IP, that will also panic the kernel every time.
>
> --David
> KI6ZHD

+mailing lists

Hi Bernard,

I've provided a bit more information on what I did here:
https://groups.google.com/d/msg/syzkaller/v-4B3zoBC-4/MVgYoeSQCgAJ

I really did not do anything fancy.

FWIW I had to do the following locally just to prevent rose from
crashing my machine all the time. I don't know if it's the right fix
or not, I just used this as stop-gap.

diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 77e9f85a2c92..218308a3c02c 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -874,6 +874,8 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
             skb->data[ROSE_CALL_REQ_ADDR_LEN_OFF] !=
             ROSE_CALL_REQ_ADDR_LEN_VAL))
                return res;
+       if (ax25 == NULL)
+               return res;
        src_addr  = (rose_address *)(skb->data + ROSE_CALL_REQ_SRC_ADDR_OFF);
        dest_addr = (rose_address *)(skb->data + ROSE_CALL_REQ_DEST_ADDR_OFF);


rose_xmit calls rose_route_frame with ax25==NULL, then
rose_route_frame uses ax25 without any checks.
diff mbox

Patch

diff --git a/net/ax25/ax25_subr.c b/net/ax25/ax25_subr.c
index 4855d18..038b109 100644
--- a/net/ax25/ax25_subr.c
+++ b/net/ax25/ax25_subr.c
@@ -264,7 +264,7 @@  void ax25_disconnect(ax25_cb *ax25, int reason)
 {
 	ax25_clear_queues(ax25);

-	if (!sock_flag(ax25->sk, SOCK_DESTROY))
+	if (!ax25->sk || !sock_flag(ax25->sk, SOCK_DESTROY))
 		ax25_stop_heartbeat(ax25);
 	ax25_stop_t1timer(ax25);
 	ax25_stop_t2timer(ax25);