October 2024 - Linux-stable-mirror

[PATCH v2] drm: xlnx: zynqmp_dpsub: fix hotplug detection

by Steffen Dirkwinkel

From: Steffen Dirkwinkel <s.dirkwinkel(a)beckhoff.com> drm_kms_helper_poll_init needs to be called after zynqmp_dpsub_kms_init. zynqmp_dpsub_kms_init creates the connector and without it we don't enable hotplug detection. Fixes: eb2d64bfcc17 ("drm: xlnx: zynqmp_dpsub: Report HPD through the bridge") Cc: stable(a)vger.kernel.org Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel(a)beckhoff.com> --- drivers/gpu/drm/xlnx/zynqmp_kms.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c index bd1368df7870..311397cee5ca 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c @@ -509,12 +509,12 @@ int zynqmp_dpsub_drm_init(struct zynqmp_dpsub *dpsub) if (ret) return ret; - drm_kms_helper_poll_init(drm); - ret = zynqmp_dpsub_kms_init(dpsub); if (ret < 0) goto err_poll_fini; + drm_kms_helper_poll_init(drm); + /* Reset all components and register the DRM device. */ drm_mode_config_reset(drm); -- 2.47.0

7 months, 1 week

2
1
0 0

[PATCH 0/2] cpuidle: qcom-spm: fix resource release in spm_cpuidle_register

by Javier Carrasco

This series addresses two issues in spm_cpuidle_register: a missing device_node release in an error path, and releasing a reference to a device after its usage. These issues were found while analyzing the code, and the patches have been successfully compiled and statically analyzed, but not tested on real hardware as I don't have access to it. Any volunteering for testing is always more than welcome. Signed-off-by: Javier Carrasco <javier.carrasco.cruz(a)gmail.com> --- Javier Carrasco (2): cpuidle: qcom-spm: fix device node release in spm_cpuidle_register cpuidle: qcom-spm: fix platform device release in spm_cpuidle_register drivers/cpuidle/cpuidle-qcom-spm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) --- base-commit: 6fb2fa9805c501d9ade047fc511961f3273cdcb5 change-id: 20241029-cpuidle-qcom-spm-cleanup-6f03669bcd70 Best regards, -- Javier Carrasco <javier.carrasco.cruz(a)gmail.com>

7 months, 1 week

2
4
0 0

[PATCH] accel/ivpu: Fix NOC firewall interrupt handling

by Jacek Lawrynowicz

