December 2025 - Linux-stable-mirror

[PATCH net v2] virtio-net: enable all napis before scheduling refill work

by Bui Quang Minh

Calling napi_disable() on an already disabled napi can cause the deadlock. In commit 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx"), to avoid the deadlock, when pausing the RX in virtnet_rx_pause[_all](), we disable and cancel the delayed refill work. However, in the virtnet_rx_resume_all(), we enable the delayed refill work too early before enabling all the receive queue napis. The deadlock can be reproduced by running selftests/drivers/net/hw/xsk_reconfig.py with multiqueue virtio-net device and inserting a cond_resched() inside the for loop in virtnet_rx_resume_all() to increase the success rate. Because the worker processing the delayed refilled work runs on the same CPU as virtnet_rx_resume_all(), a reschedule is needed to cause the deadlock. In real scenario, the contention on netdev_lock can cause the reschedule. This fixes the deadlock by ensuring all receive queue's napis are enabled before we enable the delayed refill work in virtnet_rx_resume_all() and virtnet_open(). Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx") Reported-by: Paolo Abeni <pabeni(a)redhat.com> Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-… Cc: stable(a)vger.kernel.org Signed-off-by: Bui Quang Minh <minhquangbui99(a)gmail.com> --- Changes in v2: - Move try_fill_recv() before rx napi_enable() - Link to v1: https://lore.kernel.org/netdev/20251208153419.18196-1-minhquangbui99@gmail.… --- drivers/net/virtio_net.c | 71 +++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 8e04adb57f52..4e08880a9467 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -3214,21 +3214,31 @@ static void virtnet_update_settings(struct virtnet_info *vi) static int virtnet_open(struct net_device *dev) { struct virtnet_info *vi = netdev_priv(dev); + bool schedule_refill = false; int i, err; - enable_delayed_refill(vi); - + /* - We must call try_fill_recv before enabling napi of the same receive + * queue so that it doesn't race with the call in virtnet_receive. + * - We must enable and schedule delayed refill work only when we have + * enabled all the receive queue's napi. Otherwise, in refill_work, we + * have a deadlock when calling napi_disable on an already disabled + * napi. + */ for (i = 0; i < vi->max_queue_pairs; i++) { if (i < vi->curr_queue_pairs) /* Make sure we have some buffers: if oom use wq. */ if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL)) - schedule_delayed_work(&vi->refill, 0); + schedule_refill = true; err = virtnet_enable_queue_pair(vi, i); if (err < 0) goto err_enable_qp; } + enable_delayed_refill(vi); + if (schedule_refill) + schedule_delayed_work(&vi->refill, 0); + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) { if (vi->status & VIRTIO_NET_S_LINK_UP) netif_carrier_on(vi->dev); @@ -3463,39 +3473,48 @@ static void virtnet_rx_pause(struct virtnet_info *vi, struct receive_queue *rq) __virtnet_rx_pause(vi, rq); } -static void __virtnet_rx_resume(struct virtnet_info *vi, - struct receive_queue *rq, - bool refill) +static void virtnet_rx_resume_all(struct virtnet_info *vi) { - bool running = netif_running(vi->dev); bool schedule_refill = false; + int i; - if (refill && !try_fill_recv(vi, rq, GFP_KERNEL)) - schedule_refill = true; - if (running) - virtnet_napi_enable(rq); - - if (schedule_refill) - schedule_delayed_work(&vi->refill, 0); -} + if (netif_running(vi->dev)) { + /* See the comment in virtnet_open for the ordering rule + * of try_fill_recv, receive queue napi_enable and delayed + * refill enable/schedule. + */ + for (i = 0; i < vi->max_queue_pairs; i++) { + if (i < vi->curr_queue_pairs) + if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL)) + schedule_refill = true; -static void virtnet_rx_resume_all(struct virtnet_info *vi) -{ - int i; + virtnet_napi_enable(&vi->rq[i]); + } - enable_delayed_refill(vi); - for (i = 0; i < vi->max_queue_pairs; i++) { - if (i < vi->curr_queue_pairs) - __virtnet_rx_resume(vi, &vi->rq[i], true); - else - __virtnet_rx_resume(vi, &vi->rq[i], false); + enable_delayed_refill(vi); + if (schedule_refill) + schedule_delayed_work(&vi->refill, 0); } } static void virtnet_rx_resume(struct virtnet_info *vi, struct receive_queue *rq) { - enable_delayed_refill(vi); - __virtnet_rx_resume(vi, rq, true); + bool schedule_refill = false; + + if (netif_running(vi->dev)) { + /* See the comment in virtnet_open for the ordering rule + * of try_fill_recv, receive queue napi_enable and delayed + * refill enable/schedule. + */ + if (!try_fill_recv(vi, rq, GFP_KERNEL)) + schedule_refill = true; + + virtnet_napi_enable(rq); + + enable_delayed_refill(vi); + if (schedule_refill) + schedule_delayed_work(&vi->refill, 0); + } } static int virtnet_rx_resize(struct virtnet_info *vi, -- 2.43.0

