Ticket #1020 (closed task: fixed)

Opened 2 years ago

Last modified 10 months ago

Check for NULL pointer dereferencing

Reported by: tobiasoed@hotmail.com Assigned to:
Priority: major Milestone: version 0.9.5
Component: madwifi: other Version:
Keywords: Cc:
Patch is attached: 0 Pending:

Description (Last modified by mrenzmann)

I'm having trouble since I updated from r1616 to r1820. This part of the r1819 commit looks fishy to me, 

--- net80211/ieee80211_wireless.c       (revision 1818)
+++ net80211/ieee80211_wireless.c       (revision 1819)
@@ -3321,12 +3321,15 @@
                        if (!IEEE80211_ADDR_EQ(mlme->im_macaddr, vap->iv_dev->broadcast)) {
                                ni = ieee80211_find_node(&ic->ic_sta,
                                        mlme->im_macaddr);
-                               if (ni == NULL)
+                               if (ni == NULL) {
+                                       ieee80211_free_node(ni);
                                        return -EINVAL;

because the first thing ieee80211_free_node() does is 

	struct ieee80211_node_table *nt = ni->ni_table;

so it's going to deref a null pointer. Maybe the _free functions should be converted like other kernel free function to accept NULL as arg?

Tobias.

Attachments

Change History

11/22/06 18:15:29 changed by tobiasoed@hotmail.com

Oh boy, that didn't work. Here is the diff excerpt again 

--- net80211/ieee80211_wireless.c       (revision 1818)
+++ net80211/ieee80211_wireless.c       (revision 1819)
@@ -3321,12 +3321,15 @@
                        if (!IEEE80211_ADDR_EQ(mlme->im_macaddr, vap->iv_dev->broadcast)) {
                                ni = ieee80211_find_node(&ic->ic_sta,
                                        mlme->im_macaddr);
-                               if (ni == NULL)
+                               if (ni == NULL) {
+                                       ieee80211_free_node(ni);

Tobias

11/22/06 20:14:59 changed by tobiasoed@hotmail.com

Ok, so my troubles come from r1819, with the following partial revert of that commit, things work for me again: 

Index: net80211/ieee80211_node.c
===================================================================
--- net80211/ieee80211_node.c   (revision 1820)
+++ net80211/ieee80211_node.c   (working copy)
@@ -1679,7 +1679,7 @@
        TAILQ_FOREACH(ni, &nt->nt_node, ni_list) {
                if (dev != NULL && ni->ni_vap->iv_dev != dev)
                        continue;  /* skip node not for this vap */
-               if (ni->ni_scangen) {
+               if (ni->ni_scangen != gen) {
                        ni->ni_scangen = gen;
                        (void) ieee80211_ref_node(ni);
                        IEEE80211_NODE_UNLOCK(nt);
Index: net80211/ieee80211_wireless.c
===================================================================
--- net80211/ieee80211_wireless.c       (revision 1820)
+++ net80211/ieee80211_wireless.c       (working copy)
@@ -3321,10 +3321,8 @@
                        if (!IEEE80211_ADDR_EQ(mlme->im_macaddr, vap->iv_dev->broadcast)) {
                                ni = ieee80211_find_node(&ic->ic_sta,
                                        mlme->im_macaddr);
-                               if (ni == NULL) {
-                                       ieee80211_free_node(ni);
+                               if (ni == NULL)
                                        return -EINVAL;
-                               }
                                if (dev == ni->ni_vap->iv_dev)
                                        domlme(mlme, ni);
                                ieee80211_free_node(ni);

Tobias.

11/23/06 10:51:52 changed by kelmo

I reverted r1819, in commit r1821. Please further describe your "troubles" on #969, so there is no more duplication of discussion.

11/24/06 07:00:09 changed by mrenzmann

  • description changed.

There are various places in the source that potentially dereference NULL pointers similar to the one you found here. I once started to work on this, but my attention got caught by other things and that task got lost on my to-do list.

I'd welcome if someone could take over that stuff. My suggestion is to make lines like: 

struct ieee80211_node_table *nt = ni->ni_table;

look like: 

struct ieee80211_node_table *nt;
...
KASSERT(ni != NULL, "null pointer deref!");
...
nt = ni->ni_table;

12/22/06 13:42:46 changed by rozteck@interia.pl

I was having crashes related with this bug in r1860 with 802.11a cards (with 802.11b/g cards the crashes were not occuring). I got: 

[42949481.390000] Internal error: Oops: 17 [#1]
[42949481.390000] Modules linked in: sch_sfq sch_htb ipt_REJECT bridge llc tun iptable_filter iptable_nat ip_nat bonding e100 pcnet32 wlan_scan_ap wlan_scan_sta ath_pci ath_dfs ath_rate_atheros wlan ath_hal hdlc syncppp lapb ixp4xx cryptodev ocf ixp400_eth ixp400
[42949481.390000] CPU: 0
[42949481.390000] PC is at ieee80211_node_saveq_drain+0x58/0x9c [wlan]
[42949481.390000] LR is at node_cleanup+0xe4/0x170 [wlan]
[42949481.390000] pc : [<bf1f82d0>]    lr : [<bf1f16fc>]    Tainted: P     
[42949481.390000] sp : c02d5e38  ip : c02d5e5c  fp : c02d5e58
[42949481.390000] r10: c020d0cc  r9 : 00000000  r8 : c0372000
[42949481.390000] r7 : c3557400  r6 : c35575cc  r5 : c02d4000  r4 : 00000004
[42949481.390000] r3 : c0372000  r2 : 00000004  r1 : 00000000  r0 : c3557400
[42949481.390000] Flags: nzcv  IRQs off  FIQs on  Mode SVC_32  Segment kernel
[42949481.390000] Control: 39FF  Table: 03840000  DAC: 00000017
[42949481.390000] Process softirq-tasklet (pid: 8, stack limit = 0xc02d4250)
[42949481.390000] Stack: (0xc02d5e38 to 0xc02d6000)
[42949481.390000] 5e20:                                                       c3557400 60000093 
[42949481.390000] 5e40: c3557400 c2c70280 c3cf0de0 c02d5e78 c02d5e5c bf1f16fc bf1f8284 c02d4000 
[42949481.390000] 5e60: 60000093 c3557400 c3cfc280 c02d5e98 c02d5e7c bf22b444 bf1f1624 c3557400 
[42949481.390000] 5e80: c3557400 c2c70280 ffc03060 c02d5eac c02d5e9c bf1f17a4 bf22b224 c3cfc280 
[42949481.390000] 5ea0: c02d5ec4 c02d5eb0 bf22bfa4 bf1f1794 c3557400 00000000 c02d5eec c02d5ec8 
[42949481.390000] 5ec0: bf1f19d0 bf22bf7c bf21a870 bf21a87b 00301116 c02d4000 80000013 c3cfd720 
[42949481.390000] 5ee0: c02d5f04 c02d5ef0 bf1f1a74 bf1f18fc c3557400 c3cfc280 c02d5f44 c02d5f08 
[42949481.390000] 5f00: bf22fbbc bf1f19f0 c0059fe4 00000000 00000000 c3750000 c3cfd718 00000002 
[42949481.390000] 5f20: c3cfc280 c3cfd718 00000002 c3cfc000 00000000 c020d0cc c02d5f70 c02d5f48 
[42949481.390000] 5f40: bf232358 bf22f760 6589c780 00000002 c3cfd944 00000000 c020d0c0 00000020 
[42949481.390000] 5f60: 00000000 c02d5f88 c02d5f74 c003c1ec bf2322f8 c02d4000 c020cf20 c02d5f98 
[42949481.390000] 5f80: c02d5f8c c003c238 c003c180 c02d5fc4 c02d5f9c c003c44c c003c20c 00000001 
[42949481.390000] 5fa0: c020d0c0 c02d4000 c02c3f1c c003c33c fffffffc 00000000 c02d5ff4 c02d5fc8 
[42949481.390000] 5fc0: c004c124 c003c348 00000001 ffffffff ffffffff 00000000 00000000 00000000 
[42949481.390000] 5fe0: 00000000 00000000 00000000 c02d5ff8 c0038ae8 c004c040 cc13cc33 cc33cc33 
[42949481.390000] Backtrace: 
[42949481.390000] [<bf1f8278>] (ieee80211_node_saveq_drain+0x0/0x9c [wlan]) from [<bf1f16fc>] (node_cleanup+0xe4/0x170 [wlan])
[42949481.390000]  r8 = C3CF0DE0  r7 = C2C70280  r6 = C3557400  r5 = 60000093
[42949481.390000]  r4 = C3557400 
[42949481.390000] [<bf1f1618>] (node_cleanup+0x0/0x170 [wlan]) from [<bf22b444>] (ath_node_cleanup+0x22c/0x250 [ath_pci])
[42949481.390000]  r7 = C3CFC280  r6 = C3557400  r5 = 60000093  r4 = C02D4000
[42949481.390000] [<bf22b218>] (ath_node_cleanup+0x0/0x250 [ath_pci]) from [<bf1f17a4>] (node_free+0x1c/0x58 [wlan])
[42949481.390000]  r7 = FFC03060  r6 = C2C70280  r5 = C3557400  r4 = C3557400
[42949481.390000] [<bf1f1788>] (node_free+0x0/0x58 [wlan]) from [<bf22bfa4>] (ath_node_free+0x34/0x40 [ath_pci])
[42949481.390000]  r4 = C3CFC280 
[42949481.390000] [<bf22bf70>] (ath_node_free+0x0/0x40 [ath_pci]) from [<bf1f19d0>] (_ieee80211_free_node+0xe0/0xf4 [wlan])
[42949481.390000]  r5 = 00000000  r4 = C3557400 
[42949481.390000] [<bf1f18f0>] (_ieee80211_free_node+0x0/0xf4 [wlan]) from [<bf1f1a74>] (ieee80211_free_node+0x90/0xb0 [wlan])
[42949481.390000]  r6 = C3CFD720  r5 = 80000013  r4 = C02D4000 
[42949481.390000] [<bf1f19e4>] (ieee80211_free_node+0x0/0xb0 [wlan]) from [<bf22fbbc>] (ath_tx_processq+0x468/0x878 [ath_pci])
[42949481.390000]  r5 = C3CFC280  r4 = C3557400 
[42949481.390000] [<bf22f754>] (ath_tx_processq+0x0/0x878 [ath_pci]) from [<bf232358>] (ath_tx_tasklet+0x6c/0x114 [ath_pci])
[42949481.390000] [<bf2322ec>] (ath_tx_tasklet+0x0/0x114 [ath_pci]) from [<c003c1ec>] (__tasklet_action+0x78/0x8c)
[42949481.390000]  r8 = 00000000  r7 = 00000020  r6 = C020D0C0  r5 = 00000000
[42949481.390000]  r4 = C3CFD944 
[42949481.390000] [<c003c174>] (__tasklet_action+0x0/0x8c) from [<c003c238>] (tasklet_action+0x38/0x40)
[42949481.390000]  r5 = C020CF20  r4 = C02D4000 
[42949481.390000] [<c003c200>] (tasklet_action+0x0/0x40) from [<c003c44c>] (ksoftirqd+0x110/0x1bc)
[42949481.390000] [<c003c33c>] (ksoftirqd+0x0/0x1bc) from [<c004c124>] (kthread+0xf0/0x120)
[42949481.390000] [<c004c034>] (kthread+0x0/0x120) from [<c0038ae8>] (do_exit+0x0/0x894)
[42949481.390000]  r8 = 00000000  r7 = 00000000  r6 = 00000000  r5 = 00000000
[42949481.390000]  r4 = 00000000 
[42949481.390000] Code: e1520006 e1a04002 0a000009 e5963008 (e5922000) 
[42949481.390000]  <2>kernel BUG at kernel/exit.c:862!

So I have added if (ni == NULL) return; statements to ieee80211_free_node, _ieee80211_free_node, ath_node_free, ath_node_cleanup and ieee80211_node_saveq_drain which solves the problem.

02/02/07 22:10:31 changed by mentor

  • type changed from defect to task.
  • summary changed from r1818 bug to Check for NULL pointer dereferencing.

02/07/08 04:39:00 changed by mtaylor

  • status changed from new to closed.
  • resolution set to fixed.

Should be fixed in trunk.

(follow-up: ↓ 9 ) 02/07/08 06:13:06 changed by mrenzmann

  • milestone set to version 0.9.5.

(in reply to: ↑ 8 ) 02/07/08 22:20:27 changed by foodoc

Replying to mrenzmann:

one questions, where can I get those "out of tree" ath_dfs, ath_rate_atheros stuff (that's what I'm looking for: 

[42949481.390000] Internal error: Oops: 17 [#1]
[42949481.390000] Modules linked in: [...] ath_dfs ath_rate_atheros [...]

I can not find it in anywhere... But some tickets "link" to them...

Add/Change #1020 (Check for NULL pointer dereferencing)