May 2024 - Linux-stable-mirror

[PATCH] ACPI: APEI: EINJ: Fix einj_dev release leak

by Dan Williams

The platform driver conversion of EINJ mistakenly used platform_device_del() to unwind platform_device_register_full() at module exit. This leads to a small leak of one 'struct platform_device' instance per module load/unload cycle. Switch to platform_device_unregister() which performs both device_del() and final put_device(). Fixes: 5621fafaac00 ("EINJ: Migrate to a platform driver") Cc: <stable(a)vger.kernel.org> Cc: Jonathan Cameron <Jonathan.Cameron(a)huawei.com> Cc: Ben Cheatham <Benjamin.Cheatham(a)amd.com> Signed-off-by: Dan Williams <dan.j.williams(a)intel.com> --- drivers/acpi/apei/einj-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c index 01faca3a238a..bb9f8475ce59 100644 --- a/drivers/acpi/apei/einj-core.c +++ b/drivers/acpi/apei/einj-core.c @@ -903,7 +903,7 @@ static void __exit einj_exit(void) if (einj_initialized) platform_driver_unregister(&einj_driver); - platform_device_del(einj_dev); + platform_device_unregister(einj_dev); } module_init(einj_init);

1 year, 5 months

3
2
0 0

[git:media_stage/fixes] media: mgb4: Fix double debugfs remove

by Hans Verkuil

This is an automatic generated email to let you know that the following patch were queued: Subject: media: mgb4: Fix double debugfs remove Author: Martin Tůma <martin.tuma(a)digiteqautomotive.com> Date: Tue May 21 18:22:54 2024 +0200 Fixes an error where debugfs_remove_recursive() is called first on a parent directory and then again on a child which causes a kernel panic. Signed-off-by: Martin Tůma <martin.tuma(a)digiteqautomotive.com> Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl> Fixes: 0ab13674a9bd ("media: pci: mgb4: Added Digiteq Automotive MGB4 driver") Cc: <stable(a)vger.kernel.org> [hverkuil: added Fixes/Cc tags] drivers/media/pci/mgb4/mgb4_core.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) --- diff --git a/drivers/media/pci/mgb4/mgb4_core.c b/drivers/media/pci/mgb4/mgb4_core.c index 60498a5abebf..ab4f07e2e560 100644 --- a/drivers/media/pci/mgb4/mgb4_core.c +++ b/drivers/media/pci/mgb4/mgb4_core.c @@ -642,9 +642,6 @@ static void mgb4_remove(struct pci_dev *pdev) struct mgb4_dev *mgbdev = pci_get_drvdata(pdev); int i; -#ifdef CONFIG_DEBUG_FS - debugfs_remove_recursive(mgbdev->debugfs); -#endif #if IS_REACHABLE(CONFIG_HWMON) hwmon_device_unregister(mgbdev->hwmon_dev); #endif @@ -659,6 +656,10 @@ static void mgb4_remove(struct pci_dev *pdev) if (mgbdev->vin[i]) mgb4_vin_free(mgbdev->vin[i]); +#ifdef CONFIG_DEBUG_FS + debugfs_remove_recursive(mgbdev->debugfs); +#endif + device_remove_groups(&mgbdev->pdev->dev, mgb4_pci_groups); free_spi(mgbdev); free_i2c(mgbdev);

1 year, 5 months

1
0
0 0

[PATCH] xhci: Handle TD clearing for multiple streams case

by Hector Martin