1 week, 1 day

4
7
0 0

[PATCH v2 0/2] fix demotion targets checks in reclaim/demotion

by Bing Jiao

This patch series addresses two issues in demote_folio_list() and can_demote() in reclaim/demotion. Commit 7d709f49babc ("vmscan,cgroup: apply mems_effective to reclaim") introduces the cpuset.mems_effective check and applies it to can_demote(). However: 1. It does not apply this check in demote_folio_list(), which leads to situations where pages are demoted to nodes that are explicitly excluded from the task's cpuset.mems. 2. It checks only the nodes in the immediate next demotion hierarchy and does not check all allowed demotion targets in can_demote(). This can cause pages to never be demoted if the nodes in the next demotion hierarchy are not set in mems_effective. To address these bugs, implement a new function mem_cgroup_filter_mems_allowed() to filter out nodes that are not set in mems_effective, and update demote_folio_list() and can_demote() accordingly. Reproduct Bug 1: Assume a system with 4 nodes, where nodes 0-1 are top-tier and nodes 2-3 are far-tier memory. All nodes have equal capacity. Test script to reproduct: echo 1 > /sys/kernel/mm/numa/demotion_enabled mkdir /sys/fs/cgroup/test echo +cpuset > /sys/fs/cgroup/cgroup.subtree_control echo "0-2" > /sys/fs/cgroup/test/cpuset.mems echo $$ > /sys/fs/cgroup/test/cgroup.procs swapoff -a # Expectation: Should respect node 0-2 limit. # Observation: Node 3 shows significant allocation (MemFree drops) stress-ng --oomable --vm 1 --vm-bytes 150% --mbind 0,1 Reproduct Bug 2: Assume a system with 6 nodes, where nodes 0-2 are top-tier, node 3 is far-tier node, and nodes 4-5 are the farthest-tier nodes. All nodes have equal capacity. Test script: echo 1 > /sys/kernel/mm/numa/demotion_enabled mkdir /sys/fs/cgroup/test echo +cpuset > /sys/fs/cgroup/cgroup.subtree_control echo "0-2,4-5" > /sys/fs/cgroup/test/cpuset.mems echo $$ > /sys/fs/cgroup/test/cgroup.procs swapoff -a # Expectation: Pages are demoted to Nodes 4-5 # Observation: No pages are demoted before oom. stress-ng --oomable --vm 1 --vm-bytes 150% --mbind 0,1,2 Bing Jiao (2): mm/vmscan: respect mems_effective in demote_folio_list() mm/vmscan: check all allowed targets in can_demote() include/linux/cpuset.h | 6 +++--- include/linux/memcontrol.h | 6 +++--- kernel/cgroup/cpuset.c | 12 +++++------- mm/memcontrol.c | 5 +++-- mm/vmscan.c | 27 ++++++++++++++++++--------- 5 files changed, 32 insertions(+), 24 deletions(-) -- 2.52.0.351.gbe84eed79e-goog

1 week, 1 day

3
9
0 0

[PATCH v2] virtio: console: fix lost wakeup when device is written and polled

by Lorenz Bauer

