Bug 252608
| Summary: | [panic] [usb] [if_ure] Kernel fault on ure disconnect | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Ali Abdallah <ali.abdallah> | ||||||||
| Component: | usb | Assignee: | Hans Petter Selasky <hselasky> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Only Me | CC: | bz, emaste, hselasky, oleg.nauman, pi | ||||||||
| Priority: | --- | Keywords: | crash | ||||||||
| Version: | CURRENT | ||||||||||
| Hardware: | amd64 | ||||||||||
| OS: | Any | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Ali Abdallah
2021-01-12 12:20:58 UTC
There is likely a missing drain of the USB transfers. Do you think there is an easy way to avoid the panic? I suspect "sc->sc_txavail" is not properly managed at shutdown and that freed USB transfers are kicked alive again. Adding jmg@ Likely it is just more safe to kick all the transfers when there is a packet for TX and not have some many USB TX transfers. --HPS > Do you think there is an easy way to avoid the panic?
Yes,
Try the attached patch.
Created attachment 221491 [details]
Fix race
(In reply to Hans Petter Selasky from comment #5) Thanks very much for the patch, will give it a try and report back. It crashes as well with the patch from comment 5 --- trap 0xc, rip = 0xffffffff803b28f1, rsp = 0xfffffe00c83604c0, rbp = 0xfffffe00c8360540 --- __mtx_lock_sleep() at __mtx_lock_sleep+0xd1/frame 0xfffffe00c8360540 usbd_do_request_flags() at usbd_do_request_flags+0x740/frame 0xfffffe00c83605d0 usbd_do_request_proc() at usbd_do_request_proc+0x60/frame 0xfffffe00c8360630 ure_miibus_readreg() at ure_miibus_readreg+0x11c/frame 0xfffffe00c83606a0 rgephy_status() at rgephy_status+0x80/frame 0xfffffe00c83606f0 rgephy_service() at rgephy_service+0x417/frame 0xfffffe00c8360750 mii_pollstat() at mii_pollstat+0x46/frame 0xfffffe00c8360780 ure_ifmedia_sts() at ure_ifmedia_sts+0x41/frame 0xfffffe00c83607b0 ifmedia_ioctl() at ifmedia_ioctl+0x16a/frame 0xfffffe00c83607e0 ifhwioctl() at ifhwioctl+0x2af/frame 0xfffffe00c83608e0 ifioctl() at ifioctl+0x2eb/frame 0xfffffe00c8360990 kern_ioctl() at kern_ioctl+0x26d/frame 0xfffffe00c8360a00 sys_ioctl() at sys_ioctl+0xf2/frame 0xfffffe00c8360ac0 Use after free of the if_ure mutex. Ali, This is another issue, and is expected. Can we solve that separately? Do you see any more TX-path panics? --HPS > This is another issue, and is expected. Can we solve that separately? Yes absolutely, thanks. > Do you see any more TX-path panics? Not anymore (Will report here if I get another tx-path panic). @bz: ether_ifdetach() nor if_free() is draining if_ioctl. We should have a solution in place for all kernel drivers! --HPS A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6e5baec33c1032f4fbf713d601a34b4658b918a2 commit 6e5baec33c1032f4fbf713d601a34b4658b918a2 Author: Hans Petter Selasky <hselasky@FreeBSD.org> AuthorDate: 2021-01-12 13:13:14 +0000 Commit: Hans Petter Selasky <hselasky@FreeBSD.org> CommitDate: 2021-01-12 16:57:58 +0000 Fix for use-after-free in if_ure(4) driver. When detaching the if_ure(4) driver, the TX active USB transfer array may point to freed USB transfers. Given that the number of USB transfers is very low, simply start all transfers every time there is a packet to keep safe from use-after-free. PR: 252608 MFC after: 1 week Sponsored by: Mellanox Technologies // NVIDIA Networking sys/dev/usb/net/if_ure.c | 49 ++++----------------------------------------- sys/dev/usb/net/if_urereg.h | 9 --------- 2 files changed, 4 insertions(+), 54 deletions(-) Created attachment 221501 [details]
Patch for second issue - please test
Ali,
Please test this patch!
--HPS
(In reply to Hans Petter Selasky from comment #12) Thanks, really appreciated your quick responses. With the patch from comment 12, it is still panicking with the following trace --- trap 0xc, rip = 0xffffffff80e85515, rsp = 0xfffffe00eaf936b0, rbp = 0xfffffe00eaf936f0 --- rgephy_status() at rgephy_status+0x95/frame 0xfffffe00eaf936f0 rgephy_service() at rgephy_service+0x417/frame 0xfffffe00eaf93750 mii_pollstat() at mii_pollstat+0x46/frame 0xfffffe00eaf93780 ure_ifmedia_sts() at ure_ifmedia_sts+0x44/frame 0xfffffe00eaf937b0 ifmedia_ioctl() at ifmedia_ioctl+0x16a/frame 0xfffffe00eaf937e0 ifhwioctl() at ifhwioctl+0x2af/frame 0xfffffe00eaf938e0 ifioctl() at ifioctl+0x2eb/frame 0xfffffe00eaf93990 kern_ioctl() at kern_ioctl+0x26d/frame 0xfffffe00eaf93a00 sys_ioctl() at sys_ioctl+0xf2/frame 0xfffffe00eaf93ac0 Created attachment 221506 [details]
Patch for second issue - please test
Please test updated patch.
(In reply to Hans Petter Selasky from comment #14) I can confirm that my system is no longer panicking when the dock is unplugged. Thanks!!! @Hans, is there is any change to get the patches committed to stable/13? Thanks! Hi, The issue is still being discussed at: https://reviews.freebsd.org/D28136 It's been a bit slow to proceed. --HPS A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=4eac63af23ddafc2b1dfb2aad2896f4513c37cdd commit 4eac63af23ddafc2b1dfb2aad2896f4513c37cdd Author: Hans Petter Selasky <hselasky@FreeBSD.org> AuthorDate: 2021-01-12 17:51:09 +0000 Commit: Hans Petter Selasky <hselasky@FreeBSD.org> CommitDate: 2021-05-21 12:59:19 +0000 Fix for use-after-free by if_ioctl() calls from user-space in USB drivers by detaching the ifnet before the miibus. PR: 252608 Suggested by: jhb@ MFC after: 1 week Sponsored by: Mellanox Technologies // NVIDIA Networking sys/dev/usb/net/uhso.c | 2 +- sys/dev/usb/net/usb_ethernet.c | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) There are two more commits coming and this issue can be closed! A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=9f98b3ea746f97b61d37f1c11d0e7abb3cd81305 commit 9f98b3ea746f97b61d37f1c11d0e7abb3cd81305 Author: Hans Petter Selasky <hselasky@FreeBSD.org> AuthorDate: 2021-01-12 17:51:09 +0000 Commit: Hans Petter Selasky <hselasky@FreeBSD.org> CommitDate: 2021-06-02 11:25:19 +0000 Fix for use-after-free by if_ioctl() calls from user-space in USB drivers by detaching the ifnet before the miibus. PR: 252608 Suggested by: jhb@ Sponsored by: Mellanox Technologies // NVIDIA Networking (cherry picked from commit 4eac63af23ddafc2b1dfb2aad2896f4513c37cdd) sys/dev/usb/net/uhso.c | 2 +- sys/dev/usb/net/usb_ethernet.c | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) ^Triage: assign to committer that resolved. |
