netdev - Commit 05cf0d1bf4 ("net: stmmac: free an skb first when there are no longer any descriptors using it") breaks stmmac?

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: <daa77a04-ff06-bb66-92e1-2b2b3694c695@synopsys.com>
Date:   Mon, 27 Nov 2017 14:41:00 +0000
From:   Jose Abreu <Jose.Abreu@...opsys.com>
To:     Niklas Cassel <niklas.cassel@...s.com>
CC:     Joao Pinto <Joao.Pinto@...opsys.com>,
        linux-netdev <netdev@...r.kernel.org>,
        Giuseppe CAVALLARO <peppe.cavallaro@...com>
Subject: Commit 05cf0d1bf4 ("net: stmmac: free an skb first when there are no
 longer any descriptors using it") breaks stmmac?

Hi Niklas,

I think your commit 05cf0d1bf4 ("net: stmmac: free an skb first
when there are no longer any descriptors using it") is breaking
stmmac driver in multi-queue configuration (this stacktrace may
contain some extra characters as I was using serial port):

------------->8-------------
general protection fault: 0000 [#1] SMP
Modules linked in: stmmac_pci stmmac libphy igb ptp pps_core
x86_pkg_temp_thermal
CPU: 5 PID: 0 Comm: swapper/5 Tainted: G        W       4.14.0-rc5 #2
Hardware name: Default string Default string/SKYBAY, BIOS 5.0.1.1
10/06/2016
task: ffffa2fe14d8b100 task.stack: ffffb8c6000b8000
RIP: 0010:skb_release_data+0x66/0x110
RSP: 0018:ffffa2fe2dd43d98 EFLAGS: 00010206
RAX: 0000000000000030 RBX: ffffa2fe13fab100 RCX: 00000000000005aa
RDX: ffffa2fe12a50000 RSI: 0000000000000000 RDI: fffcfffdfffbfffc
RBP: ffffa2fe2dd43db0 R08: ffffa2fe2dfcd000 R09: 0000000000000001
R10: ffffffffa06245d0 R11: ffffa2fe14c03700 R12: 0000000000000000
R13: ffffa2fe11e686c0 R14: ffffa2fe13fab100 R15: ffffa2fe129b8940
FS:  0000000000000000(0000) GS:ffffa2fe2dd40000(0000)
knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe26c457000 CR3: 000000002b609003 CR4: 00000000003606e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <IRQ>
 skb_release_all+0x1f/0x30
 consume_skb+0x1d/0x40
 __dev_kfree_skb_any+0x2a/0x30
 stmmac_tx_clean+0x230/0x4d0 [stmmac]
 stmmac_poll+0x4b/0x980 [stmmac]
 net_rx_action+0x1ad/0x290
 __do_softirq+0xdd/0x1d6
 irq_exit+0x77/0x80
 do_IRQ+0x4a/0xc0
 common_interrupt+0x93/0x93
 </IRQ>
RIP: 0010:cpuidle_enter_state+0x16a/0x210
RSP: 0018:ffffb8c6000bbe90 EFLAGS: 00000286 ORIG_RAX:
ffffffffffffffae
RAX: ffffa2fe2dd575c0 RBX: ffffa2fe14560200 RCX: 000000000000001f
RDX: 0000000000000000 RSI: 00000122dbd7ae06 RDI: 0000000000000000
RBP: ffffb8c6000bbec0 R08: 0000000000000020 R09: 0000000000000002
R10: ffffb8c6000bbe60 R11: 0000000000123400 R12: 0000004f3c11b47a
R13: 0000000000000003 R14: ffffffffa0e3aa58 R15: 0000000000000003
 cpuidle_enter+0x12/0x20
 call_cpuidle+0x1e/0x40
 do_idle+0x16a/0x1c0
 cpu_startup_entry+0x18/0x20
 start_secondary+0x10d/0x110
 secondary_startup_64+0xa5/0xa5
------------->8-------------

Using tree with your commit I get this stacktrace upon streaming
data at random time (stacktrace does not appear everytime),
without the commit I get no errors.

I understand the reason for your commit but we already have
dirty_tx in stmmac_tx_clean(), shouldn't it be enough to prevent
skb use-after-free? Can you please review your patch to make sure
nothing else is missing?

BTW, this was not *officially* a git bissect, but I only reverted
your commit and it is working fine now.

Best Regards,
Jose Miguel Abreu