A process issuing blocking writes to a virtio console may get stuck indefinitely if another thread polls the device. Here is how to trigger the bug: - Thread A writes to the port until the virtqueue is full. - Thread A calls wait_port_writable() and goes to sleep, waiting on port->waitqueue. - The host processes some of the write, marks buffers as used and raises an interrupt. - Before the interrupt is serviced, thread B executes port_fops_poll(). This calls reclaim_consumed_buffers() via will_write_block() and consumes all used buffers. - The interrupt is serviced. vring_interrupt() finds no used buffers via more_used() and returns without waking port->waitqueue. - Thread A is still in wait_event(port->waitqueue), waiting for a wakeup that never arrives. The crux is that invoking reclaim_consumed_buffers() may cause vring_interrupt() to omit wakeups. Fix this by calling reclaim_consumed_buffers() in out_int() before waking. This is similar to the call to discard_port_data() in in_intr() which also frees buffer from a non-sleepable context. This in turn guarantees that port->outvq_full is up to date when handling polling. Since in_intr() already populates port->inbuf we use that to avoid changing reader state. Cc: stable(a)vger.kernel.org Signed-off-by: Lorenz Bauer <lmb(a)isovalent.com> --- As far as I can tell all currently maintained stable series kernels need this commit. Applies and builds cleanly on 5.10.247, verified to fix the issue. --- Changes in v2: - Call reclaim_consumed_buffers() in out_intr instead of issuing another wake. - Link to v1: https://lore.kernel.org/r/20251215-virtio-console-lost-wakeup-v1-1-79a5c578… --- drivers/char/virtio_console.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 088182e54debd6029ea2c2a5542d7a28500e67b8..351e445da35e25910671615a8ecc79165dfc66b7 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -971,10 +971,17 @@ static __poll_t port_fops_poll(struct file *filp, poll_table *wait) return EPOLLHUP; } ret = 0; - if (!will_read_block(port)) + + spin_lock(&port->inbuf_lock); + if (port->inbuf) ret |= EPOLLIN | EPOLLRDNORM; - if (!will_write_block(port)) + spin_unlock(&port->inbuf_lock); + + spin_lock(&port->outvq_lock); + if (!port->outvq_full) ret |= EPOLLOUT; + spin_unlock(&port->outvq_lock); + if (!port->host_connected) ret |= EPOLLHUP; @@ -1698,6 +1705,7 @@ static void flush_bufs(struct virtqueue *vq, bool can_sleep) static void out_intr(struct virtqueue *vq) { struct port *port; + unsigned long flags; port = find_port_by_vq(vq->vdev->priv, vq); if (!port) { @@ -1705,6 +1713,10 @@ static void out_intr(struct virtqueue *vq) return; } + spin_lock_irqsave(&port->outvq_lock, flags); + reclaim_consumed_buffers(port); + spin_unlock_irqrestore(&port->outvq_lock, flags); + wake_up_interruptible(&port->waitqueue); } --- base-commit: d358e5254674b70f34c847715ca509e46eb81e6f change-id: 20251215-virtio-console-lost-wakeup-0f566c5cd35f Best regards, -- Lorenz Bauer <lmb(a)isovalent.com>

1 week, 1 day

2
3
0 0

[PATCH v2] mm/memory-failure: teach kill_accessing_process to accept hugetlb tail page pfn

by Jane Chu

