[PATCH] media: venus: prevent potential integer overflow in decide_core()

Pavel Zhigulin posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/media/platform/qcom/venus/pm_helpers.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] media: venus: prevent potential integer overflow in decide_core()
Posted by Pavel Zhigulin 1 month, 1 week ago
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@kaspersky.com>
---
 drivers/media/platform/qcom/venus/pm_helpers.c | 8 ++++----
 1 file changed, 4 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..caaab097a04d 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, u64 *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,8 @@ 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;
+	u64 min_coreid, min_load, cur_inst_load;
+	u64 min_lp_coreid, 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

Re: [PATCH] media: venus: prevent potential integer overflow in decide_core()
Posted by Dikshita Agarwal 1 month, 1 week ago

On 10/6/2025 9:10 PM, 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@kaspersky.com>
> ---
>  drivers/media/platform/qcom/venus/pm_helpers.c | 8 ++++----
>  1 file changed, 4 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..caaab097a04d 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, u64 *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,8 @@ 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;
> +	u64 min_coreid, min_load, cur_inst_load;
> +	u64 min_lp_coreid, min_lp_load, cur_inst_lp_load;

why update type for min_coreid and min_lp_coreid, the number of cores will
never approach the u32 limit.

Thanks,
Dikshita
>  	struct hfi_videocores_usage_type cu;
>  	unsigned long max_freq = ULONG_MAX;
>  	struct device *dev = core->dev;
> --
> 2.43.0
> 
>