When multiple streams are in use, multiple TDs might be in flight when an endpoint is stopped. We need to issue a Set TR Dequeue Pointer for each, to ensure everything is reset properly and the caches cleared. Change the logic so that any N>1 TDs found active for different streams are deferred until after the first one is processed, calling xhci_invalidate_cancelled_tds() again from xhci_handle_cmd_set_deq() to queue another command until we are done with all of them. Also change the error/"should never happen" paths to ensure we at least clear any affected TDs, even if we can't issue a command to clear the hardware cache, and complain loudly with an xhci_warn() if this ever happens. This problem case dates back to commit e9df17eb1408 ("USB: xhci: Correct assumptions about number of rings per endpoint.") early on in the XHCI driver's life, when stream support was first added. At that point, this condition would cause TDs to not be given back at all, causing hanging transfers - but no security bug. It was then identified but not fixed nor made into a warning in commit 674f8438c121 ("xhci: split handling halted endpoints into two steps"), which added a FIXME comment for the problem case (without materially changing the behavior as far as I can tell, though the new logic made the problem more obvious). Then later, in commit 94f339147fc3 ("xhci: Fix failure to give back some cached cancelled URBs."), it was acknowledged again. This commit was unfortunately not reviewed at all, as it was authored by the maintainer directly. Had it been, perhaps a second set of eyes would've noticed that it does not fix the bug, but rather just makes it (much) worse. It turns the "transfers hang" bug into a "random memory corruption" bug, by blindly marking TDs as complete without actually clearing them at all nor moving the dequeue pointer past them, which means they aren't actually complete, and the xHC will try to transfer data to/from them when the endpoint resumes, now to freed memory buffers. This could have been a legitimate oversight, but apparently the commit author was aware of the problem (yet still chose to submit it): It was still mentioned as a FIXME, an xhci_dbg() was added to log the problem condition, and the remaining issue was mentioned in the commit description. The choice of making the log type xhci_dbg() for what is, at this point, a completely unhandled and known broken condition is puzzling and unfortunate, as it guarantees that no actual users would see the log in production, thereby making it nigh undebuggable (indeed, even if you turn on DEBUG, the message doesn't really hint at there being a problem at all). It took me *months* of random xHC crashes to finally find a reliable repro and be able to do a deep dive debug session, which could all have been avoided had this unhandled, broken condition been actually reported with a warning, as it should have been as a bug intentionally left in unfixed (never mind that it shouldn't have been left in at all). > Another fix to solve clearing the caches of all stream rings with > cancelled TDs is needed, but not as urgent. 3 years after that statement and 14 years after the original bug was introduced, I think it's finally time to fix it. And maybe next time let's not leave bugs unfixed (that are actually worse than the original bug), and let's actually get people to review kernel commits please. Fixes xHC crashes and IOMMU faults with UAS devices when handling errors/faults. Easiest repro is to use `hdparm` to mark an early sector (e.g. 1024) on a disk as bad, then `cat /dev/sdX > /dev/null` in a loop. At least in the case of JMicron controllers, the read errors end up having to cancel two TDs (for two queued requests to different streams) and the one that didn't get cleared properly ends up faulting the xHC entirely when it tries to access DMA pages that have since been unmapped, referred to by the stale TDs. This normally happens quickly (after two or three loops). After this fix, I left the `cat` in a loop running overnight and experienced no xHC failures, with all read errors recovered properly. Repro'd and tested on an Apple M1 Mac Mini (dwc3 host). On systems without an IOMMU, this bug would instead silently corrupt freed memory, making this a security bug (even on systems with IOMMUs this could silently corrupt memory belonging to other USB devices on the same controller, so it's still a security bug). Given that the kernel autoprobes partition tables, I'm pretty sure a malicious USB device pretending to be a UAS device and reporting an error with the right timing could deliberately trigger a UAF and write to freed memory, with no user action. Fixes: e9df17eb1408 ("USB: xhci: Correct assumptions about number of rings per endpoint.") Fixes: 94f339147fc3 ("xhci: Fix failure to give back some cached cancelled URBs.") Fixes: 674f8438c121 ("xhci: split handling halted endpoints into two steps") Cc: stable(a)vger.kernel.org Cc: security(a)kernel.org Signed-off-by: Hector Martin <marcan(a)marcan.st> --- drivers/usb/host/xhci-ring.c | 54 +++++++++++++++++++++++++++++++++++--------- drivers/usb/host/xhci.h | 1 + 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 575f0fd9c9f1..9c06502be098 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1034,13 +1034,27 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) break; case TD_DIRTY: /* TD is cached, clear it */ case TD_HALTED: + case TD_CLEARING_CACHE_DEFERRED: + if (cached_td) { + if (cached_td->urb->stream_id != td->urb->stream_id) { + /* Multiple streams case, defer move dq */ + xhci_dbg(xhci, + "Move dq deferred: stream %u URB %p\n", + td->urb->stream_id, td->urb); + td->cancel_status = TD_CLEARING_CACHE_DEFERRED; + break; + } + + /* Should never happen, at least try to clear the TD if it does */ + xhci_warn(xhci, + "Found multiple active URBs %p and %p in stream %u?\n", + td->urb, cached_td->urb, + td->urb->stream_id); + td_to_noop(xhci, ring, cached_td, false); + cached_td->cancel_status = TD_CLEARED; + } + td->cancel_status = TD_CLEARING_CACHE; - if (cached_td) - /* FIXME stream case, several stopped rings */ - xhci_dbg(xhci, - "Move dq past stream %u URB %p instead of stream %u URB %p\n", - td->urb->stream_id, td->urb, - cached_td->urb->stream_id, cached_td->urb); cached_td = td; break; } @@ -1060,10 +1074,16 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) if (err) { /* Failed to move past cached td, just set cached TDs to no-op */ list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) { - if (td->cancel_status != TD_CLEARING_CACHE) + /* + * Deferred TDs need to have the deq pointer set after the above command + * completes, so if that failed we just give up on all of them (and + * complain loudly since this could cause issues due to caching). + */ + if (td->cancel_status != TD_CLEARING_CACHE && + td->cancel_status != TD_CLEARING_CACHE_DEFERRED) continue; - xhci_dbg(xhci, "Failed to clear cancelled cached URB %p, mark clear anyway\n", - td->urb); + xhci_warn(xhci, "Failed to clear cancelled cached URB %p, mark clear anyway\n", + td->urb); td_to_noop(xhci, ring, td, false); td->cancel_status = TD_CLEARED; } @@ -1350,6 +1370,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, struct xhci_ep_ctx *ep_ctx; struct xhci_slot_ctx *slot_ctx; struct xhci_td *td, *tmp_td; + bool deferred = false; ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2])); @@ -1436,6 +1457,8 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, xhci_dbg(ep->xhci, "%s: Giveback cancelled URB %p TD\n", __func__, td->urb); xhci_td_cleanup(ep->xhci, td, ep_ring, td->status); + } else if (td->cancel_status == TD_CLEARING_CACHE_DEFERRED) { + deferred = true; } else { xhci_dbg(ep->xhci, "%s: Keep cancelled URB %p TD as cancel_status is %d\n", __func__, td->urb, td->cancel_status); @@ -1445,8 +1468,17 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, ep->ep_state &= ~SET_DEQ_PENDING; ep->queued_deq_seg = NULL; ep->queued_deq_ptr = NULL; - /* Restart any rings with pending URBs */ - ring_doorbell_for_active_rings(xhci, slot_id, ep_index); + + if (deferred) { + /* We have more streams to clear */ + xhci_dbg(ep->xhci, "%s: Pending TDs to clear, continuing with invalidation\n", + __func__); + xhci_invalidate_cancelled_tds(ep); + } else { + /* Restart any rings with pending URBs */ + xhci_dbg(ep->xhci, "%s: All TDs cleared, ring doorbell\n", __func__); + ring_doorbell_for_active_rings(xhci, slot_id, ep_index); + } } static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id, diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 6f4bf98a6282..aa4379bdb90c 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1276,6 +1276,7 @@ enum xhci_cancelled_td_status { TD_DIRTY = 0, TD_HALTED, TD_CLEARING_CACHE, + TD_CLEARING_CACHE_DEFERRED, TD_CLEARED, }; --- base-commit: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6 change-id: 20240524-xhci-streams-124e88db52e6 Best regards, -- Hector Martin <marcan(a)marcan.st>

