[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <41899b79-7b12-4f94-a55a-7d365336a147@linaro.org>
Date: Tue, 7 Oct 2025 12:38:08 +0100
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Pavel Zhigulin <Pavel.Zhigulin@...persky.com>,
Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>
Cc: Vikash Garodia <vikash.garodia@....qualcomm.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Stanimir Varbanov <stanimir.varbanov@...aro.org>,
linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH v2] media: venus: prevent potential integer overflow in
decide_core()
On 07/10/2025 11:30, Pavel Zhigulin wrote:
> The function 'decide_core()' contains the following code:
>
> cur_inst_load = load_per_instance(inst);
> cur_inst_load *= inst->clk_data.vpp_freq;
> ...
> cur_inst_lp_load = load_per_instance(inst);
> cur_inst_lp_load *= inst->clk_data.low_power_freq;
>
> This can lead to an integer overflow because the variables
> 'cur_inst_load' and 'cur_inst_lp_load' are of type u32.
>
> The overflow can occur in the following scenario:
>
> 1. The current FPS is 240 (VENUS_MAX_FPS constant).
> The processed image frame has a resolution of 4096x4096 pixels.
> 2. According to 'codec_freq_data':
> - 'inst->clk_data.low_power_freq' can be up to 320
> - 'inst->clk_data.vpp_freq' can be up to 675
> (see drivers/media/platform/qcom/venus/hfi_platform_v4.c
> and drivers/media/platform/qcom/venus/hfi_platform_v6.c)
> 3. 'load_per_instance()' returns 15728640 under these conditions.
> 4. As a result:
> cur_inst_load *= inst->clk_data.vpp_freq → 10616832000
> cur_inst_lp_load *= inst->clk_data.low_power_freq → 5033164800
>
> The proposed fix changes the type of these variables from u32 to u64
> to prevent overflow.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 3cfe5815ce0e ("media: venus: Enable low power setting for encoder")
> Signed-off-by: Pavel Zhigulin <Pavel.Zhigulin@...persky.com>
> ---
> v2: Revert min_coreid and min_lp_coreid to u32 as
> Dikshita Agarwal <dikshita.agarwal@....qualcomm.com> suggested
> during review
> v1: https://lore.kernel.org/all/20251006154041.1804800-1-Pavel.Zhigulin@kaspersky.com/
> drivers/media/platform/qcom/venus/pm_helpers.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index f0269524ac70..eec49590e806 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -582,9 +582,9 @@ static int move_core_to_power_save_mode(struct venus_core *core,
> }
>
> static void
> -min_loaded_core(struct venus_inst *inst, u32 *min_coreid, u32 *min_load, bool low_power)
> +min_loaded_core(struct venus_inst *inst, u32 *min_coreid, u64 *min_load, bool low_power)
> {
> - u32 mbs_per_sec, load, core1_load = 0, core2_load = 0;
> + u64 mbs_per_sec, load, core1_load = 0, core2_load = 0;
> u32 cores_max = core_num_max(inst);
> struct venus_core *core = inst->core;
> struct venus_inst *inst_pos;
> @@ -639,8 +639,9 @@ static int decide_core(struct venus_inst *inst)
> {
> const u32 ptype = HFI_PROPERTY_CONFIG_VIDEOCORES_USAGE;
> struct venus_core *core = inst->core;
> - u32 min_coreid, min_load, cur_inst_load;
> - u32 min_lp_coreid, min_lp_load, cur_inst_lp_load;
> + u32 min_coreid, min_lp_coreid;
> + u64 min_load, cur_inst_load;
> + u64 min_lp_load, cur_inst_lp_load;
> struct hfi_videocores_usage_type cu;
> unsigned long max_freq = ULONG_MAX;
> struct device *dev = core->dev;
> --
> 2.43.0
>
Contingent on passing tests.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Powered by blists - more mailing lists