[PATCH 3/6]NET:AX25:ROSE Traps calls to rose_route_frame with a NULL ax25 callback

[PATCH 3/6]NET:AX25:ROSE Traps calls to rose_route_frame with a NULL ax25 callback

[f6bvp]

Hi Richard,

Thanks for this patch and all others for ROSE code you sent recently.
I already committed a similar one in february 2016.

Subject: [Patch] rose_route_frame() NULL pointer dereference kernel panic
From: f6bvp <f6bvp@xxxxxxx>
Date: Wed, 24 Feb 2016 17:53:11 +0100
Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>, "'f6bvp'" <f6bvp@xxxxxxx>, 
davem@xxxxxxxxxxxxx

You may browse linux servers for author f6bvp and see my last findings 
about null ax25.

However, despite all my efforts, I did no convince referees of the 
patch's legitimity and it had not been accepted.
I hope your work for improving rose code will be successful to the 
benefit of all radioamateur packet community!

73 de Bernard, f6bvp

[BUG] unregistered netdevice: wainting for rose0 to become free. Usage count = xx

[f6bvp]

Hi Richard,

I have just reinvestigated the long lasting rose module 
unregister_netdevice issue with kernel 4.10.0.

Here is the context: when removing rose module

rmmod rose

NET: Unregistered protocol family 11

is followed by a message looping indefinitely with a random xx count 
number :

unregistered_netdevice: waiting for rose0 to become free. Usage count = xx
unregistered_netdevice: waiting for rose0 to become free. Usage count = xx
unregistered_netdevice: waiting for rose0 to become free. Usage count = xx
.....

I tried to apply the patches you sent to linux-netdev on 2016-07-16
[PATCH 1/6]NET:AX25:ROSE Add device use count

First I applied it and find out that it was successfully removing the 
unregister bug.
Then I cut it into 5 parts and unpatched the parts one by one.
Then I applied different patch combinations in order to find out which 
ones were sufficient to cure the unregister issue.
Here is the result : parts 1, 2 and 5 are necessary all together.
Part 3 and 4 of your original patch did not add anything according to 
unregister issue (same results as with 5 parts).
According to the result, I am not sure parts 3 and 4 are absolutely 
necessary here.

When applied, rmmod rose is correctly removing rose module and no error 
message occurs.
Here is your patch reduced to parts 1, 2 and 5.

index 36dbc2d..89745aa 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -687,8 +687,10 @@ static int rose_bind(struct socket *sock, struct 
sockaddr *uaddr, int addr_len)
                 rose->source_call = user->call;
                 ax25_uid_put(user);
         } else {
-               if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE))
+               if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE)) {
+                       dev_put(dev);
                         return -EACCES;
+               }
                 rose->source_call   = *source;
         }

index 36dbc2d..89745aa 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -709,6 +711,7 @@ static int rose_bind(struct socket *sock, struct 
sockaddr *uaddr, int addr_len)
         rose_insert_socket(sk);

         sock_reset_flag(sk, SOCK_ZAPPED);
+       dev_put(dev);

         return 0;
  }

diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index 3444562..ea48cee 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -102,6 +102,7 @@ static void rose_loopback_timer(unsigned long param)
                         if ((dev = rose_dev_get(dest)) != NULL) {
                                 if (rose_rx_call_request(skb, dev, 
rose_loopback_neigh, lci_o) == 0)
                                         kfree_skb(skb);
+                               dev_put(dev);
                         } else {
                                 kfree_skb(skb);
                         }

I encourage you to publish a new commit limited to this set of three 
dev_put(dev) that are obviously missing to allow a correct removal of 
rose module.

Bernard

[BUG] unregistered netdevice: wainting for rose0 to become free. Usage count = xx

[f6bvp]

Hi Richard,

I am using kernel-4.1.21 on an i686 machine for it is running AX.25 
almost without issue present in more advanced kernels.
However the bug "unregistered netdevice: wainting for rose0 to become 
free" is already present in kernel 4.1.21.
I thus applied your patch that adds three dev_put(dev) to this kernel.
The patch was successful and removing rose module is now fine.


[root@f6bvp-6 ax25]# lsmod | grep rose
rose                   53248  0
ax25                   65536  3 rose,mkiss,netrom
[root@f6bvp-6 ax25]# rmmod rose

Bernard