1 year, 5 months

3
2
0 0

[PATCH v3] af_packet: Handle outgoing VLAN packets without hardware offloading

by Chengen Du

The issue initially stems from libpcap [1]. In the outbound packet path, if hardware VLAN offloading is unavailable, the VLAN tag is inserted into the payload but then cleared from the sk_buff struct. Consequently, this can lead to a false negative when checking for the presence of a VLAN tag, causing the packet sniffing outcome to lack VLAN tag information (i.e., TCI-TPID). As a result, the packet capturing tool may be unable to parse packets as expected. The TCI-TPID is missing because the prb_fill_vlan_info() function does not modify the tp_vlan_tci/tp_vlan_tpid values, as the information is in the payload and not in the sk_buff struct. The skb_vlan_tag_present() function only checks vlan_all in the sk_buff struct. In cooked mode, the L2 header is stripped, preventing the packet capturing tool from determining the correct TCI-TPID value. Additionally, the protocol in SLL is incorrect, which means the packet capturing tool cannot parse the L3 header correctly. [1] https://github.com/the-tcpdump-group/libpcap/issues/1105 Fixes: f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer implementation.") Cc: stable(a)vger.kernel.org Signed-off-by: Chengen Du <chengen.du(a)canonical.com> --- net/packet/af_packet.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index ea3ebc160e25..82b36e90d73b 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1011,6 +1011,10 @@ static void prb_fill_vlan_info(struct tpacket_kbdq_core *pkc, ppd->hv1.tp_vlan_tci = skb_vlan_tag_get(pkc->skb); ppd->hv1.tp_vlan_tpid = ntohs(pkc->skb->vlan_proto); ppd->tp_status = TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; + } else if (eth_type_vlan(pkc->skb->protocol)) { + ppd->hv1.tp_vlan_tci = ntohs(vlan_eth_hdr(pkc->skb)->h_vlan_TCI); + ppd->hv1.tp_vlan_tpid = ntohs(pkc->skb->protocol); + ppd->tp_status = TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; } else { ppd->hv1.tp_vlan_tci = 0; ppd->hv1.tp_vlan_tpid = 0; @@ -2428,6 +2432,10 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, h.h2->tp_vlan_tci = skb_vlan_tag_get(skb); h.h2->tp_vlan_tpid = ntohs(skb->vlan_proto); status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; + } else if (eth_type_vlan(skb->protocol)) { + h.h2->tp_vlan_tci = ntohs(vlan_eth_hdr(skb)->h_vlan_TCI); + h.h2->tp_vlan_tpid = ntohs(skb->protocol); + status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; } else { h.h2->tp_vlan_tci = 0; h.h2->tp_vlan_tpid = 0; @@ -2457,7 +2465,8 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, sll->sll_halen = dev_parse_header(skb, sll->sll_addr); sll->sll_family = AF_PACKET; sll->sll_hatype = dev->type; - sll->sll_protocol = skb->protocol; + sll->sll_protocol = (skb->protocol == htons(ETH_P_8021Q)) ? + vlan_eth_hdr(skb)->h_vlan_encapsulated_proto : skb->protocol; sll->sll_pkttype = skb->pkt_type; if (unlikely(packet_sock_flag(po, PACKET_SOCK_ORIGDEV))) sll->sll_ifindex = orig_dev->ifindex; @@ -3482,7 +3491,8 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, /* Original length was stored in sockaddr_ll fields */ origlen = PACKET_SKB_CB(skb)->sa.origlen; sll->sll_family = AF_PACKET; - sll->sll_protocol = skb->protocol; + sll->sll_protocol = (skb->protocol == htons(ETH_P_8021Q)) ? + vlan_eth_hdr(skb)->h_vlan_encapsulated_proto : skb->protocol; } sock_recv_cmsgs(msg, sk, skb); @@ -3539,6 +3549,10 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, aux.tp_vlan_tci = skb_vlan_tag_get(skb); aux.tp_vlan_tpid = ntohs(skb->vlan_proto); aux.tp_status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; + } else if (eth_type_vlan(skb->protocol)) { + aux.tp_vlan_tci = ntohs(vlan_eth_hdr(skb)->h_vlan_TCI); + aux.tp_vlan_tpid = ntohs(skb->protocol); + aux.tp_status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; } else { aux.tp_vlan_tci = 0; aux.tp_vlan_tpid = 0; -- 2.40.1

