| 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
| ||
|
Message-ID: <578305fc-fe93-2ef4-908a-d728a5ae6485@quicinc.com>
Date: Wed, 30 Apr 2025 16:10:44 +0530
From: Vikash Garodia <quic_vgarodia@...cinc.com>
To: Dikshita Agarwal <quic_dikshita@...cinc.com>,
Abhinav Kumar
<quic_abhinavk@...cinc.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Stefan Schmidt <stefan.schmidt@...aro.org>,
Hans Verkuil
<hverkuil@...all.nl>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio
<konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski
<krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>
CC: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Dmitry Baryshkov
<dmitry.baryshkov@....qualcomm.com>,
Neil Armstrong
<neil.armstrong@...aro.org>,
Nicolas Dufresne
<nicolas.dufresne@...labora.com>,
<linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
<20250417-topic-sm8x50-iris-v10-v7-0-f020cb1d0e98@...aro.org>,
<20250424-qcs8300_iris-v5-0-f118f505c300@...cinc.com>,
<stable@...r.kernel.org>
Subject: Re: [PATCH v2 17/23] media: iris: Track flush responses to prevent
premature completion
On 4/28/2025 2:59 PM, Dikshita Agarwal wrote:
> Currently, two types of flush commands are queued to the firmware,
> the first flush queued as part of sequence change, does not wait for a
> response, while the second flush queued as part of stop, expects a
> completion response before proceeding further.
>
> Due to timing issue, the flush response corresponding to the first
> command could arrive after the second flush is issued. This casuses the
> driver to incorrectly assume that the second flush has completed,
> leading to the premature signaling of flush_completion.
>
> To address this, introduce a counter to track the number of pending
> flush responses and signal flush completion only when all expected
> responses are received.
>
> Cc: stable@...r.kernel.org
> Fixes: 11712ce70f8e ("media: iris: implement vb2 streaming ops")
> Signed-off-by: Dikshita Agarwal <quic_dikshita@...cinc.com>
> ---
> .../media/platform/qcom/iris/iris_hfi_gen1_command.c | 4 +++-
> .../media/platform/qcom/iris/iris_hfi_gen1_response.c | 17 +++++++++++------
> drivers/media/platform/qcom/iris/iris_instance.h | 2 ++
> 3 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index f9f3e2d2ce29..ef3ca676d2ea 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -208,8 +208,10 @@ static int iris_hfi_gen1_session_stop(struct iris_inst *inst, u32 plane)
> flush_pkt.flush_type = flush_type;
>
> ret = iris_hfi_queue_cmd_write(core, &flush_pkt, flush_pkt.shdr.hdr.size);
> - if (!ret)
> + if (!ret) {
> + inst->flush_responses_pending++;
> ret = iris_wait_for_session_response(inst, true);
> + }
> }
>
> return ret;
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> index dfca45d85759..01338baf3788 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> @@ -207,7 +207,8 @@ static void iris_hfi_gen1_event_seq_changed(struct iris_inst *inst,
> flush_pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_FLUSH;
> flush_pkt.shdr.session_id = inst->session_id;
> flush_pkt.flush_type = HFI_FLUSH_OUTPUT;
> - iris_hfi_queue_cmd_write(inst->core, &flush_pkt, flush_pkt.shdr.hdr.size);
> + if (!iris_hfi_queue_cmd_write(inst->core, &flush_pkt, flush_pkt.shdr.hdr.size))
> + inst->flush_responses_pending++;
> }
>
> iris_vdec_src_change(inst);
> @@ -408,7 +409,9 @@ static void iris_hfi_gen1_session_ftb_done(struct iris_inst *inst, void *packet)
> flush_pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_FLUSH;
> flush_pkt.shdr.session_id = inst->session_id;
> flush_pkt.flush_type = HFI_FLUSH_OUTPUT;
> - iris_hfi_queue_cmd_write(core, &flush_pkt, flush_pkt.shdr.hdr.size);
> + if (!iris_hfi_queue_cmd_write(core, &flush_pkt, flush_pkt.shdr.hdr.size))
> + inst->flush_responses_pending++;
> +
> iris_inst_sub_state_change_drain_last(inst);
>
> return;
> @@ -570,7 +573,6 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
> const struct iris_hfi_gen1_response_pkt_info *pkt_info;
> struct device *dev = core->dev;
> struct hfi_session_pkt *pkt;
> - struct completion *done;
> struct iris_inst *inst;
> bool found = false;
> u32 i;
> @@ -631,9 +633,12 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
> if (shdr->error_type != HFI_ERR_NONE)
> iris_inst_change_state(inst, IRIS_INST_ERROR);
>
> - done = pkt_info->pkt == HFI_MSG_SESSION_FLUSH ?
> - &inst->flush_completion : &inst->completion;
> - complete(done);
> + if (pkt_info->pkt == HFI_MSG_SESSION_FLUSH) {
> + if (--inst->flush_responses_pending <= 0)
No need to check for < 0 condition as its an unsigned int. Signal when equals 0.
With above change, mark
Reviewed-by: Vikash Garodia <quic_vgarodia@...cinc.com>
> + complete(&inst->flush_completion);
> + } else {
> + complete(&inst->completion);
> + }
> }
> mutex_unlock(&inst->lock);
>
> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
> index 5150237f0020..9ed197799ee7 100644
> --- a/drivers/media/platform/qcom/iris/iris_instance.h
> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
> @@ -27,6 +27,7 @@
> * @crop: structure of crop info
> * @completion: structure of signal completions
> * @flush_completion: structure of signal completions for flush cmd
> + * @flush_responses_pending: counter to track number of pending flush responses
> * @fw_caps: array of supported instance firmware capabilities
> * @buffers: array of different iris buffers
> * @fw_min_count: minimnum count of buffers needed by fw
> @@ -59,6 +60,7 @@ struct iris_inst {
> struct iris_hfi_rect_desc crop;
> struct completion completion;
> struct completion flush_completion;
> + u32 flush_responses_pending;
> struct platform_inst_fw_cap fw_caps[INST_FW_CAP_MAX];
> struct iris_buffers buffers[BUF_TYPE_MAX];
> u32 fw_min_count;
>
Powered by blists - more mailing lists