From: Andrzej Kacprowski <Andrzej.Kacprowski(a)intel.com> The NOC firewall interrupt means that the HW prevented unauthorized access to a protected resource, so there is no need to trigger device reset in such case. To facilitate security testing add firewall_irq_counter debugfs file that tracks firewall interrupts. Fixes: 8a27ad81f7d3 ("accel/ivpu: Split IP and buttress code") Cc: <stable(a)vger.kernel.org> # v6.11+ Signed-off-by: Andrzej Kacprowski <Andrzej.Kacprowski(a)intel.com> Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz(a)linux.intel.com> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz(a)linux.intel.com> --- drivers/accel/ivpu/ivpu_debugfs.c | 9 +++++++++ drivers/accel/ivpu/ivpu_hw.c | 1 + drivers/accel/ivpu/ivpu_hw.h | 1 + drivers/accel/ivpu/ivpu_hw_ip.c | 5 ++++- 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/accel/ivpu/ivpu_debugfs.c b/drivers/accel/ivpu/ivpu_debugfs.c index 8958145c49adb..8180b95ed69dc 100644 --- a/drivers/accel/ivpu/ivpu_debugfs.c +++ b/drivers/accel/ivpu/ivpu_debugfs.c @@ -116,6 +116,14 @@ static int reset_pending_show(struct seq_file *s, void *v) return 0; } +static int firewall_irq_counter_show(struct seq_file *s, void *v) +{ + struct ivpu_device *vdev = seq_to_ivpu(s); + + seq_printf(s, "%d\n", atomic_read(&vdev->hw->firewall_irq_counter)); + return 0; +} + static const struct drm_debugfs_info vdev_debugfs_list[] = { {"bo_list", bo_list_show, 0}, {"fw_name", fw_name_show, 0}, @@ -125,6 +133,7 @@ static const struct drm_debugfs_info vdev_debugfs_list[] = { {"last_bootmode", last_bootmode_show, 0}, {"reset_counter", reset_counter_show, 0}, {"reset_pending", reset_pending_show, 0}, + {"firewall_irq_counter", firewall_irq_counter_show, 0}, }; static int dvfs_mode_get(void *data, u64 *dvfs_mode) diff --git a/drivers/accel/ivpu/ivpu_hw.c b/drivers/accel/ivpu/ivpu_hw.c index 09ada8b500b99..4e1054f3466e8 100644 --- a/drivers/accel/ivpu/ivpu_hw.c +++ b/drivers/accel/ivpu/ivpu_hw.c @@ -252,6 +252,7 @@ int ivpu_hw_init(struct ivpu_device *vdev) platform_init(vdev); wa_init(vdev); timeouts_init(vdev); + atomic_set(&vdev->hw->firewall_irq_counter, 0); return 0; } diff --git a/drivers/accel/ivpu/ivpu_hw.h b/drivers/accel/ivpu/ivpu_hw.h index dc5518248c405..fc4dbfc980c81 100644 --- a/drivers/accel/ivpu/ivpu_hw.h +++ b/drivers/accel/ivpu/ivpu_hw.h @@ -51,6 +51,7 @@ struct ivpu_hw_info { int dma_bits; ktime_t d0i3_entry_host_ts; u64 d0i3_entry_vpu_ts; + atomic_t firewall_irq_counter; }; int ivpu_hw_init(struct ivpu_device *vdev); diff --git a/drivers/accel/ivpu/ivpu_hw_ip.c b/drivers/accel/ivpu/ivpu_hw_ip.c index b9b16f4041434..029dd065614b2 100644 --- a/drivers/accel/ivpu/ivpu_hw_ip.c +++ b/drivers/accel/ivpu/ivpu_hw_ip.c @@ -1073,7 +1073,10 @@ static void irq_wdt_mss_handler(struct ivpu_device *vdev) static void irq_noc_firewall_handler(struct ivpu_device *vdev) { - ivpu_pm_trigger_recovery(vdev, "NOC Firewall IRQ"); + atomic_inc(&vdev->hw->firewall_irq_counter); + + ivpu_dbg(vdev, IRQ, "NOC Firewall interrupt detected, counter %d\n", + atomic_read(&vdev->hw->firewall_irq_counter)); } /* Handler for IRQs from NPU core */ -- 2.45.1

7 months, 1 week

2
3
0 0

+ mm-page_alloc-keep-track-of-free-highatomic.patch added to mm-hotfixes-unstable branch

by Andrew Morton

