netdev - [PATCH v2] net-fq: Add WARN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1528415316-6379-1-git-send-email-greearb@candelatech.com>
Date:   Thu,  7 Jun 2018 16:48:36 -0700
From:   greearb@...delatech.com
To:     netdev@...r.kernel.org
Cc:     Ben Greear <greearb@...delatech.com>
Subject: [PATCH v2] net-fq:  Add WARN_ON check for null flow.

From: Ben Greear <greearb@...delatech.com>

While testing an ath10k firmware that often crashed under load,
I was seeing kernel crashes as well.  One of them appeared to
be a dereference of a NULL flow object in fq_tin_dequeue.

I have since fixed the firmware flaw, but I think it would be
worth adding the WARN_ON in case the problem appears again.

BUG: unable to handle kernel NULL pointer dereference at 000000000000003c
IP: ieee80211_tx_dequeue+0xfb/0xb10 [mac80211]
PGD 80000001417fe067 P4D 80000001417fe067 PUD 13db41067 PMD 0
Oops: 0000 [#1] PREEMPT SMP PTI
Modules linked in: nf_conntrack_netlink nf_conntrack nfnetlink nf_defrag_ipv4 libcrc32c vrf 8021q garp mrp stp llc fuse macvlan wanlink(O) pktgen lm78 ]
CPU: 2 PID: 21733 Comm: ip Tainted: G        W  O     4.16.8+ #35
Hardware name: _ _/, BIOS 5.11 08/26/2016
RIP: 0010:ieee80211_tx_dequeue+0xfb/0xb10 [mac80211]
RSP: 0018:ffff880172d03c30 EFLAGS: 00010286
RAX: ffff88013b2c0000 RBX: ffff88013b2c00b8 RCX: 0000000000000898
RDX: 0000000000000001 RSI: ffff88013b2c00d8 RDI: ffff88016ac40820
RBP: ffff88016ac42ba0 R08: 0000000000200000 R09: 0000000000000000
R10: 0000000000100000 R11: 0000001256c89fd8 R12: ffff88013b2c0000
R13: ffff88013b2c00d8 R14: 0000000000000000 R15: ffff88013b2c00d8
FS:  00007f04e3606700(0000) GS:ffff880172d00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000000003c CR3: 000000013b35a005 CR4: 00000000003606e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <IRQ>
 ? update_load_avg+0x607/0x6f0
 ath10k_mac_tx_push_txq+0x6e/0x220 [ath10k_core]
 ath10k_mac_tx_push_pending+0x151/0x1e0 [ath10k_core]
 ath10k_htt_txrx_compl_task+0x113e/0x1940 [ath10k_core]
 ? ath10k_ce_completed_send_next_nolock+0x6f/0x90 [ath10k_pci]
 ? ath10k_ce_completed_send_next+0x31/0x40 [ath10k_pci]
 ? ath10k_pci_htc_tx_cb+0x30/0xc0 [ath10k_pci]
 ? ath10k_bus_pci_write32+0x3c/0xa0 [ath10k_pci]
 ath10k_pci_napi_poll+0x44/0xf0 [ath10k_pci]
 net_rx_action+0x250/0x3b0
 __do_softirq+0xc2/0x2c2
 irq_exit+0x93/0xa0
 do_IRQ+0x45/0xc0
 common_interrupt+0xf/0xf
 </IRQ>

Signed-off-by: Ben Greear <greearb@...delatech.com>
---

* v2:  Use list_first_entry_or_null as suggested.

 include/net/fq_impl.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/net/fq_impl.h b/include/net/fq_impl.h
index be7c0fa..cb911f0 100644
--- a/include/net/fq_impl.h
+++ b/include/net/fq_impl.h
@@ -78,7 +78,10 @@ static struct sk_buff *fq_tin_dequeue(struct fq *fq,
 			return NULL;
 	}
 
-	flow = list_first_entry(head, struct fq_flow, flowchain);
+	flow = list_first_entry_or_null(head, struct fq_flow, flowchain);
+
+	if (WARN_ON_ONCE(!flow))
+		return NULL;
 
 	if (flow->deficit <= 0) {
 		flow->deficit += fq->quantum;
-- 
2.4.11