From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Nicolas Dufresne <nicolas.dufresne@collabora.com>,
Hans Verkuil <hverkuil@xs4all.nl>,
Sasha Levin <sashal@kernel.org>,
ezequiel@vanguardiasur.com.ar, mchehab@kernel.org,
gregkh@linuxfoundation.org, linux-media@vger.kernel.org,
linux-rockchip@lists.infradead.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.1 26/58] media: rkvdec: Initialize the m2m context before the controls
Date: Sun, 1 Jun 2025 19:39:39 -0400 [thread overview]
Message-ID: <20250601234012.3516352-26-sashal@kernel.org> (raw)
In-Reply-To: <20250601234012.3516352-1-sashal@kernel.org>
From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
[ Upstream commit d43d7db3c8a1868dcbc6cb8de90a3cdf309d6cbb ]
Setting up the control handler calls into .s_ctrl ops. While validating
the controls the ops may need to access some of the context state, which
could lead to a crash if not properly initialized.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
Now let me analyze the specific changes proposed in the commit: ##
Analysis **YES** This commit should be backported to stable kernel
trees. Here's my extensive analysis: ### Core Issue Analysis The commit
addresses a critical initialization order bug that can lead to crashes.
The problem occurs in the `rkvdec_open()` function at lines 822-830 in
the current code: 1. **Current problematic order:** - Line 822:
`rkvdec_init_ctrls(ctx)` is called first - Line 826-830:
`v4l2_m2m_ctx_init()` is called second 2. **The problem:** According to
the commit message and my analysis of the kernel documentation,
`v4l2_ctrl_handler_setup()` (called inside `rkvdec_init_ctrls()`) calls
`.s_ctrl` for all controls unconditionally to initialize hardware to
default values. The `.s_ctrl` handlers may need to access the m2m
context state, but if the m2m context (`ctx->fh.m2m_ctx`) isn't
initialized yet, this can cause a crash or undefined behavior. ### Code
Changes Analysis The fix is minimal and surgical: - **Lines moved:** The
initialization order is swapped - m2m context initialization moves
before control handler setup - **Error handling updated:** The error
handling paths are correctly updated to match the new initialization
order - **No functional changes:** The fix doesn't change driver
functionality, only initialization sequence ### Why This Should Be
Backported 1. **Fixes a real crash bug:** This addresses a potential
crash scenario that affects users 2. **Small and contained:** The change
is minimal - just reordering initialization and updating error paths 3.
**Low regression risk:** Moving m2m init before control init is safer
since m2m context doesn't depend on controls, but controls may depend on
m2m context 4. **Follows established patterns:** Similar commit #2 in
the historical examples shows a "YES" backport for proper initialization
order in V4L2 drivers 5. **Critical subsystem:** Media drivers crashing
can affect user applications ### Comparison with Historical Commits -
**Similar to commit #2 (YES):** Also fixes initialization order in V4L2
media driver to prevent crashes - **Similar to commit #4 (YES):** Media
driver fix that's low-risk and contained - **Unlike commits #1, #3, #5
(NO):** This actually fixes a bug rather than just cleanup/code style
improvements ### Stable Tree Criteria Met ✅ **Important bugfix:**
Prevents crashes ✅ **Minimal risk:** Simple reordering with no side
effects ✅ **Small change:** Only affects initialization sequence ✅ **No
architectural changes:** Pure bug fix ✅ **Confined to subsystem:** Only
affects rkvdec driver The fix follows the stable tree rules perfectly -
it's an important crash prevention fix with minimal regression risk,
making it an ideal backport candidate.
drivers/staging/media/rkvdec/rkvdec.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
index d16cf4115d03a..6cf551cc861a4 100644
--- a/drivers/staging/media/rkvdec/rkvdec.c
+++ b/drivers/staging/media/rkvdec/rkvdec.c
@@ -821,24 +821,24 @@ static int rkvdec_open(struct file *filp)
rkvdec_reset_decoded_fmt(ctx);
v4l2_fh_init(&ctx->fh, video_devdata(filp));
- ret = rkvdec_init_ctrls(ctx);
- if (ret)
- goto err_free_ctx;
-
ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(rkvdec->m2m_dev, ctx,
rkvdec_queue_init);
if (IS_ERR(ctx->fh.m2m_ctx)) {
ret = PTR_ERR(ctx->fh.m2m_ctx);
- goto err_cleanup_ctrls;
+ goto err_free_ctx;
}
+ ret = rkvdec_init_ctrls(ctx);
+ if (ret)
+ goto err_cleanup_m2m_ctx;
+
filp->private_data = &ctx->fh;
v4l2_fh_add(&ctx->fh);
return 0;
-err_cleanup_ctrls:
- v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
+err_cleanup_m2m_ctx:
+ v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
err_free_ctx:
kfree(ctx);
--
2.39.5
WARNING: multiple messages have this Message-ID (diff)
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Nicolas Dufresne <nicolas.dufresne@collabora.com>,
Hans Verkuil <hverkuil@xs4all.nl>,
Sasha Levin <sashal@kernel.org>,
ezequiel@vanguardiasur.com.ar, mchehab@kernel.org,
gregkh@linuxfoundation.org, linux-media@vger.kernel.org,
linux-rockchip@lists.infradead.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.1 26/58] media: rkvdec: Initialize the m2m context before the controls
Date: Sun, 1 Jun 2025 19:39:39 -0400 [thread overview]
Message-ID: <20250601234012.3516352-26-sashal@kernel.org> (raw)
In-Reply-To: <20250601234012.3516352-1-sashal@kernel.org>
From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
[ Upstream commit d43d7db3c8a1868dcbc6cb8de90a3cdf309d6cbb ]
Setting up the control handler calls into .s_ctrl ops. While validating
the controls the ops may need to access some of the context state, which
could lead to a crash if not properly initialized.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
Now let me analyze the specific changes proposed in the commit: ##
Analysis **YES** This commit should be backported to stable kernel
trees. Here's my extensive analysis: ### Core Issue Analysis The commit
addresses a critical initialization order bug that can lead to crashes.
The problem occurs in the `rkvdec_open()` function at lines 822-830 in
the current code: 1. **Current problematic order:** - Line 822:
`rkvdec_init_ctrls(ctx)` is called first - Line 826-830:
`v4l2_m2m_ctx_init()` is called second 2. **The problem:** According to
the commit message and my analysis of the kernel documentation,
`v4l2_ctrl_handler_setup()` (called inside `rkvdec_init_ctrls()`) calls
`.s_ctrl` for all controls unconditionally to initialize hardware to
default values. The `.s_ctrl` handlers may need to access the m2m
context state, but if the m2m context (`ctx->fh.m2m_ctx`) isn't
initialized yet, this can cause a crash or undefined behavior. ### Code
Changes Analysis The fix is minimal and surgical: - **Lines moved:** The
initialization order is swapped - m2m context initialization moves
before control handler setup - **Error handling updated:** The error
handling paths are correctly updated to match the new initialization
order - **No functional changes:** The fix doesn't change driver
functionality, only initialization sequence ### Why This Should Be
Backported 1. **Fixes a real crash bug:** This addresses a potential
crash scenario that affects users 2. **Small and contained:** The change
is minimal - just reordering initialization and updating error paths 3.
**Low regression risk:** Moving m2m init before control init is safer
since m2m context doesn't depend on controls, but controls may depend on
m2m context 4. **Follows established patterns:** Similar commit #2 in
the historical examples shows a "YES" backport for proper initialization
order in V4L2 drivers 5. **Critical subsystem:** Media drivers crashing
can affect user applications ### Comparison with Historical Commits -
**Similar to commit #2 (YES):** Also fixes initialization order in V4L2
media driver to prevent crashes - **Similar to commit #4 (YES):** Media
driver fix that's low-risk and contained - **Unlike commits #1, #3, #5
(NO):** This actually fixes a bug rather than just cleanup/code style
improvements ### Stable Tree Criteria Met ✅ **Important bugfix:**
Prevents crashes ✅ **Minimal risk:** Simple reordering with no side
effects ✅ **Small change:** Only affects initialization sequence ✅ **No
architectural changes:** Pure bug fix ✅ **Confined to subsystem:** Only
affects rkvdec driver The fix follows the stable tree rules perfectly -
it's an important crash prevention fix with minimal regression risk,
making it an ideal backport candidate.
drivers/staging/media/rkvdec/rkvdec.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
index d16cf4115d03a..6cf551cc861a4 100644
--- a/drivers/staging/media/rkvdec/rkvdec.c
+++ b/drivers/staging/media/rkvdec/rkvdec.c
@@ -821,24 +821,24 @@ static int rkvdec_open(struct file *filp)
rkvdec_reset_decoded_fmt(ctx);
v4l2_fh_init(&ctx->fh, video_devdata(filp));
- ret = rkvdec_init_ctrls(ctx);
- if (ret)
- goto err_free_ctx;
-
ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(rkvdec->m2m_dev, ctx,
rkvdec_queue_init);
if (IS_ERR(ctx->fh.m2m_ctx)) {
ret = PTR_ERR(ctx->fh.m2m_ctx);
- goto err_cleanup_ctrls;
+ goto err_free_ctx;
}
+ ret = rkvdec_init_ctrls(ctx);
+ if (ret)
+ goto err_cleanup_m2m_ctx;
+
filp->private_data = &ctx->fh;
v4l2_fh_add(&ctx->fh);
return 0;
-err_cleanup_ctrls:
- v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
+err_cleanup_m2m_ctx:
+ v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
err_free_ctx:
kfree(ctx);
--
2.39.5
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2025-06-01 23:41 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-01 23:39 [PATCH AUTOSEL 6.1 01/58] drm/amdgpu/gfx6: fix CSIB handling Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 02/58] drm/dp: add option to disable zero sized address only transactions Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 03/58] sunrpc: update nextcheck time when adding new cache entries Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 04/58] drm/amd/display: DCN32 null data check Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 05/58] drm/bridge: analogix_dp: Add irq flag IRQF_NO_AUTOEN instead of calling disable_irq() Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 06/58] exfat: fix double free in delayed_free Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 07/58] arm64/cpuinfo: only show one cpu's info in c_show() Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 08/58] drm/bridge: anx7625: change the gpiod_set_value API Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 09/58] drm/amdgpu/gfx11: fix CSIB handling Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 10/58] media: i2c: imx334: Enable runtime PM before sub-device registration Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 11/58] drm/msm/hdmi: add runtime PM calls to DDC transfer function Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 12/58] media: uapi: v4l: Fix V4L2_TYPE_IS_OUTPUT condition Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 13/58] drm/amd/display: Add NULL pointer checks in dm_force_atomic_commit() Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 14/58] drm/amd/display: Skip to enable dsc if it has been off Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 15/58] drm/msm/a6xx: Increase HFI response timeout Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 16/58] media: i2c: imx334: Fix runtime PM handling in remove function Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 17/58] drm/amdgpu/gfx10: fix CSIB handling Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 18/58] drm: panel-orientation-quirks: Add ZOTAC Gaming Zone Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 19/58] media: ccs-pll: Better validate VT PLL branch Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 20/58] media: uapi: v4l: Change V4L2_TYPE_IS_CAPTURE condition Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 21/58] drm/amdgpu/gfx7: fix CSIB handling Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 22/58] ext4: ext4: unify EXT4_EX_NOCACHE|NOFAIL flags in ext4_ext_remove_space() Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 23/58] jfs: fix array-index-out-of-bounds read in add_missing_indices Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 24/58] media: ti: cal: Fix wrong goto on error path Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 25/58] media: rkvdec: h264: Use bytesperline and buffer height as virstride Sasha Levin
2025-06-01 23:39 ` Sasha Levin
2025-06-01 23:39 ` Sasha Levin [this message]
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 26/58] media: rkvdec: Initialize the m2m context before the controls Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 27/58] sunrpc: fix race in cache cleanup causing stale nextcheck time Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 28/58] ext4: prevent stale extent cache entries caused by concurrent get es_cache Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 29/58] drm/amdgpu/gfx8: fix CSIB handling Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 30/58] drm/amdgpu/gfx9: " Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 31/58] jfs: Fix null-ptr-deref in jfs_ioc_trim Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 32/58] drm/amd/display: Correct prefetch calculation Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 33/58] drm/msm/dpu: don't select single flush for active CTL blocks Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 34/58] drm/amdkfd: Set SDMA_RLCx_IB_CNTL/SWITCH_INSIDE_IB Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 35/58] media: tc358743: ignore video while HPD is low Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 36/58] media: platform: exynos4-is: Add hardware sync wait to fimc_is_hw_change_mode() Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 37/58] media: i2c: imx334: update mode_3840x2160_regs array Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 38/58] nios2: force update_mmu_cache on spurious tlb-permission--related pagefaults Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 39/58] media: rcar-vin: Fix stride setting for RAW8 formats Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 40/58] media: qcom: venus: Fix uninitialized variable warning Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 41/58] ACPI: bus: Bail out if acpi_kobj registration fails Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 42/58] pmdomain: ti: Fix STANDBY handling of PER power domain Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 43/58] PM: runtime: fix denying of auto suspend in pm_suspend_timer_fn() Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 44/58] ASoC: amd: yc: Add quirk for Lenovo Yoga Pro 7 14ASP9 Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 45/58] thermal/drivers/qcom/tsens: Update conditions to strictly evaluate for IP v2+ Sasha Levin
2025-06-01 23:39 ` [PATCH AUTOSEL 6.1 46/58] clocksource/drivers/timer-tegra186: Fix watchdog self-pinging Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 47/58] gpio: pxa: Make irq_chip immutable Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 48/58] gpio: grgpio: " Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 49/58] gpio: xgene-sb: " Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 50/58] mmc: Add quirk to disable DDR50 tuning Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 51/58] clocksource: Fix the CPUs' choice in the watchdog per CPU verification Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 52/58] ACPICA: Avoid sequence overread in call to strncmp() Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 53/58] ACPICA: utilities: Fix overflow check in vsnprintf() Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 54/58] ACPI: EC: Add device to acpi_ec_no_wakeup[] qurik list Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 55/58] ALSA: seq: Remove unused snd_seq_queue_client_leave_cells Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 56/58] cpufreq: Force sync policy boost with global boost on sysfs update Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 57/58] power: supply: bq27xxx: Retrieve again when busy Sasha Levin
2025-06-01 23:40 ` [PATCH AUTOSEL 6.1 58/58] ASoC: tas2770: Power cycle amp on ISENSE/VSENSE change Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250601234012.3516352-26-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=ezequiel@vanguardiasur.com.ar \
--cc=gregkh@linuxfoundation.org \
--cc=hverkuil@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-staging@lists.linux.dev \
--cc=mchehab@kernel.org \
--cc=nicolas.dufresne@collabora.com \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.