When a hugetlb folio is being poisoned again, try_memory_failure_hugetlb() passed head pfn to kill_accessing_process(), that is not right. The precise pfn of the poisoned page should be used in order to determine the precise vaddr as the SIGBUS payload. This issue has already been taken care of in the normal path, that is, hwpoison_user_mappings(), see [1][2]. Further more, for [3] to work correctly in the hugetlb repoisoning case, it's essential to inform VM the precise poisoned page, not the head page. [1] https://lkml.kernel.org/r/20231218135837.3310403-1-willy@infradead.org [2] https://lkml.kernel.org/r/20250224211445.2663312-1-jane.chu@oracle.com [3] https://lore.kernel.org/lkml/20251116013223.1557158-1-jiaqiyan@google.com/ Cc: <stable(a)vger.kernel.org> Signed-off-by: Jane Chu <jane.chu(a)oracle.com> Reviewed-by: Liam R. Howlett <Liam.Howlett(a)oracle.com> --- v1 -> v2: pickup R-B, add stable to cc list. --- mm/memory-failure.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 3edebb0cda30..c9d87811b1ea 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -681,9 +681,11 @@ static void set_to_kill(struct to_kill *tk, unsigned long addr, short shift) } static int check_hwpoisoned_entry(pte_t pte, unsigned long addr, short shift, - unsigned long poisoned_pfn, struct to_kill *tk) + unsigned long poisoned_pfn, struct to_kill *tk, + int pte_nr) { unsigned long pfn = 0; + unsigned long hwpoison_vaddr; if (pte_present(pte)) { pfn = pte_pfn(pte); @@ -694,10 +696,11 @@ static int check_hwpoisoned_entry(pte_t pte, unsigned long addr, short shift, pfn = swp_offset_pfn(swp); } - if (!pfn || pfn != poisoned_pfn) + if (!pfn || (pfn > poisoned_pfn || (pfn + pte_nr - 1) < poisoned_pfn)) return 0; - set_to_kill(tk, addr, shift); + hwpoison_vaddr = addr + ((poisoned_pfn - pfn) << PAGE_SHIFT); + set_to_kill(tk, hwpoison_vaddr, shift); return 1; } @@ -749,7 +752,7 @@ static int hwpoison_pte_range(pmd_t *pmdp, unsigned long addr, for (; addr != end; ptep++, addr += PAGE_SIZE) { ret = check_hwpoisoned_entry(ptep_get(ptep), addr, PAGE_SHIFT, - hwp->pfn, &hwp->tk); + hwp->pfn, &hwp->tk, 1); if (ret == 1) break; } @@ -772,8 +775,8 @@ static int hwpoison_hugetlb_range(pte_t *ptep, unsigned long hmask, ptl = huge_pte_lock(h, walk->mm, ptep); pte = huge_ptep_get(walk->mm, addr, ptep); - ret = check_hwpoisoned_entry(pte, addr, huge_page_shift(h), - hwp->pfn, &hwp->tk); + ret = check_hwpoisoned_entry(pte, addr, huge_page_shift(h), hwp->pfn, + &hwp->tk, pages_per_huge_page(h)); spin_unlock(ptl); return ret; } @@ -2023,10 +2026,8 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb *hugetlb = 0; return 0; } else if (res == -EHWPOISON) { - if (flags & MF_ACTION_REQUIRED) { - folio = page_folio(p); - res = kill_accessing_process(current, folio_pfn(folio), flags); - } + if (flags & MF_ACTION_REQUIRED) + res = kill_accessing_process(current, pfn, flags); action_result(pfn, MF_MSG_ALREADY_POISONED, MF_FAILED); return res; } else if (res == -EBUSY) { @@ -2037,6 +2038,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb return action_result(pfn, MF_MSG_GET_HWPOISON, MF_IGNORED); } + folio = page_folio(p); folio_lock(folio); -- 2.43.5

1 week, 1 day

3
2
0 0

[PATCH] drm/msm/a6xx: fix bogus hwcg register updates

by Johan Hovold

The hw clock gating register sequence consists of register value pairs that are written to the GPU during initialisation. The a690 hwcg sequence has two GMU registers in it that used to amount to random writes in the GPU mapping, but since commit 188db3d7fe66 ("drm/msm/a6xx: Rebase GMU register offsets") they trigger a fault as the updated offsets now lie outside the mapping. This in turn breaks boot of machines like the Lenovo ThinkPad X13s. Note that the updates of these GMU registers is already taken care of properly since commit 40c297eb245b ("drm/msm/a6xx: Set GMU CGC properties on a6xx too"), but for some reason these two entries were left in the table. Fixes: 5e7665b5e484 ("drm/msm/adreno: Add Adreno A690 support") Cc: stable(a)vger.kernel.org # 6.5 Cc: Bjorn Andersson <andersson(a)kernel.org> Cc: Konrad Dybcio <konradybcio(a)kernel.org> Signed-off-by: Johan Hovold <johan(a)kernel.org> --- drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c index 29107b362346..4c2f739ee9b7 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c @@ -501,8 +501,6 @@ static const struct adreno_reglist a690_hwcg[] = { {REG_A6XX_RBBM_CLOCK_CNTL_GMU_GX, 0x00000222}, {REG_A6XX_RBBM_CLOCK_DELAY_GMU_GX, 0x00000111}, {REG_A6XX_RBBM_CLOCK_HYST_GMU_GX, 0x00000555}, - {REG_A6XX_GPU_GMU_AO_GMU_CGC_DELAY_CNTL, 0x10111}, - {REG_A6XX_GPU_GMU_AO_GMU_CGC_HYST_CNTL, 0x5555}, {} }; -- 2.51.2

