linux-kernel - [PATCHv3 1/2] media: venus: destroy hfi session after m2m

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241224072444.2044956-2-senozhatsky@chromium.org>
Date: Tue, 24 Dec 2024 16:24:05 +0900
From: Sergey Senozhatsky <senozhatsky@...omium.org>
To: Stanimir Varbanov <stanimir.k.varbanov@...il.com>,
	Vikash Garodia <quic_vgarodia@...cinc.com>,
	"Bryan O'Donoghue" <bryan.odonoghue@...aro.org>,
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Hans Verkuil <hverkuil@...all.nl>,
	linux-media@...r.kernel.org,
	linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	stable@...r.kernel.org,
	Nathan Hebert <nhebert@...gle.com>
Subject: [PATCHv3 1/2] media: venus: destroy hfi session after m2m_ctx release

This partially reverts commit that made hfi_session_destroy()
the first step of vdec/venc close().  The reason being is a
regression report when, supposedly, encode/decoder is closed
with still active streaming (no ->stop_streaming() call before
close()) and pending pkts, so isr_thread cannot find instance
and fails to process those pending pkts.  This was the idea
behind the original patch - make it impossible to use instance
under destruction, because this is racy, but apparently there
are uses cases that depend on that unsafe pattern.  Return to
the old (unsafe) behaviour for the time being (until a better
fix is found).

Fixes: 45b1a1b348ec1 ("media: venus: sync with threaded IRQ during inst destruction")
Cc: stable@...r.kernel.org
Reported-by: Nathan Hebert <nhebert@...gle.com>
Signed-off-by: Sergey Senozhatsky <senozhatsky@...omium.org>
---
 drivers/media/platform/qcom/venus/core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index 2d27c5167246..807487a1f536 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -506,18 +506,14 @@ static __maybe_unused int venus_runtime_suspend(struct device *dev)
 void venus_close_common(struct venus_inst *inst)
 {
 	/*
-	 * First, remove the inst from the ->instances list, so that
-	 * to_instance() will return NULL.
-	 */
-	hfi_session_destroy(inst);
-	/*
-	 * Second, make sure we don't have IRQ/IRQ-thread currently running
+	 * Make sure we don't have IRQ/IRQ-thread currently running
 	 * or pending execution, which would race with the inst destruction.
 	 */
 	synchronize_irq(inst->core->irq);
 
 	v4l2_m2m_ctx_release(inst->m2m_ctx);
 	v4l2_m2m_release(inst->m2m_dev);
+	hfi_session_destroy(inst);
 	v4l2_fh_del(&inst->fh);
 	v4l2_fh_exit(&inst->fh);
 	v4l2_ctrl_handler_free(&inst->ctrl_handler);
-- 
2.47.1.613.gc27f4b7a9f-goog