1 year, 5 months

1
0
0 0

[PATCH v2] af_packet: Handle outgoing VLAN packets without hardware offloading

by Chengen Du

The issue initially stems from libpcap [1]. In the outbound packet path, if hardware VLAN offloading is unavailable, the VLAN tag is inserted into the payload but then cleared from the sk_buff struct. Consequently, this can lead to a false negative when checking for the presence of a VLAN tag, causing the packet sniffing outcome to lack VLAN tag information (i.e., TCI-TPID). As a result, the packet capturing tool may be unable to parse packets as expected. The TCI-TPID is missing because the prb_fill_vlan_info() function does not modify the tp_vlan_tci/tp_vlan_tpid values, as the information is in the payload and not in the sk_buff struct. The skb_vlan_tag_present() function only checks vlan_all in the sk_buff struct. In cooked mode, the L2 header is stripped, preventing the packet capturing tool from determining the correct TCI-TPID value. Additionally, the protocol in SLL is incorrect, which means the packet capturing tool cannot parse the L3 header correctly. [1] https://github.com/the-tcpdump-group/libpcap/issues/1105 Fixes: f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer implementation.") Signed-off-by: Chengen Du <chengen.du(a)canonical.com> --- net/packet/af_packet.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index ea3ebc160e25..82b36e90d73b 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1011,6 +1011,10 @@ static void prb_fill_vlan_info(struct tpacket_kbdq_core *pkc, ppd->hv1.tp_vlan_tci = skb_vlan_tag_get(pkc->skb); ppd->hv1.tp_vlan_tpid = ntohs(pkc->skb->vlan_proto); ppd->tp_status = TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; + } else if (eth_type_vlan(pkc->skb->protocol)) { + ppd->hv1.tp_vlan_tci = ntohs(vlan_eth_hdr(pkc->skb)->h_vlan_TCI); + ppd->hv1.tp_vlan_tpid = ntohs(pkc->skb->protocol); + ppd->tp_status = TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; } else { ppd->hv1.tp_vlan_tci = 0; ppd->hv1.tp_vlan_tpid = 0; @@ -2428,6 +2432,10 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, h.h2->tp_vlan_tci = skb_vlan_tag_get(skb); h.h2->tp_vlan_tpid = ntohs(skb->vlan_proto); status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; + } else if (eth_type_vlan(skb->protocol)) { + h.h2->tp_vlan_tci = ntohs(vlan_eth_hdr(skb)->h_vlan_TCI); + h.h2->tp_vlan_tpid = ntohs(skb->protocol); + status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; } else { h.h2->tp_vlan_tci = 0; h.h2->tp_vlan_tpid = 0; @@ -2457,7 +2465,8 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, sll->sll_halen = dev_parse_header(skb, sll->sll_addr); sll->sll_family = AF_PACKET; sll->sll_hatype = dev->type; - sll->sll_protocol = skb->protocol; + sll->sll_protocol = (skb->protocol == htons(ETH_P_8021Q)) ? + vlan_eth_hdr(skb)->h_vlan_encapsulated_proto : skb->protocol; sll->sll_pkttype = skb->pkt_type; if (unlikely(packet_sock_flag(po, PACKET_SOCK_ORIGDEV))) sll->sll_ifindex = orig_dev->ifindex; @@ -3482,7 +3491,8 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, /* Original length was stored in sockaddr_ll fields */ origlen = PACKET_SKB_CB(skb)->sa.origlen; sll->sll_family = AF_PACKET; - sll->sll_protocol = skb->protocol; + sll->sll_protocol = (skb->protocol == htons(ETH_P_8021Q)) ? + vlan_eth_hdr(skb)->h_vlan_encapsulated_proto : skb->protocol; } sock_recv_cmsgs(msg, sk, skb); @@ -3539,6 +3549,10 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, aux.tp_vlan_tci = skb_vlan_tag_get(skb); aux.tp_vlan_tpid = ntohs(skb->vlan_proto); aux.tp_status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; + } else if (eth_type_vlan(skb->protocol)) { + aux.tp_vlan_tci = ntohs(vlan_eth_hdr(skb)->h_vlan_TCI); + aux.tp_vlan_tpid = ntohs(skb->protocol); + aux.tp_status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; } else { aux.tp_vlan_tci = 0; aux.tp_vlan_tpid = 0; -- 2.40.1

1 year, 5 months

2
2
0 0

RE: [PATCH] Bluetooth: btnxpuart: Shutdown timer and prevent rearming when driver unloading

by Bough Chen