> Hi Richard,
>
> I have just reinvestigated the long lasting rose module
> unregister_netdevice issue with kernel 4.10.0.
>
> Here is the context: when removing rose module
>
> rmmod rose
>
> NET: Unregistered protocol family 11
>
> is followed by a message looping indefinitely with a random xx count
> number :
>
> unregistered_netdevice: waiting for rose0 to become free. Usage count = xx
> unregistered_netdevice: waiting for rose0 to become free. Usage count = xx
> unregistered_netdevice: waiting for rose0 to become free. Usage count = xx
> .....
>
> I tried to apply the patches you sent to linux-netdev on 2016-07-16
> [PATCH 1/6]NET:AX25:ROSE  Add device use count
>
> First I applied it and find out that it was successfully removing the
> unregister bug.
> Then I cut it into 5 parts and unpatched the parts one by one.
> Then I applied different patch combinations in order to find out which
> ones were sufficient to cure the unregister issue.
> Here is the result : parts 1, 2 and 5 are necessary all together.
> Part 3 and 4 of your original patch did not add anything according to
> unregister issue (same results as with 5 parts).
> According to the result, I am not sure parts 3 and 4 are absolutely
> necessary here.
>
> When applied, rmmod rose is correctly removing rose module and no error
> message occurs.
> Here is your patch reduced to parts 1, 2 and 5.
>
> index 36dbc2d..89745aa 100644
> --- a/net/rose/af_rose.c
> +++ b/net/rose/af_rose.c
> @@ -687,8 +687,10 @@ static int rose_bind(struct socket *sock, struct
> sockaddr *uaddr, int addr_len)
>                  rose->source_call = user->call;
>                  ax25_uid_put(user);
>          } else {
> -               if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE))
> +               if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE)) {
> +                       dev_put(dev);
>                          return -EACCES;
> +               }
>                  rose->source_call   = *source;
>          }
>
> index 36dbc2d..89745aa 100644
> --- a/net/rose/af_rose.c
> +++ b/net/rose/af_rose.c
> @@ -709,6 +711,7 @@ static int rose_bind(struct socket *sock, struct
> sockaddr *uaddr, int addr_len)
>          rose_insert_socket(sk);
>
>          sock_reset_flag(sk, SOCK_ZAPPED);
> +       dev_put(dev);
>
>          return 0;
>   }
>
> diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
> index 3444562..ea48cee 100644
> --- a/net/rose/rose_loopback.c
> +++ b/net/rose/rose_loopback.c
> @@ -102,6 +102,7 @@ static void rose_loopback_timer(unsigned long param)
>                          if ((dev = rose_dev_get(dest)) != NULL) {
>                                  if (rose_rx_call_request(skb, dev,
> rose_loopback_neigh, lci_o) == 0)
>                                          kfree_skb(skb);
> +                               dev_put(dev);
>                          } else {
>                                  kfree_skb(skb);
>                          }
>
> I encourage you to publish a new commit limited to this set of three
> dev_put(dev) that are obviously missing to allow a correct removal of
> rose module.
>
> Bernard
>

[PATCH 3/6]NET:AX25:ROSE Traps calls to rose_route_frame with a NULL ax25 callback

[Richard Stearn]

Subject: [PATCH 3/6]NET:AX25:ROSE Traps calls to rose_route_frame with a NULL ax25 callback
Traps calls to rose_route_frame with a NULL ax25 callback to
prevent a kernel crash.

Calling rose_route_frame with a NULL ax25 callback parameter indicates a
locally generated frame.  The existing code does not handle the NULL value
and the kernel hard crashes in an interrupt, resulting in the system stopping
processing.

Signed-off-by: Richard Stearn <richard@rns-stearn.demon.co.uk>
---
 net/rose/rose_route.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 23e0fbd..96ed06c 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -877,6 +877,11 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
 	src_addr  = (rose_address *)(skb->data + ROSE_CALL_REQ_SRC_ADDR_OFF);
 	dest_addr = (rose_address *)(skb->data + ROSE_CALL_REQ_DEST_ADDR_OFF);
 
+	if (ax25 == NULL) {
+		printk(KERN_ERR "rose_route_frame : called with ax25 callback == NULL\n");
+		return res;
+	}
+
 	spin_lock_bh(&rose_neigh_list_lock);
 	spin_lock_bh(&rose_route_list_lock);