The patch titled Subject: mm/page_alloc: keep track of free highatomic has been added to the -mm mm-hotfixes-unstable branch. Its filename is mm-page_alloc-keep-track-of-free-highatomic.patch This patch will shortly appear at https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patche… This patch will later appear in the mm-hotfixes-unstable branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm Before you just go and hit "reply", please: a) Consider who else should be cc'ed b) Prefer to cc a suitable mailing list as well c) Ideally: find the original patch on the mailing list and do a reply-to-all to that, adding suitable additional cc's *** Remember to use Documentation/process/submit-checklist.rst when testing your code *** The -mm tree is included into linux-next via the mm-everything branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm and is updated there every 2-3 working days ------------------------------------------------------ From: Yu Zhao <yuzhao(a)google.com> Subject: mm/page_alloc: keep track of free highatomic Date: Mon, 28 Oct 2024 12:26:53 -0600 OOM kills due to vastly overestimated free highatomic reserves were observed: ... invoked oom-killer: gfp_mask=0x100cca(GFP_HIGHUSER_MOVABLE), order=0 ... Node 0 Normal free:1482936kB boost:0kB min:410416kB low:739404kB high:1068392kB reserved_highatomic:1073152KB ... Node 0 Normal: 1292*4kB (ME) 1920*8kB (E) 383*16kB (UE) 220*32kB (ME) 340*64kB (E) 2155*128kB (UE) 3243*256kB (UE) 615*512kB (U) 1*1024kB (M) 0*2048kB 0*4096kB = 1477408kB The second line above shows that the OOM kill was due to the following condition: free (1482936kB) - reserved_highatomic (1073152kB) = 409784KB < min (410416kB) And the third line shows there were no free pages in any MIGRATE_HIGHATOMIC pageblocks, which otherwise would show up as type 'H'. Therefore __zone_watermark_unusable_free() underestimated the usable free memory by over 1GB, which resulted in the unnecessary OOM kill above. The comments in __zone_watermark_unusable_free() warns about the potential risk, i.e., If the caller does not have rights to reserves below the min watermark then subtract the high-atomic reserves. This will over-estimate the size of the atomic reserve but it avoids a search. However, it is possible to keep track of free pages in reserved highatomic pageblocks with a new per-zone counter nr_free_highatomic protected by the zone lock, to avoid a search when calculating the usable free memory. And the cost would be minimal, i.e., simple arithmetics in the highatomic alloc/free/move paths. Note that since nr_free_highatomic can be relatively small, using a per-cpu counter might cause too much drift and defeat its purpose, in addition to the extra memory overhead. Link: https://lkml.kernel.org/r/20241028182653.3420139-1-yuzhao@google.com Signed-off-by: Yu Zhao <yuzhao(a)google.com> Reported-by: Link Lin <linkl(a)google.com> Acked-by: David Rientjes <rientjes(a)google.com> Acked-by: Vlastimil Babka <vbabka(a)suse.cz> Cc: <stable(a)vger.kernel.org> [6.12+] Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org> --- include/linux/mmzone.h | 1 + mm/page_alloc.c | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) --- a/include/linux/mmzone.h~mm-page_alloc-keep-track-of-free-highatomic +++ a/include/linux/mmzone.h @@ -823,6 +823,7 @@ struct zone { unsigned long watermark_boost; unsigned long nr_reserved_highatomic; + unsigned long nr_free_highatomic; /* * We don't know if the memory that we're going to allocate will be --- a/mm/page_alloc.c~mm-page_alloc-keep-track-of-free-highatomic +++ a/mm/page_alloc.c @@ -635,6 +635,8 @@ compaction_capture(struct capture_contro static inline void account_freepages(struct zone *zone, int nr_pages, int migratetype) { + lockdep_assert_held(&zone->lock); + if (is_migrate_isolate(migratetype)) return; @@ -642,6 +644,9 @@ static inline void account_freepages(str if (is_migrate_cma(migratetype)) __mod_zone_page_state(zone, NR_FREE_CMA_PAGES, nr_pages); + + if (is_migrate_highatomic(migratetype)) + WRITE_ONCE(zone->nr_free_highatomic, zone->nr_free_highatomic + nr_pages); } /* Used for pages not on another list */ @@ -3081,11 +3086,10 @@ static inline long __zone_watermark_unus /* * If the caller does not have rights to reserves below the min - * watermark then subtract the high-atomic reserves. This will - * over-estimate the size of the atomic reserve but it avoids a search. + * watermark then subtract the free pages reserved for highatomic. */ if (likely(!(alloc_flags & ALLOC_RESERVES))) - unusable_free += z->nr_reserved_highatomic; + unusable_free += READ_ONCE(z->nr_free_highatomic); #ifdef CONFIG_CMA /* If allocation can't use CMA areas don't use free CMA pages */ _ Patches currently in -mm which might be from yuzhao(a)google.com are mm-allow-set-clear-page_type-again.patch mm-multi-gen-lru-remove-mm_leaf_old-and-mm_nonleaf_total-stats.patch mm-multi-gen-lru-use-pteppmdp_clear_young_notify.patch mm-page_alloc-keep-track-of-free-highatomic.patch

7 months, 1 week

2
3
0 0

[PATCH v7 6/7] PCI: qcom: Disable ASPM L0s and remove BDF2SID mapping config for X1E80100 SoC

