Re: [Patch] rose_route_frame() NULL pointer dereference kernel panic
François, Thank you for providing information about rose_rebuild_header history. I was not able to find Jonathan Naylor G4KLX email. However as rose_xmit() has been recently changed by Eric Biederman, he may have more precise information about rose_route_frame() NULL argument. Meanwhile, I browsed into rose_route.c release 001 you referenced and found an interesting comment before rose_route_frame() : +/* + * Route a frame to an appropriate AX.25 connection. A NULL ax25_cb + * indicates an internally generated frame. + */ +int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25) Now, I hope that ROSE guru will explain us what means an "internally generated frame" and if, by chance, it means that it is not to be sent via a net device, but rather to be used locally ? Bernard
Re: [Patch] rose_route_frame() NULL pointer dereference kernel panic
f6bvp: > Le 05/03/2016 17:22, David Miller a écrit : [...] > > If that's what he intended he would have implemented the entirety of > > rose_xmit() as "kfree_skb(skb)". But that's obviously not the case. > > > > The author meant the packet to be sent in some way, perhaps using a > > default path or something like that. > > Via a NULL pointer ? > I don't see how it could work. Ask G4KLX what he meant when he wrote rose_rebuild_header (since that's where Eric B. took rose_xmit from) back in the 2.1.9 era ? See https://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/?id=d75df542864496c92ff705d7d072a58b0119a4ff -- Ueimor
Re: [Patch] rose_route_frame() NULL pointer dereference kernel panic
Le 05/03/2016 17:22, David Miller a écrit : > From: f6bvp> Date: Sat, 5 Mar 2016 16:32:42 +0100 > >> I understand I did not explain clearly or completely things. >> >> I agree that each time patched rose_xmit() is calling >> rose_route_frame() it will >> get a 0 return. >> And I think this is what was intended by the author of rose_xmit(). > > If that's what he intended he would have implemented the entirety of > rose_xmit() as "kfree_skb(skb)". But that's obviously not the case. > > The author meant the packet to be sent in some way, perhaps using a > default path or something like that. Via a NULL pointer ? I don't see how it could work. > > So please stop telling me over and over again that this function > is meant to simply drop all packets, it's not true. > I am just making hypothesis and trying to infer some deductions from the behaviour of program when there is no more kernel panic. If there is a situation leading to a kernel panic, I thought code should be changed ? What is the problem replacing a NULL argument by an array of 0 ?
Re: [Patch] rose_route_frame() NULL pointer dereference kernel panic
From: f6bvpDate: Sat, 5 Mar 2016 16:32:42 +0100 > I understand I did not explain clearly or completely things. > > I agree that each time patched rose_xmit() is calling > rose_route_frame() it will > get a 0 return. > And I think this is what was intended by the author of rose_xmit(). If that's what he intended he would have implemented the entirety of rose_xmit() as "kfree_skb(skb)". But that's obviously not the case. The author meant the packet to be sent in some way, perhaps using a default path or something like that. So please stop telling me over and over again that this function is meant to simply drop all packets, it's not true.
Re: [Patch] rose_route_frame() NULL pointer dereference kernel panic
David, I understand I did not explain clearly or completely things. I agree that each time patched rose_xmit() is calling rose_route_frame() it will get a 0 return. And I think this is what was intended by the author of rose_xmit(). He wrote a null argument in order to obtain this result but this situation was never reached until I configurerd a secondary network with the following attributes (lack of route gateway) and thus the bug had not been detected before. /sbin/ifconfig enp4s0:1 44.168.19.22 netmask 255.255.255.240 With original rose_route_frame(), when ax25cmp() was called with a NULL argument it always got a null dereference pointer and a kernel panic occured. I conducted a few trials with printks in rose functions and patched rose_xmit() otherwise kernel panic would have occured. For my setting of rose network, I configured a device axudp to send encapsulated AX.25 frames into UDP frames via ax25ipd daemon. First, I set ax25ipd configuration with a rose neighbour with a local network address. # Route HAMNET # F6BVP-10/11 route f6bvp-10 44.168.19.19 udp 10093 route f6bvp-11 44.168.19.19 udp 10093 b # kernel route looks like: Destination Passerelle Genmask Indic Metric Ref Use Iface 0.0.0.0 192.168.0.254 0.0.0.0 UG10 00 enp4s0 44.168.19.160.0.0.0 255.255.255.240 U 0 00 enp4s0 169.254.0.0 0.0.0.0 255.255.0.0 U 10 00 enp4s0 192.168.0.0 0.0.0.0 255.255.255.0 U 10 00 enp4s0 Those are printks when starting AX.25, and ROSE fpac node application: [ 410.759423] NET: Registered protocol family 3 [ 410.784477] mkiss: AX.25 Multikiss, Hans Albas PE1AYX [ 410.785506] mkiss: ax0: crc mode is auto. [ 410.786135] IPv6: ADDRCONF(NETDEV_CHANGE): ax0: link becomes ready [ 411.011461] ROSE: rose_setup() [ 411.012685] ROSE: rose_setup() [ 411.014506] ROSE: rose_setup() [ 411.016902] ROSE: rose_setup() [ 411.021736] ROSE: rose_setup() [ 411.023884] ROSE: rose_setup() [ 411.026132] ROSE: rose_setup() [ 411.028349] ROSE: rose_setup() [ 411.030648] ROSE: rose_setup() [ 411.032975] ROSE: rose_setup() [ 411.033688] NET: Registered protocol family 11 [ 411.037511] ROSE: rose_set_mac_address() [ 411.037987] ROSE: rose_open() [ 412.041361] ROSE: rose_connect() [ 414.053240] ROSE: rose_connect() [ 414.053599] mkiss: ax0: Trying crc-smack [ 414.058881] mkiss: ax0: Trying crc-flexnet [ 414.086670] ROSE: rose_route_frame() [ 414.152265] ROSE: rose_route_frame() [ 471.414622] ROSE: rose_connect() [ 471.449136] ROSE: rose_route_frame() [ 471.694472] ROSE: rose_route_frame() [ 471.695823] ROSE: rose_recvmsg() Application fpacnode client shows that node is connected to local neighbour and application works normally. Next configuration trial was with ax25ipd.conf configured for and a remote subnet rose neighbour: # Route HAMNET # F6BVP-10/11 route f6cnb-9 44.168.12.18 udp 10092 b route f6cnb-11 44.168.12.20 udp 10092 b # Kernel route table is the same as before, i.e. still without any gateway for 44.0.0.0 route. This time printks show a different scenario when starting rose: 1863.750045] mkiss: AX.25 Multikiss, Hans Albas PE1AYX [ 1863.751165] mkiss: ax0: crc mode is auto. [ 1863.755760] IPv6: ADDRCONF(NETDEV_CHANGE): ax0: link becomes ready [ 1863.792418] ROSE: rose_set_mac_address() [ 1863.795116] ROSE: rose_open() [ 1864.797375] ROSE: rose_connect() [ 1866.809240] ROSE: rose_connect() [ 1866.809662] mkiss: ax0: Trying crc-smack [ 1866.811740] ROSE: rose_connect() [ 1866.811983] ROSE: rose_header() [ 1866.811990] ROSE: rose_xmit() [ 1866.811990] ROSE: rose_route_frame() [ 1866.811992] rose_route : unknown neighbour or device '*' [ 1866.813647] mkiss: ax0: Trying crc-flexnet [ 1866.815623] ROSE: rose_header() [ 1866.817228] ROSE: rose_xmit() [ 1866.818808] ROSE: rose_route_frame() [ 1866.820411] rose_route : unknown neighbour or device '*' [ 1876.832984] ROSE: rose_header() [ 1876.834572] ROSE: rose_xmit() [ 1876.836093] ROSE: rose_route_frame() [ 1876.837614] rose_route : unknown neighbour or device '*' [ 1876.839574] ROSE: rose_header() [ 1876.841099] ROSE: rose_xmit() [ 1876.842586] ROSE: rose_route_frame() [ 1876.844083] rose_route : unknown neighbour or device '*' With the patch ax25cmp() comparison fails and message unknown neighbor or device '*' is correctly displayed for rose_neigh == NULL Of course, because there is no route or gateway toward this subnetwork in kernel route table, neighbour node is not connected and fpacnode application is informed about it. Thus system operator knows there is something wrong in rose network configuration. Next trial was performed after adding a gateway toward remote rose neighbour sub net. /sbin/route add -net 44.0.0.0/8 gw 44.168.19.17 This time starting rose shows no more rose_xmit(): 9871.374021] mkiss: AX.25 Multikiss, Hans Albas PE1AYX [ 9871.375418] mkiss: ax0: crc mode is auto. [ 9871.376747] IPv6:
Re: [Patch] rose_route_frame() NULL pointer dereference kernel panic
From: f6bvpDate: Tue, 1 Mar 2016 21:37:14 +0100 > I built the following patch in order to obtain the same result without > NULL pointer. But it will cause every packet to be dropped because rose_route_frame() won't find a matching neighbour, and therefore return 0 to rose_xmit().
Re: [Patch] rose_route_frame() NULL pointer dereference kernel panic
Hi David, Ralf, David is absolutely right about my unappropriate patch. Although I had searched functions calling rose_route_frame(), I did not notice rose_xmit() was involved. Shame on me ! Then, David precisely located the source of the issue we are facing. When rose_xmit() calls rose_route_frame() with NULL as second argument, there is always a null pointer dereference when rose_route_frame() calls ax25cmp(). Here is the explanation : When rose_route_frame() is called by rose_xmit() with NULL *ax25 argument the following comparison (rose_route.c , line 883) if (ax25cmp(>dest_addr, _neigh->callsign) == 0 && always has a pointer dereference leading to a kernel panic. I noticed, using a few printk, that during rose normal operations rose_xmit() was never called when ax25ipd sends an UDP frame. Otherwise, this bug would have been found earlier. It is only because FPAC application asked for a connection to an address without defined route and gateway that rose_xmit() was activated. I am not sure I understood well the purpose of the NULL second argument. I only guess it was intended to have ax25->dest_addr empty in order to make the comparison with all possible rose_neigh->callsign always false. I built the following patch in order to obtain the same result without NULL pointer. --- a/net/rose/rose_dev.c 2016-02-25 21:01:36.0 +0100 +++ b/net/rose/rose_dev.c 2016-03-01 14:08:29.911389078 +0100 @@ -101,13 +101,16 @@ static netdev_tx_t rose_xmit(struct sk_b { struct net_device_stats *stats = >stats; unsigned int len = skb->len; + struct ax25_cb ax25; + memset(, 0, sizeof(struct ax25_cb)); + if (!netif_running(dev)) { printk(KERN_ERR "ROSE: rose_xmit - called when iface is down\n"); return NETDEV_TX_BUSY; } - if (!rose_route_frame(skb, NULL)) { + if (!rose_route_frame(skb, )) { dev_kfree_skb(skb); stats->tx_errors++; return NETDEV_TX_OK; Could Ralf or David please check if above code syntax is correct. I tested the patch and found rose was working correctly with no more panic nor unwanted effects on rose_route_frame() normal operations. Bernard Pidoux
Re: [Patch] rose_route_frame() NULL pointer dereference kernel panic
From: f6bvpDate: Wed, 24 Feb 2016 17:53:11 +0100 > @@ -863,6 +863,11 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb > *ax25) First of all this patch was corrupted by your email client. > int res = 0; > char buf[11]; > > + if (ax25 == NULL) { > + printk("Null ax25 destination !\n"); > + return res; > + } Second of all, this log message is not appropriate, especially given that rose_xmit() calls rose_rout_frame() with an explicit NULL second argument.
[Patch] rose_route_frame() NULL pointer dereference kernel panic
[Patch] Null pointer in rose_route_frame() Bug appears when setting a second IP to ethernet device without adding a route and a gateway: /sbin/ifconfig enp4s0:1 44.168.19.22 netmask 255.255.255.240 If a route and a gateway are not added for subnet, and if ax25ipd configuration includes a destination address in this subnet, then a comparison of destinations address performed by ax25cmp() called by rose_route_frame() is facing a null pointer and a kernel panic occurs. Attached is the report of kernel panic followed by a report of successful patched function. Bernard == 6,756,516974441,-;NET: Registered protocol family 3 6,757,516978403,-;mkiss: AX.25 Multikiss, Hans Albas PE1AYX 6,758,516979388,-;mkiss: ax0: crc mode is auto. 6,759,516979945,-;IPv6: ADDRCONF(NETDEV_CHANGE): ax0: link becomes ready 6,760,519023446,-;NET: Registered protocol family 11 6,761,522043100,-;mkiss: ax0: Trying crc-smack 6,762,522044882,-;mkiss: ax0: Trying crc-flexnet 1,763,522044973,c;BUG: unable to handle kernel 4,764,522044974,+;NULL pointer dereference 4,765,522044975,+; at 0017 1,766,522044976,c;IP: 4,767,522044986,+; [] ax25cmp+0x19/0x60 [ax25] 4,768,522044987,c;PGD 3cd61067 4,769,522044987,+;PUD 35ac0067 4,770,522044988,+;PMD 0 4,771,522044989,+; 4,772,522044990,c;Oops: [#1] 4,773,522044991,+;SMP 4,774,522044991,+; 4,775,522044994,c;Modules linked in: 4,776,522044995,+; rose 4,777,522044996,+; mkiss 4,778,522044996,+; ax25 4,779,522044997,+; netconsole 4,846,522045047,+; 4,847,522045050,-;CPU: 1 PID: 11873 Comm: ax25ipd Not tainted 4.4.1 #2 4,848,522045051,-;Hardware name: /D975XBX2, BIOS BX97520J.86A.2797.2007.1008.1941 10/08/2007 4,849,522045053,-;task: 880037beb500 ti: 88003432 task.ti: 88003432 4,850,522045055,c;RIP: 0010:[] 4,851,522045058,+; [] ax25cmp+0x19/0x60 [ax25] 4,852,522045059,-;RSP: 0018:880034323938 EFLAGS: 00010246 4,876,522045080,+; 4,877,522045081,-;Call Trace: 4,878,522045088,-; [] rose_route_frame+0x9c/0x670 [rose] 4,879,522045094,-; [] ? __init_waitqueue_head+0x10/0x20 4,971,522045204,+; 0,978,522045215,-;Kernel panic - not syncing: Fatal exception in interrupt 0,979,522045763,-;Kernel Offset: disabled 0,980,522045763,c;Rebooting in 30 seconds.. After patch is applied : 6,767,4251903518,-;NET: Registered protocol family 3 6,768,4251907330,-;mkiss: AX.25 Multikiss, Hans Albas PE1AYX 6,769,4251908399,-;mkiss: ax0: crc mode is auto. 6,770,4251909044,-;IPv6: ADDRCONF(NETDEV_CHANGE): ax0: link becomes ready 6,771,4253957114,-;NET: Registered protocol family 11 6,772,4256972259,-;mkiss: ax0: Trying crc-smack 6,773,4256974292,-;mkiss: ax0: Trying crc-flexnet 4,774,4256974372,-;Null ax25 destination ! 4,775,4256978218,-;Null ax25 destination ! 4,776,4266975133,-;Null ax25 destination ! 4,777,4267007092,-;Null ax25 destination ! 4,778,4287007148,-;Null ax25 destination ! diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c index 0fc76d8..254e528 100644 --- a/net/rose/rose_route.c +++ b/net/rose/rose_route.c @@ -863,6 +863,11 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25) int res = 0; char buf[11]; + if (ax25 == NULL) { + printk("Null ax25 destination !\n"); + return res; + } + if (skb->len < ROSE_MIN_LEN) return res; frametype = skb->data[2]; Signed-off-by: Bernard Pidoux