Topics

Compass udrc kernel panic

f6bvp
 

To Jonathan Naylor via Groups.Io <naylorjs=yahoo.com@groups.io>

Hi Jonathan,

I contact you through nw-digital-radiogroups for the reason that we are
facing a kernel panic while using ROSE kernel module.
I am glad to see you are still involved in digital communications.
Of course, as you mentionned, it's been a while since you have been
involved in packet radio AX.25 development and peculiarly in the code we
are interested in here.

There is a part of the code causing kernel panic that I identified but I
don't really understand what is its purpose.

Here is the kernel panic context.

-------- Message transféré --------
Sujet : [ROSE] rose dereferenced pointer kernel panic
Date : Sat, 8 Dec 2018 16:17:02 +0100
De : f6bvp <f6bvp@...>
Pour : David Ranch <dranch@...>, Basil Gunn
<@basil860>, Ralf Bächle DL5RB <ralf@...>, Richard
Stearn <richard@...>
Copie à : C Schuman <k4gbb1@...>, linux-hams@...,
Annaliese McDermond <mcdermj@...>, Bernard Pidoux
<bernard.pidoux@...>

Hi All,

While running packet radio network switch ROSE node a kernel panic
occurs systematically when opening a Chromium session.

kernel is 4.14.79-v7+ on a Raspberry Pi with Compass Linux (Debian stretch).

According to Kernel message panic is related to ax25cmp() performed with
a null pointer argument.

The function from which ax25cmp() gets a NULL pointer is rose_route_frame().
Function rose_route_frame() is called by rose_xmit() in the following
code sequence with explicit NULL pointer argument :

if (!rose_route_frame(skb, NULL)) {
dev_kfree_skb(skb);
stats->tx_errors++;
return NETDEV_TX_OK;
}

In order to avoid fatal ax25cmp() comparison we could use the following
rose_route_frame patch that has been already proposed by a number of
observers facing such dereferenced pointer.

diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 452bbb38d943..8f2f1fb1707d 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -863,6 +863,10 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb
*ax25)
int res = 0;
char buf[11];

+ if (ax25 == NULL) {
+ return res;
+ }
+
if (skb->len < ROSE_MIN_LEN)
return res;
frametype = skb->data[2];

I actually patched rose module kernel 4.14.79-v7+ on
my Raspberry Pi and it succeeded in preventing kernel panic.

However, this is not satisfactory as the code sequence calling
rose_route_frame with a NULL argument should certainly have been written
on purpose. My concern is that I don't understand the significance of
this code in case it would not trigger a kernel panic. Is this a bug ?
Instead of NULL argument, did the author want actually to send an empty
value ?

Richard Stearn explained formerly :
https://marc.info/?l=linux-hams&m=146866282201792&w=2
"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."

Francois Romieu in 2016 suggested to ask you what you meant when you
wrote rose_rebuild_header (since that's where Eric B. took rose_xmit
from) back in the 2.1.9 era ?
https://marc.info/?l=linux-netdev&m=145720784703135&w=2

I know this was written long time ago but I wonder if you could give me
some explanations about what the code is supposed to do and how we could
keep it and avoid kernel panic at the same time.

Regards,

Bernard Pidoux, f6bvp

Jonathan Naylor
 

Hi Bernard


To be honest I remember very little about the ROSE implementation. It was over twenty years ago, and I haven't revisited since. I'm pretty sure I handed over maintenance to someone else. As such I can't really help you, I've moved on to other projects which take all my time and I don't feel minded to dig into such old code. Surely it's more of a case that the software calling it should be modified to not do so, or maybe someone else would like to have a look.

I'm sorry to be evasive, but my input is about as useful as someone else's after this amount of time.

Jonathan  G4KLX


On Saturday, 8 December 2018, 23:33:08 GMT, f6bvp <f6bvp@...> wrote:


To Jonathan Naylor via Groups.Io <naylorjs=yahoo.com@groups.io>

Hi Jonathan,

I contact you through nw-digital-radiogroups for the reason that we are
facing a kernel panic while using ROSE kernel module.
I am glad to see you are still involved in digital communications.
Of course, as you mentionned, it's been a while since you have been
involved in packet radio AX.25 development and peculiarly in the code we
are interested in here.

There is a part of the code causing kernel panic that I identified but I
don't really understand what is its purpose.

Here is the kernel panic context.

-------- Message transféré --------
Sujet : [ROSE] rose dereferenced pointer kernel panic
Date : Sat, 8 Dec 2018 16:17:02 +0100
De : f6bvp <f6bvp@...>
Pour : David Ranch <dranch@...>, Basil Gunn
<basil@...>, Ralf Bächle DL5RB <ralf@...>, Richard
Stearn <richard@...>
Copie à : C Schuman <k4gbb1@...>, linux-hams@...,
Annaliese McDermond <mcdermj@...>, Bernard Pidoux
<bernard.pidoux@...>

Hi All,

While running packet radio network switch ROSE node a kernel panic
occurs systematically when opening a Chromium session.

kernel is 4.14.79-v7+ on a Raspberry Pi with Compass Linux (Debian stretch).

According to Kernel message panic is related to ax25cmp() performed with
a null pointer argument.

The function from which ax25cmp() gets a NULL pointer is rose_route_frame().
Function rose_route_frame() is called by rose_xmit() in the following
code sequence with explicit NULL pointer argument :

        if (!rose_route_frame(skb, NULL)) {
                dev_kfree_skb(skb);
                stats->tx_errors++;
                return NETDEV_TX_OK;
        }

In order to avoid fatal ax25cmp() comparison we could use the following
rose_route_frame patch that has been already proposed by a number of
observers facing such dereferenced pointer.

diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 452bbb38d943..8f2f1fb1707d 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -863,6 +863,10 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb
*ax25)
    int res = 0;
    char buf[11];

+    if (ax25 == NULL) {
+        return res;
+    }
+
    if (skb->len < ROSE_MIN_LEN)
        return res;
    frametype = skb->data[2];

I actually patched rose module kernel 4.14.79-v7+ on
my Raspberry Pi and it succeeded in preventing kernel panic.

However, this is not satisfactory as the code sequence calling
rose_route_frame with a NULL argument should certainly have been written
on purpose. My concern is that I don't understand the significance of
this code in case it would not trigger a kernel panic. Is this a bug ?
Instead of NULL argument, did the author want actually to send an empty
value ?

Richard Stearn explained formerly :
https://marc.info/?l=linux-hams&m=146866282201792&w=2
"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."

Francois Romieu in 2016 suggested to ask you what you meant when you
wrote rose_rebuild_header (since that's where Eric B. took rose_xmit
from) back in the 2.1.9 era ?
https://marc.info/?l=linux-netdev&m=145720784703135&w=2

I know this was written long time ago but I wonder if you could give me
some explanations about what the code is supposed to do and how we could
keep it and avoid kernel panic at the same time.

Regards,

Bernard Pidoux, f6bvp