1 week, 1 day

3
2
0 0

[PATCH] virtio: console: fix lost wakeup when device is written and polled

by Lorenz Bauer

A process issuing blocking writes to a virtio console may get stuck indefinitely if another thread polls the device. Here is how to trigger the bug: - Thread A writes to the port until the virtqueue is full. - Thread A calls wait_port_writable() and goes to sleep, waiting on port->waitqueue. - The host processes some of the write, marks buffers as used and raises an interrupt. - Before the interrupt is serviced, thread B executes port_fops_poll(). This calls reclaim_consumed_buffers() via will_write_block() and consumes all used buffers. - The interrupt is serviced. vring_interrupt() finds no used buffers via more_used() and returns without waking port->waitqueue. - Thread A is still in wait_event(port->waitqueue), waiting for a wakeup that never arrives. The crux is that invoking reclaim_consumed_buffers() may cause vring_interrupt() to omit wakeups. Fix this by making reclaim_consumed_buffers() issue an additional wake up if it consumed any buffers. Signed-off-by: Lorenz Bauer <lmb(a)isovalent.com> --- As far as I can tell all currently maintained stable series kernels need this commit. I've tested that it applies cleanly to 5.10.247, however wasn't able to build the kernel due to an unrelated link error. Instead I applied it to 5.15.197, which compiled and verified to be fixed. --- drivers/char/virtio_console.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 088182e54debd6029ea2c2a5542d7a28500e67b8..7cd3ad9da9b53a7a570410f12501acc7fd7e3b9b 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -581,6 +581,7 @@ static ssize_t send_control_msg(struct port *port, unsigned int event, static void reclaim_consumed_buffers(struct port *port) { struct port_buffer *buf; + bool freed = false; unsigned int len; if (!port->portdev) { @@ -589,7 +590,15 @@ static void reclaim_consumed_buffers(struct port *port) } while ((buf = virtqueue_get_buf(port->out_vq, &len))) { free_buf(buf, false); + freed = true; + } + if (freed) { + /* We freed all used buffers. Issue a wake up so that other pending + * tasks do not get stuck. This is necessary because vring_interrupt() + * will drop wakeups from the host if there are no used buffers. + */ port->outvq_full = false; + wake_up_interruptible(&port->waitqueue); } } --- base-commit: d358e5254674b70f34c847715ca509e46eb81e6f change-id: 20251215-virtio-console-lost-wakeup-0f566c5cd35f Best regards, -- Lorenz Bauer <lmb(a)isovalent.com>

1 week, 1 day

3
4
0 0

[PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU

by Ariadne Conill

Xen domU cannot access the given MMIO address for security reasons, resulting in a failed hypercall in ioremap() due to permissions. Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset") Signed-off-by: Ariadne Conill <ariadne(a)ariadne.space> Cc: xen-devel(a)lists.xenproject.org Cc: stable(a)vger.kernel.org --- arch/x86/kernel/cpu/amd.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index a6f88ca1a6b4..99308fba4d7d 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -29,6 +29,8 @@ # include <asm/mmconfig.h> #endif +#include <xen/xen.h> + #include "cpu.h" u16 invlpgb_count_max __ro_after_init = 1; @@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void) if (!cpu_feature_enabled(X86_FEATURE_ZEN)) return 0; + /* Xen PV domU cannot access hardware directly, so bail for domU case */ + if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain()) + return 0; + addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value)); if (!addr) return 0; -- 2.51.0

1 week, 2 days

5
11
0 0

[PATCH v2] usb: typec: ucsi: Fix null pointer dereference in ucsi_sync_control_common

by Mario Limonciello (AMD)

Add missing null check for cci parameter before dereferencing it in ucsi_sync_control_common(). The function can be called with cci=NULL from ucsi_acknowledge(), which leads to a null pointer dereference when accessing *cci in the condition check. The crash occurs because the code checks if cci is not null before calling ucsi->ops->read_cci(ucsi, cci), but then immediately dereferences cci without a null check in the following condition: (*cci & UCSI_CCI_COMMAND_COMPLETE). KASAN trace: KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] RIP: 0010:ucsi_sync_control_common+0x2ae/0x4e0 [typec_ucsi] Cc: stable(a)vger.kernel.org Fixes: 667ecac55861 ("usb: typec: ucsi: return CCI and message from sync_control callback") Reviewed-by: Heikki Krogerus <heikki.krogerus(a)linux.intel.com> Signed-off-by: Mario Limonciello (AMD) <superm1(a)kernel.org> --- v2: * Add stable tag * Add Heikki's tag --- drivers/usb/typec/ucsi/ucsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index 9b3df776137a1..7129973f19e7e 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -97,7 +97,7 @@ int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci) if (!ret && cci) ret = ucsi->ops->read_cci(ucsi, cci); - if (!ret && ucsi->message_in_size > 0 && + if (!ret && cci && ucsi->message_in_size > 0 && (*cci & UCSI_CCI_COMMAND_COMPLETE)) ret = ucsi->ops->read_message_in(ucsi, ucsi->message_in, ucsi->message_in_size); -- 2.43.0

1 week, 2 days

2
1
0 0

[PATCH net v3] net: nfc: nci: Fix parameter validation for packet data

by Michael Thalmeier

Since commit 9c328f54741b ("net: nfc: nci: Add parameter validation for packet data") communication with nci nfc chips is not working any more. The mentioned commit tries to fix access of uninitialized data, but failed to understand that in some cases the data packet is of variable length and can therefore not be compared to the maximum packet length given by the sizeof(struct). Fixes: 9c328f54741b ("net: nfc: nci: Add parameter validation for packet data") Cc: stable(a)vger.kernel.org Signed-off-by: Michael Thalmeier <michael.thalmeier(a)hale.at> --- v3: - perform complete checks - replace magic numbers with offsetofend and sizeof v2: - Reference correct commit hash --- net/nfc/nci/ntf.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c index 418b84e2b260..0190332cf454 100644 --- a/net/nfc/nci/ntf.c +++ b/net/nfc/nci/ntf.c @@ -58,7 +58,7 @@ static int nci_core_conn_credits_ntf_packet(struct nci_dev *ndev, struct nci_conn_info *conn_info; int i; - if (skb->len < sizeof(struct nci_core_conn_credit_ntf)) + if (skb->len < offsetofend(struct nci_core_conn_credit_ntf, num_entries)) return -EINVAL; ntf = (struct nci_core_conn_credit_ntf *)skb->data; @@ -68,6 +68,10 @@ static int nci_core_conn_credits_ntf_packet(struct nci_dev *ndev, if (ntf->num_entries > NCI_MAX_NUM_CONN) ntf->num_entries = NCI_MAX_NUM_CONN; + if (skb->len < offsetofend(struct nci_core_conn_credit_ntf, num_entries) + + ntf->num_entries * sizeof(struct conn_credit_entry)) + return -EINVAL; + /* update the credits */ for (i = 0; i < ntf->num_entries; i++) { ntf->conn_entries[i].conn_id = @@ -364,7 +368,7 @@ static int nci_rf_discover_ntf_packet(struct nci_dev *ndev, const __u8 *data; bool add_target = true; - if (skb->len < sizeof(struct nci_rf_discover_ntf)) + if (skb->len < offsetofend(struct nci_rf_discover_ntf, rf_tech_specific_params_len)) return -EINVAL; data = skb->data; @@ -380,6 +384,10 @@ static int nci_rf_discover_ntf_packet(struct nci_dev *ndev, pr_debug("rf_tech_specific_params_len %d\n", ntf.rf_tech_specific_params_len); + if (skb->len < (data - skb->data) + + ntf.rf_tech_specific_params_len + sizeof(ntf.ntf_type)) + return -EINVAL; + if (ntf.rf_tech_specific_params_len > 0) { switch (ntf.rf_tech_and_mode) { case NCI_NFC_A_PASSIVE_POLL_MODE: @@ -596,7 +604,7 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev, const __u8 *data; int err = NCI_STATUS_OK; - if (skb->len < sizeof(struct nci_rf_intf_activated_ntf)) + if (skb->len < offsetofend(struct nci_rf_intf_activated_ntf, rf_tech_specific_params_len)) return -EINVAL; data = skb->data; @@ -628,6 +636,9 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev, if (ntf.rf_interface == NCI_RF_INTERFACE_NFCEE_DIRECT) goto listen; + if (skb->len < (data - skb->data) + ntf.rf_tech_specific_params_len) + return -EINVAL; + if (ntf.rf_tech_specific_params_len > 0) { switch (ntf.activation_rf_tech_and_mode) { case NCI_NFC_A_PASSIVE_POLL_MODE: @@ -668,6 +679,11 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev, } } + if (skb->len < (data - skb->data) + sizeof(ntf.data_exch_rf_tech_and_mode) + + sizeof(ntf.data_exch_tx_bit_rate) + sizeof (ntf.data_exch_rx_bit_rate) + + sizeof(ntf.activation_params_len)) + return -EINVAL; + ntf.data_exch_rf_tech_and_mode = *data++; ntf.data_exch_tx_bit_rate = *data++; ntf.data_exch_rx_bit_rate = *data++; @@ -679,6 +695,9 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev, pr_debug("data_exch_rx_bit_rate 0x%x\n", ntf.data_exch_rx_bit_rate); pr_debug("activation_params_len %d\n", ntf.activation_params_len); + if (skb->len < (data - skb->data) + ntf.activation_params_len) + return -EINVAL; + if (ntf.activation_params_len > 0) { switch (ntf.rf_interface) { case NCI_RF_INTERFACE_ISO_DEP: -- 2.52.0

1 week, 2 days

2
1
0 0

[PATCH v6 05/14] reset: rzv2h-usb2phy: Keep PHY clock enabled for entire device lifetime

by Tommaso Merciai

The driver was disabling the USB2 PHY clock immediately after register initialization in probe() and after each reset operation. This left the PHY unclocked even though it must remain active for USB functionality. The behavior appeared to work only when another driver (e.g., USB controller) had already enabled the clock, making operation unreliable and hardware-dependent. In configurations where this driver is the sole clock user, USB functionality would fail. Fix this by: - Enabling the clock once in probe() via pm_runtime_resume_and_get() - Removing all pm_runtime_put() calls from assert/deassert/status - Registering a devm cleanup action to release the clock at removal - Removed rzv2h_usbphy_assert_helper() and its call in rzv2h_usb2phy_reset_probe() This ensures the PHY clock remains enabled for the entire device lifetime, preventing instability and aligning with hardware requirements. Cc: stable(a)vger.kernel.org Fixes: e3911d7f865b ("reset: Add USB2PHY port reset driver for Renesas RZ/V2H(P)") Signed-off-by: Tommaso Merciai <tommaso.merciai.xr(a)bp.renesas.com> --- v5->v6: - No changes v4->v5: - No changes. v3->v4: - No changes. v2->v3: - Added missing Cc: stable(a)vger.kernel.org - Improved commit body describing the removal of rzv2h_usbphy_assert_helper() from rzv2h_usb2phy_reset_probe(). v1->v2: - Improve commit body and commit msg - Added Fixes tag - Dropped unnecessary rzv2h_usbphy_assert_helper() function drivers/reset/reset-rzv2h-usb2phy.c | 64 ++++++++--------------------- 1 file changed, 18 insertions(+), 46 deletions(-) diff --git a/drivers/reset/reset-rzv2h-usb2phy.c b/drivers/reset/reset-rzv2h-usb2phy.c index ae643575b067..5bdd39274612 100644 --- a/drivers/reset/reset-rzv2h-usb2phy.c +++ b/drivers/reset/reset-rzv2h-usb2phy.c @@ -49,9 +49,10 @@ static inline struct rzv2h_usb2phy_reset_priv return container_of(rcdev, struct rzv2h_usb2phy_reset_priv, rcdev); } -/* This function must be called only after pm_runtime_resume_and_get() has been called */ -static void rzv2h_usbphy_assert_helper(struct rzv2h_usb2phy_reset_priv *priv) +static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev, + unsigned long id) { + struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev); const struct rzv2h_usb2phy_reset_of_data *data = priv->data; scoped_guard(spinlock, &priv->lock) { @@ -60,24 +61,6 @@ static void rzv2h_usbphy_assert_helper(struct rzv2h_usb2phy_reset_priv *priv) } usleep_range(11, 20); -} - -static int rzv2h_usbphy_reset_assert(struct reset_controller_dev *rcdev, - unsigned long id) -{ - struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev); - struct device *dev = priv->dev; - int ret; - - ret = pm_runtime_resume_and_get(dev); - if (ret) { - dev_err(dev, "pm_runtime_resume_and_get failed\n"); - return ret; - } - - rzv2h_usbphy_assert_helper(priv); - - pm_runtime_put(dev); return 0; } @@ -87,14 +70,6 @@ static int rzv2h_usbphy_reset_deassert(struct reset_controller_dev *rcdev, { struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev); const struct rzv2h_usb2phy_reset_of_data *data = priv->data; - struct device *dev = priv->dev; - int ret; - - ret = pm_runtime_resume_and_get(dev); - if (ret) { - dev_err(dev, "pm_runtime_resume_and_get failed\n"); - return ret; - } scoped_guard(spinlock, &priv->lock) { writel(data->reset_deassert_val, priv->base + data->reset_reg); @@ -102,8 +77,6 @@ static int rzv2h_usbphy_reset_deassert(struct reset_controller_dev *rcdev, writel(data->reset_release_val, priv->base + data->reset_reg); } - pm_runtime_put(dev); - return 0; } @@ -111,20 +84,10 @@ static int rzv2h_usbphy_reset_status(struct reset_controller_dev *rcdev, unsigned long id) { struct rzv2h_usb2phy_reset_priv *priv = rzv2h_usbphy_rcdev_to_priv(rcdev); - struct device *dev = priv->dev; - int ret; u32 reg; - ret = pm_runtime_resume_and_get(dev); - if (ret) { - dev_err(dev, "pm_runtime_resume_and_get failed\n"); - return ret; - } - reg = readl(priv->base + priv->data->reset_reg); - pm_runtime_put(dev); - return (reg & priv->data->reset_status_bits) == priv->data->reset_status_bits; } @@ -141,6 +104,11 @@ static int rzv2h_usb2phy_reset_of_xlate(struct reset_controller_dev *rcdev, return 0; } +static void rzv2h_usb2phy_reset_pm_runtime_put(void *data) +{ + pm_runtime_put(data); +} + static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev) { const struct rzv2h_usb2phy_reset_of_data *data; @@ -175,14 +143,14 @@ static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev) if (error) return dev_err_probe(dev, error, "pm_runtime_resume_and_get failed\n"); + error = devm_add_action_or_reset(dev, rzv2h_usb2phy_reset_pm_runtime_put, + dev); + if (error) + return dev_err_probe(dev, error, "unable to register cleanup action\n"); + for (unsigned int i = 0; i < data->init_val_count; i++) writel(data->init_vals[i].val, priv->base + data->init_vals[i].reg); - /* keep usb2phy in asserted state */ - rzv2h_usbphy_assert_helper(priv); - - pm_runtime_put(dev); - priv->rcdev.ops = &rzv2h_usbphy_reset_ops; priv->rcdev.of_reset_n_cells = 0; priv->rcdev.nr_resets = 1; @@ -190,7 +158,11 @@ static int rzv2h_usb2phy_reset_probe(struct platform_device *pdev) priv->rcdev.of_node = dev->of_node; priv->rcdev.dev = dev; - return devm_reset_controller_register(dev, &priv->rcdev); + error = devm_reset_controller_register(dev, &priv->rcdev); + if (error) + return dev_err_probe(dev, error, "could not register reset controller\n"); + + return 0; } /* -- 2.43.0

1 week, 2 days

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror December 2025