by Qiang Yu

Currently, the cfg_1_9_0 which is being used for X1E80100 has config_sid callback in its ops and doesn't disable ASPM L0s. However, as same as SC8280X, PCIe controllers on X1E80100 are connected to SMMUv3, hence don't need config_sid() callback and hardware team has recommended to disable L0s as it is broken in the controller. Hence reuse cfg_sc8280xp for X1E80100. Fixes: 6d0c39324c5f ("PCI: qcom: Add X1E80100 PCIe support") Cc: stable(a)vger.kernel.org Signed-off-by: Qiang Yu <quic_qianyu(a)quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov(a)linaro.org> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam(a)linaro.org> --- drivers/pci/controller/dwc/pcie-qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 468bd4242e61..c533e6024ba2 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -1847,7 +1847,7 @@ static const struct of_device_id qcom_pcie_match[] = { { .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 }, { .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 }, { .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 }, - { .compatible = "qcom,pcie-x1e80100", .data = &cfg_1_9_0 }, + { .compatible = "qcom,pcie-x1e80100", .data = &cfg_sc8280xp }, { } }; -- 2.34.1

7 months, 1 week

4
9
0 0

[PATCH 6.1/6.6] drm/amd/display: Add null checks for 'stream' and 'plane' before dereferencing

by Xiangyu Chen