> -----Original Message----- > From: Luke Wang <ziniu.wang_1(a)nxp.com> > Sent: 2024年5月17日 19:16 > To: marcel(a)holtmann.org; luiz.dentz(a)gmail.com > Cc: linux-bluetooth(a)vger.kernel.org; linux-kernel(a)vger.kernel.org; Amitkumar > Karwar <amitkumar.karwar(a)nxp.com>; Rohit Fule <rohit.fule(a)nxp.com>; > Neeraj Sanjay Kale <neeraj.sanjaykale(a)nxp.com>; Sherry Sun > <sherry.sun(a)nxp.com>; Bough Chen <haibo.chen(a)nxp.com>; Jun Li > <jun.li(a)nxp.com>; Guillaume Legoupil <guillaume.legoupil(a)nxp.com>; Salim > Chebbo <salim.chebbo(a)nxp.com>; imx(a)lists.linux.dev > Subject: [PATCH] Bluetooth: btnxpuart: Shutdown timer and prevent rearming > when driver unloading > > From: Luke Wang <ziniu.wang_1(a)nxp.com> > > When unload the btnxpuart driver, its associated timer will be deleted. > If the timer happens to be modified at this moment, it leads to the kernel call > this timer even after the driver unloaded, resulting in kernel panic. > Use timer_shutdown_sync() instead of del_timer_sync() to prevent rearming. > > panic log: > Internal error: Oops: 0000000086000007 [#1] PREEMPT SMP > Modules linked in: algif_hash algif_skcipher af_alg moal(O) mlan(O) > crct10dif_ce polyval_ce polyval_generic snd_soc_imx_card > snd_soc_fsl_asoc_card snd_soc_imx_audmux mxc_jpeg_encdec v4l2_jpeg > snd_soc_wm8962 snd_soc_fsl_micfil snd_soc_fsl_sai flexcan snd_soc_fsl_utils > ap130x rpmsg_ctrl imx_pcm_dma can_dev rpmsg_char pwm_fan fuse [last > unloaded: btnxpuart] > CPU: 5 PID: 723 Comm: memtester Tainted: G O > 6.6.23-lts-next-06207-g4aef2658ac28 #1 > Hardware name: NXP i.MX95 19X19 board (DT) > pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : 0xffff80007a2cf464 > lr : call_timer_fn.isra.0+0x24/0x80 > ... > Call trace: > 0xffff80007a2cf464 > __run_timers+0x234/0x280 > run_timer_softirq+0x20/0x40 > __do_softirq+0x100/0x26c > ____do_softirq+0x10/0x1c > call_on_irq_stack+0x24/0x4c > do_softirq_own_stack+0x1c/0x2c > irq_exit_rcu+0xc0/0xdc > el0_interrupt+0x54/0xd8 > __el0_irq_handler_common+0x18/0x24 > el0t_64_irq_handler+0x10/0x1c > el0t_64_irq+0x190/0x194 > Code: ???????? ???????? ???????? ???????? (????????) > ---[ end trace 0000000000000000 ]--- > Kernel panic - not syncing: Oops: Fatal exception in interrupt > SMP: stopping secondary CPUs > Kernel Offset: disabled > CPU features: 0x0,c0000000,40028143,1000721b > Memory Limit: none > ---[ end Kernel panic - not syncing: Oops: Fatal exception in interrupt ]--- Hi all, This patch is already accepted, but I notice it lack fix tag, can anyone help add the following fix tag? Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP Bluetooth chipsets") Cc: stable(a)vger.kernel.org This patch should also put into stable tree. I add the stable tree mail list here. Best Regards Haibo Chen > > Signed-off-by: Luke Wang <ziniu.wang_1(a)nxp.com> > --- > drivers/bluetooth/btnxpuart.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c index > 7f88b6f52f26..77f974a5251b 100644 > --- a/drivers/bluetooth/btnxpuart.c > +++ b/drivers/bluetooth/btnxpuart.c > @@ -328,7 +328,7 @@ static void ps_cancel_timer(struct btnxpuart_dev > *nxpdev) > struct ps_data *psdata = &nxpdev->psdata; > > flush_work(&psdata->work); > - del_timer_sync(&psdata->ps_timer); > + timer_shutdown_sync(&psdata->ps_timer); > } > > static void ps_control(struct hci_dev *hdev, u8 ps_state) > -- > 2.34.1

1 year, 5 months

1
0
0 0

Re: Patch "platform/x86: xiaomi-wmi: Fix race condition when reporting key events" has been added to the 5.4-stable tree

by Armin Wolf

Am 26.05.24 um 22:19 schrieb Sasha Levin: > This is a note to let you know that I've just added the patch titled > > platform/x86: xiaomi-wmi: Fix race condition when reporting key events > > to the 5.4-stable tree which can be found at: > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… > > The filename of the patch is: > platform-x86-xiaomi-wmi-fix-race-condition-when-repo.patch > and it can be found in the queue-5.4 subdirectory. > > If you, or anyone else, feels it should not be added to the stable tree, > please let <stable(a)vger.kernel.org> know about it. > Hi, the underlying race condition can only be triggered since commit e2ffcda16290 ("ACPI: OSL: Allow Notify () handlers to run on all CPUs"), which afaik was introduced with kernel 6.8. Because of this, i do not think that we have to backport this commit to kernels before 6.8. Thanks, Armin Wolf > > commit 7217162b48f60edc29afbeff641b7de02076bb86 > Author: Armin Wolf <W_Armin(a)gmx.de> > Date: Tue Apr 2 16:30:57 2024 +0200 > > platform/x86: xiaomi-wmi: Fix race condition when reporting key events > > [ Upstream commit 290680c2da8061e410bcaec4b21584ed951479af ] > > Multiple WMI events can be received concurrently, so multiple instances > of xiaomi_wmi_notify() can be active at the same time. Since the input > device is shared between those handlers, the key input sequence can be > disturbed. > > Fix this by protecting the key input sequence with a mutex. > > Compile-tested only. > > Fixes: edb73f4f0247 ("platform/x86: wmi: add Xiaomi WMI key driver") > Signed-off-by: Armin Wolf <W_Armin(a)gmx.de> > Link: https://lore.kernel.org/r/20240402143059.8456-2-W_Armin@gmx.de > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com> > Reviewed-by: Hans de Goede <hdegoede(a)redhat.com> > Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> > Signed-off-by: Sasha Levin <sashal(a)kernel.org> > > diff --git a/drivers/platform/x86/xiaomi-wmi.c b/drivers/platform/x86/xiaomi-wmi.c > index 54a2546bb93bf..be80f0bda9484 100644 > --- a/drivers/platform/x86/xiaomi-wmi.c > +++ b/drivers/platform/x86/xiaomi-wmi.c > @@ -2,8 +2,10 @@ > /* WMI driver for Xiaomi Laptops */ > > #include <linux/acpi.h> > +#include <linux/device.h> > #include <linux/input.h> > #include <linux/module.h> > +#include <linux/mutex.h> > #include <linux/wmi.h> > > #include <uapi/linux/input-event-codes.h> > @@ -20,12 +22,21 @@ > > struct xiaomi_wmi { > struct input_dev *input_dev; > + struct mutex key_lock; /* Protects the key event sequence */ > unsigned int key_code; > }; > > +static void xiaomi_mutex_destroy(void *data) > +{ > + struct mutex *lock = data; > + > + mutex_destroy(lock); > +} > + > static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) > { > struct xiaomi_wmi *data; > + int ret; > > if (wdev == NULL || context == NULL) > return -EINVAL; > @@ -35,6 +46,11 @@ static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) > return -ENOMEM; > dev_set_drvdata(&wdev->dev, data); > > + mutex_init(&data->key_lock); > + ret = devm_add_action_or_reset(&wdev->dev, xiaomi_mutex_destroy, &data->key_lock); > + if (ret < 0) > + return ret; > + > data->input_dev = devm_input_allocate_device(&wdev->dev); > if (data->input_dev == NULL) > return -ENOMEM; > @@ -59,10 +75,12 @@ static void xiaomi_wmi_notify(struct wmi_device *wdev, union acpi_object *dummy) > if (data == NULL) > return; > > + mutex_lock(&data->key_lock); > input_report_key(data->input_dev, data->key_code, 1); > input_sync(data->input_dev); > input_report_key(data->input_dev, data->key_code, 0); > input_sync(data->input_dev); > + mutex_unlock(&data->key_lock); > } > > static const struct wmi_device_id xiaomi_wmi_id_table[] = {

1 year, 5 months

1
0
0 0

Re: Patch "platform/x86: xiaomi-wmi: Fix race condition when reporting key events" has been added to the 5.10-stable tree

by Armin Wolf

Am 26.05.24 um 22:14 schrieb Sasha Levin: > This is a note to let you know that I've just added the patch titled > > platform/x86: xiaomi-wmi: Fix race condition when reporting key events > > to the 5.10-stable tree which can be found at: > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… > > The filename of the patch is: > platform-x86-xiaomi-wmi-fix-race-condition-when-repo.patch > and it can be found in the queue-5.10 subdirectory. > > If you, or anyone else, feels it should not be added to the stable tree, > please let <stable(a)vger.kernel.org> know about it. > Hi, the underlying race condition can only be triggered since commit e2ffcda16290 ("ACPI: OSL: Allow Notify () handlers to run on all CPUs"), which afaik was introduced with kernel 6.8. Because of this, i do not think that we have to backport this commit to kernels before 6.8. Thanks, Armin Wolf > > commit 6f4e7901c3ed3c0bd3da7af5854dbb765fad2e00 > Author: Armin Wolf <W_Armin(a)gmx.de> > Date: Tue Apr 2 16:30:57 2024 +0200 > > platform/x86: xiaomi-wmi: Fix race condition when reporting key events > > [ Upstream commit 290680c2da8061e410bcaec4b21584ed951479af ] > > Multiple WMI events can be received concurrently, so multiple instances > of xiaomi_wmi_notify() can be active at the same time. Since the input > device is shared between those handlers, the key input sequence can be > disturbed. > > Fix this by protecting the key input sequence with a mutex. > > Compile-tested only. > > Fixes: edb73f4f0247 ("platform/x86: wmi: add Xiaomi WMI key driver") > Signed-off-by: Armin Wolf <W_Armin(a)gmx.de> > Link: https://lore.kernel.org/r/20240402143059.8456-2-W_Armin@gmx.de > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com> > Reviewed-by: Hans de Goede <hdegoede(a)redhat.com> > Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> > Signed-off-by: Sasha Levin <sashal(a)kernel.org> > > diff --git a/drivers/platform/x86/xiaomi-wmi.c b/drivers/platform/x86/xiaomi-wmi.c > index 54a2546bb93bf..be80f0bda9484 100644 > --- a/drivers/platform/x86/xiaomi-wmi.c > +++ b/drivers/platform/x86/xiaomi-wmi.c > @@ -2,8 +2,10 @@ > /* WMI driver for Xiaomi Laptops */ > > #include <linux/acpi.h> > +#include <linux/device.h> > #include <linux/input.h> > #include <linux/module.h> > +#include <linux/mutex.h> > #include <linux/wmi.h> > > #include <uapi/linux/input-event-codes.h> > @@ -20,12 +22,21 @@ > > struct xiaomi_wmi { > struct input_dev *input_dev; > + struct mutex key_lock; /* Protects the key event sequence */ > unsigned int key_code; > }; > > +static void xiaomi_mutex_destroy(void *data) > +{ > + struct mutex *lock = data; > + > + mutex_destroy(lock); > +} > + > static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) > { > struct xiaomi_wmi *data; > + int ret; > > if (wdev == NULL || context == NULL) > return -EINVAL; > @@ -35,6 +46,11 @@ static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) > return -ENOMEM; > dev_set_drvdata(&wdev->dev, data); > > + mutex_init(&data->key_lock); > + ret = devm_add_action_or_reset(&wdev->dev, xiaomi_mutex_destroy, &data->key_lock); > + if (ret < 0) > + return ret; > + > data->input_dev = devm_input_allocate_device(&wdev->dev); > if (data->input_dev == NULL) > return -ENOMEM; > @@ -59,10 +75,12 @@ static void xiaomi_wmi_notify(struct wmi_device *wdev, union acpi_object *dummy) > if (data == NULL) > return; > > + mutex_lock(&data->key_lock); > input_report_key(data->input_dev, data->key_code, 1); > input_sync(data->input_dev); > input_report_key(data->input_dev, data->key_code, 0); > input_sync(data->input_dev); > + mutex_unlock(&data->key_lock); > } > > static const struct wmi_device_id xiaomi_wmi_id_table[] = {

1 year, 5 months

1
0
0 0

Re: Patch "platform/x86: xiaomi-wmi: Fix race condition when reporting key events" has been added to the 5.15-stable tree

by Armin Wolf

Am 26.05.24 um 22:07 schrieb Sasha Levin: > This is a note to let you know that I've just added the patch titled > > platform/x86: xiaomi-wmi: Fix race condition when reporting key events > > to the 5.15-stable tree which can be found at: > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… > > The filename of the patch is: > platform-x86-xiaomi-wmi-fix-race-condition-when-repo.patch > and it can be found in the queue-5.15 subdirectory. > > If you, or anyone else, feels it should not be added to the stable tree, > please let <stable(a)vger.kernel.org> know about it. > Hi, the underlying race condition can only be triggered since commit e2ffcda16290 ("ACPI: OSL: Allow Notify () handlers to run on all CPUs"), which afaik was introduced with kernel 6.8. Because of this, i do not think that we have to backport this commit to kernels before 6.8. Thanks, Armin Wolf > > commit 1f436551dd453c28c23f800e7273136e526197cb > Author: Armin Wolf <W_Armin(a)gmx.de> > Date: Tue Apr 2 16:30:57 2024 +0200 > > platform/x86: xiaomi-wmi: Fix race condition when reporting key events > > [ Upstream commit 290680c2da8061e410bcaec4b21584ed951479af ] > > Multiple WMI events can be received concurrently, so multiple instances > of xiaomi_wmi_notify() can be active at the same time. Since the input > device is shared between those handlers, the key input sequence can be > disturbed. > > Fix this by protecting the key input sequence with a mutex. > > Compile-tested only. > > Fixes: edb73f4f0247 ("platform/x86: wmi: add Xiaomi WMI key driver") > Signed-off-by: Armin Wolf <W_Armin(a)gmx.de> > Link: https://lore.kernel.org/r/20240402143059.8456-2-W_Armin@gmx.de > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com> > Reviewed-by: Hans de Goede <hdegoede(a)redhat.com> > Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> > Signed-off-by: Sasha Levin <sashal(a)kernel.org> > > diff --git a/drivers/platform/x86/xiaomi-wmi.c b/drivers/platform/x86/xiaomi-wmi.c > index 54a2546bb93bf..be80f0bda9484 100644 > --- a/drivers/platform/x86/xiaomi-wmi.c > +++ b/drivers/platform/x86/xiaomi-wmi.c > @@ -2,8 +2,10 @@ > /* WMI driver for Xiaomi Laptops */ > > #include <linux/acpi.h> > +#include <linux/device.h> > #include <linux/input.h> > #include <linux/module.h> > +#include <linux/mutex.h> > #include <linux/wmi.h> > > #include <uapi/linux/input-event-codes.h> > @@ -20,12 +22,21 @@ > > struct xiaomi_wmi { > struct input_dev *input_dev; > + struct mutex key_lock; /* Protects the key event sequence */ > unsigned int key_code; > }; > > +static void xiaomi_mutex_destroy(void *data) > +{ > + struct mutex *lock = data; > + > + mutex_destroy(lock); > +} > + > static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) > { > struct xiaomi_wmi *data; > + int ret; > > if (wdev == NULL || context == NULL) > return -EINVAL; > @@ -35,6 +46,11 @@ static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) > return -ENOMEM; > dev_set_drvdata(&wdev->dev, data); > > + mutex_init(&data->key_lock); > + ret = devm_add_action_or_reset(&wdev->dev, xiaomi_mutex_destroy, &data->key_lock); > + if (ret < 0) > + return ret; > + > data->input_dev = devm_input_allocate_device(&wdev->dev); > if (data->input_dev == NULL) > return -ENOMEM; > @@ -59,10 +75,12 @@ static void xiaomi_wmi_notify(struct wmi_device *wdev, union acpi_object *dummy) > if (data == NULL) > return; > > + mutex_lock(&data->key_lock); > input_report_key(data->input_dev, data->key_code, 1); > input_sync(data->input_dev); > input_report_key(data->input_dev, data->key_code, 0); > input_sync(data->input_dev); > + mutex_unlock(&data->key_lock); > } > > static const struct wmi_device_id xiaomi_wmi_id_table[] = {

1 year, 5 months

1
0
0 0

Re: Patch "platform/x86: xiaomi-wmi: Fix race condition when reporting key events" has been added to the 6.1-stable tree

by Armin Wolf

Am 26.05.24 um 21:57 schrieb Sasha Levin: > This is a note to let you know that I've just added the patch titled > > platform/x86: xiaomi-wmi: Fix race condition when reporting key events > > to the 6.1-stable tree which can be found at: > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… > > The filename of the patch is: > platform-x86-xiaomi-wmi-fix-race-condition-when-repo.patch > and it can be found in the queue-6.1 subdirectory. > > If you, or anyone else, feels it should not be added to the stable tree, > please let <stable(a)vger.kernel.org> know about it. > Hi, the underlying race condition can only be triggered since commit e2ffcda16290 ("ACPI: OSL: Allow Notify () handlers to run on all CPUs"), which afaik was introduced with kernel 6.8. Because of this, i do not think that we have to backport this commit to kernels before 6.8. Thanks, Armin Wolf > > commit 1abdef69265133db29772ed5cefea2338f8ce173 > Author: Armin Wolf <W_Armin(a)gmx.de> > Date: Tue Apr 2 16:30:57 2024 +0200 > > platform/x86: xiaomi-wmi: Fix race condition when reporting key events > > [ Upstream commit 290680c2da8061e410bcaec4b21584ed951479af ] > > Multiple WMI events can be received concurrently, so multiple instances > of xiaomi_wmi_notify() can be active at the same time. Since the input > device is shared between those handlers, the key input sequence can be > disturbed. > > Fix this by protecting the key input sequence with a mutex. > > Compile-tested only. > > Fixes: edb73f4f0247 ("platform/x86: wmi: add Xiaomi WMI key driver") > Signed-off-by: Armin Wolf <W_Armin(a)gmx.de> > Link: https://lore.kernel.org/r/20240402143059.8456-2-W_Armin@gmx.de > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com> > Reviewed-by: Hans de Goede <hdegoede(a)redhat.com> > Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> > Signed-off-by: Sasha Levin <sashal(a)kernel.org> > > diff --git a/drivers/platform/x86/xiaomi-wmi.c b/drivers/platform/x86/xiaomi-wmi.c > index 54a2546bb93bf..be80f0bda9484 100644 > --- a/drivers/platform/x86/xiaomi-wmi.c > +++ b/drivers/platform/x86/xiaomi-wmi.c > @@ -2,8 +2,10 @@ > /* WMI driver for Xiaomi Laptops */ > > #include <linux/acpi.h> > +#include <linux/device.h> > #include <linux/input.h> > #include <linux/module.h> > +#include <linux/mutex.h> > #include <linux/wmi.h> > > #include <uapi/linux/input-event-codes.h> > @@ -20,12 +22,21 @@ > > struct xiaomi_wmi { > struct input_dev *input_dev; > + struct mutex key_lock; /* Protects the key event sequence */ > unsigned int key_code; > }; > > +static void xiaomi_mutex_destroy(void *data) > +{ > + struct mutex *lock = data; > + > + mutex_destroy(lock); > +} > + > static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) > { > struct xiaomi_wmi *data; > + int ret; > > if (wdev == NULL || context == NULL) > return -EINVAL; > @@ -35,6 +46,11 @@ static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) > return -ENOMEM; > dev_set_drvdata(&wdev->dev, data); > > + mutex_init(&data->key_lock); > + ret = devm_add_action_or_reset(&wdev->dev, xiaomi_mutex_destroy, &data->key_lock); > + if (ret < 0) > + return ret; > + > data->input_dev = devm_input_allocate_device(&wdev->dev); > if (data->input_dev == NULL) > return -ENOMEM; > @@ -59,10 +75,12 @@ static void xiaomi_wmi_notify(struct wmi_device *wdev, union acpi_object *dummy) > if (data == NULL) > return; > > + mutex_lock(&data->key_lock); > input_report_key(data->input_dev, data->key_code, 1); > input_sync(data->input_dev); > input_report_key(data->input_dev, data->key_code, 0); > input_sync(data->input_dev); > + mutex_unlock(&data->key_lock); > } > > static const struct wmi_device_id xiaomi_wmi_id_table[] = {

1 year, 5 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror May 2024