From: Xiangyu Chen <xiangyu.chen(a)eng.windriver.com> [ Upstream commit 15c2990e0f0108b9c3752d7072a97d45d4283aea ] This commit adds null checks for the 'stream' and 'plane' variables in the dcn30_apply_idle_power_optimizations function. These variables were previously assumed to be null at line 922, but they were used later in the code without checking if they were null. This could potentially lead to a null pointer dereference, which would cause a crash. The null checks ensure that 'stream' and 'plane' are not null before they are used, preventing potential crashes. Fixes the below static smatch checker: drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn30/dcn30_hwseq.c:938 dcn30_apply_idle_power_optimizations() error: we previously assumed 'stream' could be null (see line 922) drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn30/dcn30_hwseq.c:940 dcn30_apply_idle_power_optimizations() error: we previously assumed 'plane' could be null (see line 922) Cc: Tom Chung <chiahsuan.chung(a)amd.com> Cc: Nicholas Kazlauskas <nicholas.kazlauskas(a)amd.com> Cc: Bhawanpreet Lakha <Bhawanpreet.Lakha(a)amd.com> Cc: Rodrigo Siqueira <Rodrigo.Siqueira(a)amd.com> Cc: Roman Li <roman.li(a)amd.com> Cc: Hersen Wu <hersenxs.wu(a)amd.com> Cc: Alex Hung <alex.hung(a)amd.com> Cc: Aurabindo Pillai <aurabindo.pillai(a)amd.com> Cc: Harry Wentland <harry.wentland(a)amd.com> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam(a)amd.com> Reviewed-by: Aurabindo Pillai <aurabindo.pillai(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> [Xiangyu: Modified file path to backport this commit] Signed-off-by: Xiangyu Chen <xiangyu.chen(a)windriver.com> --- drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c index 407f7889e8fd..7a643690fdc7 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c @@ -762,6 +762,9 @@ bool dcn30_apply_idle_power_optimizations(struct dc *dc, bool enable) stream = dc->current_state->streams[0]; plane = (stream ? dc->current_state->stream_status[0].plane_states[0] : NULL); + if (!stream || !plane) + return false; + if (stream && plane) { cursor_cache_enable = stream->cursor_position.enable && plane->address.grph.cursor_cache_addr.quad_part; -- 2.43.0

7 months, 1 week

1
0
0 0

[PATCH RESEND] drivers:media:radio: Fix atomicity violation in fmc_send_cmd()

by Qiu-ji Chen

Atomicity violation occurs when the fmc_send_cmd() function is executed simultaneously with the modification of the fmdev->resp_skb value. Consider a scenario where, after passing the validity check within the function, a non-null fmdev->resp_skb variable is assigned a null value. This results in an invalid fmdev->resp_skb variable passing the validity check. As seen in the later part of the function, skb = fmdev->resp_skb; when the invalid fmdev->resp_skb passes the check, a null pointer dereference error may occur at line 478, evt_hdr = (void *)skb->data; To address this issue, it is recommended to include the validity check of fmdev->resp_skb within the locked section of the function. This modification ensures that the value of fmdev->resp_skb does not change during the validation process, thereby maintaining its validity. This possible bug is found by an experimental static analysis tool developed by our team. This tool analyzes the locking APIs to extract function pairs that can be concurrently executed, and then analyzes the instructions in the paired functions to identify possible concurrency bugs including data races and atomicity violations. Fixes: e8454ff7b9a4 ("[media] drivers:media:radio: wl128x: FM Driver Common sources") Cc: stable(a)vger.kernel.org Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com> --- drivers/media/radio/wl128x/fmdrv_common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/radio/wl128x/fmdrv_common.c index 3d36f323a8f8..4d032436691c 100644 --- a/drivers/media/radio/wl128x/fmdrv_common.c +++ b/drivers/media/radio/wl128x/fmdrv_common.c @@ -466,11 +466,12 @@ int fmc_send_cmd(struct fmdev *fmdev, u8 fm_op, u16 type, void *payload, jiffies_to_msecs(FM_DRV_TX_TIMEOUT) / 1000); return -ETIMEDOUT; } + spin_lock_irqsave(&fmdev->resp_skb_lock, flags); if (!fmdev->resp_skb) { + spin_unlock_irqrestore(&fmdev->resp_skb_lock, flags); fmerr("Response SKB is missing\n"); return -EFAULT; } - spin_lock_irqsave(&fmdev->resp_skb_lock, flags); skb = fmdev->resp_skb; fmdev->resp_skb = NULL; spin_unlock_irqrestore(&fmdev->resp_skb_lock, flags); -- 2.34.1

7 months, 1 week

2
1
0 0

[PATCH v2] virtio: console: Fix atomicity violation in fill_readbuf()

by Qiu-ji Chen

The atomicity violation issue is due to the invalidation of the function port_has_data()'s check caused by concurrency. Imagine a scenario where a port that contains data passes the validity check but is simultaneously assigned a value with no data. This could result in an empty port passing the validity check, potentially leading to a null pointer dereference error later in the program, which is inconsistent. To address this issue, we added a separate validity check for the variable buf after its assignment. This ensures that an invalid buf does not proceed further into the program, thereby preventing a null pointer dereference error. This possible bug is found by an experimental static analysis tool developed by our team. This tool analyzes the locking APIs to extract function pairs that can be concurrently executed, and then analyzes the instructions in the paired functions to identify possible concurrency bugs including data races and atomicity violations. Fixes: 203baab8ba31 ("virtio: console: Introduce function to hand off data from host to readers") Cc: stable(a)vger.kernel.org Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com> --- V2: The logic of the fix has been modified. --- drivers/char/virtio_console.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index c62b208b42f1..54fee192d93c 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -660,6 +660,10 @@ static ssize_t fill_readbuf(struct port *port, u8 __user *out_buf, return 0; buf = port->inbuf; + + if (!buf) + return 0; + out_count = min(out_count, buf->len - buf->offset); if (to_user) { -- 2.34.1

7 months, 1 week

1
0
0 0

[PATCH v2] PM / devfreq: Fix atomicity violation in devfreq_update_interval()

by Qiu-ji Chen

The atomicity violation occurs when the variables cur_delay and new_delay are defined. Imagine a scenario where, while defining cur_delay and new_delay, the values stored in devfreq->profile->polling_ms and the delay variable change. After acquiring the mutex_lock and entering the critical section, due to possible concurrent modifications, cur_delay and new_delay may no longer represent the correct values. Subsequent usage, such as if (cur_delay > new_delay), could cause the program to run incorrectly, resulting in inconsistencies. If the read of devfreq->profile->polling_ms is not protected by the lock, the cur_delay that enters the critical section would not store the actual old value of devfreq->profile->polling_ms, which would affect the subsequent checks like if (!cur_delay) and if (cur_delay > new_delay), potentially causing the driver to perform incorrect operations. We believe that moving the read of devfreq->profile->polling_ms inside the lock is beneficial as it ensures that cur_delay stores the true old value of devfreq->profile->polling_ms, ensuring the correctness of the later checks. To address this issue, it is recommended to acquire a lock in advance, ensuring that devfreq->profile->polling_ms and delay are protected by the lock when being read. This will help ensure the consistency of the program. This possible bug is found by an experimental static analysis tool developed by our team. This tool analyzes the locking APIs to extract function pairs that can be concurrently executed, and then analyzes the instructions in the paired functions to identify possible concurrency bugs including data races and atomicity violations. Fixes: 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices which can idle") Cc: stable(a)vger.kernel.org Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com> --- V2: Added some descriptions to reduce misunderstandings. Thanks to MyungJoo Ham for suggesting this improvement. --- drivers/devfreq/devfreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 98657d3b9435..9634739fc9cb 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -616,10 +616,10 @@ EXPORT_SYMBOL(devfreq_monitor_resume); */ void devfreq_update_interval(struct devfreq *devfreq, unsigned int *delay) { + mutex_lock(&devfreq->lock); unsigned int cur_delay = devfreq->profile->polling_ms; unsigned int new_delay = *delay; - mutex_lock(&devfreq->lock); devfreq->profile->polling_ms = new_delay; if (IS_SUPPORTED_FLAG(devfreq->governor->flags, IRQ_DRIVEN)) -- 2.34.1

7 months, 1 week

1
0
0 0

[PATCH v2] drm/vc4: Fix atomicity violation in vc4_crtc_send_vblank()

by Qiu-ji Chen

An atomicity violation occurs when the vc4_crtc_send_vblank function executes simultaneously with modifications to crtc->state->event. Consider a scenario where crtc->state->event is non-null, allowing it to pass the validity check. However, at the same time, crtc->state->event might be set to null. In this case, the validity check in vc4_crtc_send_vblank might act on the old crtc->state->event (before locking), allowing invalid values to pass the validity check, which could lead to a null pointer dereference. In the drm_device structure, it is mentioned: "@event_lock: Protects @vblank_event_list and event delivery in general." I believe that the validity check and the subsequent null assignment operation are part of the event delivery process, and all of these should be protected by the event_lock. If there is no lock protection before the validity check, it is possible for a null crtc->state->event to be passed into the drm_crtc_send_vblank_event() function, leading to a null pointer dereference error. We have observed its callers and found that they are from the drm_crtc_helper_funcs driver interface. We believe that functions within driver interfaces can be concurrent, potentially causing a data race on crtc->state->event. To address this issue, it is recommended to include the validity check of crtc->state and crtc->state->event within the locking section of the function. This modification ensures that the values of crtc->state->event and crtc->state do not change during the validation process, maintaining their valid conditions. This possible bug is found by an experimental static analysis tool developed by our team. This tool analyzes the locking APIs to extract function pairs that can be concurrently executed, and then analyzes the instructions in the paired functions to identify possible concurrency bugs including data races and atomicity violations. Fixes: 68e4a69aec4d ("drm/vc4: crtc: Create vblank reporting function") Cc: stable(a)vger.kernel.org Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com> --- V2: The description of the patch has been modified to make it clearer. Thanks to Simona Vetter for suggesting this improvement. --- drivers/gpu/drm/vc4/vc4_crtc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 8b5a7e5eb146..98885f519827 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -575,10 +575,12 @@ void vc4_crtc_send_vblank(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; unsigned long flags; - if (!crtc->state || !crtc->state->event) + spin_lock_irqsave(&dev->event_lock, flags); + if (!crtc->state || !crtc->state->event) { + spin_unlock_irqrestore(&dev->event_lock, flags); return; + } - spin_lock_irqsave(&dev->event_lock, flags); drm_crtc_send_vblank_event(crtc, crtc->state->event); crtc->state->event = NULL; spin_unlock_irqrestore(&dev->event_lock, flags); -- 2.34.1

7 months, 1 week

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror October 2024