[PATCH v2] drm/meson: Use 1000ULL when operating with mode->clock

I Hsin Cheng posted 1 patch 7 months, 2 weeks ago
drivers/gpu/drm/meson/meson_encoder_hdmi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] drm/meson: Use 1000ULL when operating with mode->clock
Posted by I Hsin Cheng 7 months, 2 weeks ago
Coverity scan reported the usage of "mode->clock * 1000" may lead to
integer overflow. Use "1000ULL" instead of "1000"
when utilizing it to avoid potential integer overflow issue.

Link: https://scan5.scan.coverity.com/#/project-view/10074/10063?selectedIssue=1646759
Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
---
Changelog:

v1 -> v2:
	- Use 1000ULL instead of casting the type of "mode->clock"
	- Refine commit title and message
	- Fix the issue for the evaluation inside drm_mode_status
	  meson_encoder_hdmi_mode_valid() as well

Christophe,
Thanks for your review and your suggestion, I think I should add a tag
for you,too, but I'm not sure what should I add, if you would be so kind
please let me know how should I tag you in the patch.

Best regards,
I Hsin Cheng
---
 drivers/gpu/drm/meson/meson_encoder_hdmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 7752d8ac85f0..c08fa93e50a3 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -75,7 +75,7 @@ static void meson_encoder_hdmi_set_vclk(struct meson_encoder_hdmi *encoder_hdmi,
 	unsigned long long venc_freq;
 	unsigned long long hdmi_freq;
 
-	vclk_freq = mode->clock * 1000;
+	vclk_freq = mode->clock * 1000ULL;
 
 	/* For 420, pixel clock is half unlike venc clock */
 	if (encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
@@ -123,7 +123,7 @@ static enum drm_mode_status meson_encoder_hdmi_mode_valid(struct drm_bridge *bri
 	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
 	struct meson_drm *priv = encoder_hdmi->priv;
 	bool is_hdmi2_sink = display_info->hdmi.scdc.supported;
-	unsigned long long clock = mode->clock * 1000;
+	unsigned long long clock = mode->clock * 1000ULL;
 	unsigned long long phy_freq;
 	unsigned long long vclk_freq;
 	unsigned long long venc_freq;
-- 
2.43.0
Re: [PATCH v2] drm/meson: Use 1000ULL when operating with mode->clock
Posted by Neil Armstrong 7 months, 1 week ago
Hi,

On Tue, 06 May 2025 02:43:38 +0800, I Hsin Cheng wrote:
> Coverity scan reported the usage of "mode->clock * 1000" may lead to
> integer overflow. Use "1000ULL" instead of "1000"
> when utilizing it to avoid potential integer overflow issue.
> 
> 

Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-fixes)

[1/1] drm/meson: Use 1000ULL when operating with mode->clock
      https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/eb0851e14432f3b87c77b704c835ac376deda03a

-- 
Neil
Re: [PATCH v2] drm/meson: Use 1000ULL when operating with mode->clock
Posted by I Hsin Cheng 7 months, 1 week ago
On Fri, May 09, 2025 at 08:56:01PM +0200, Neil Armstrong wrote:
> Hi,
> 
> On Tue, 06 May 2025 02:43:38 +0800, I Hsin Cheng wrote:
> > Coverity scan reported the usage of "mode->clock * 1000" may lead to
> > integer overflow. Use "1000ULL" instead of "1000"
> > when utilizing it to avoid potential integer overflow issue.
> > 
> > 
> 
> Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-fixes)
> 
> [1/1] drm/meson: Use 1000ULL when operating with mode->clock
>       https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/eb0851e14432f3b87c77b704c835ac376deda03a
> 
> -- 
> Neil
>

Hi Neil, Hi Martin,

Thanks for your kindness for adding the tag for me, I'll make sure to
add Fixes tag in future when I send patches.

Big thanks!

Best regards,
I Hsin Cheng.
Re: [PATCH v2] drm/meson: Use 1000ULL when operating with mode->clock
Posted by neil.armstrong@linaro.org 7 months, 1 week ago
On 05/05/2025 20:43, I Hsin Cheng wrote:
> Coverity scan reported the usage of "mode->clock * 1000" may lead to
> integer overflow. Use "1000ULL" instead of "1000"
> when utilizing it to avoid potential integer overflow issue.
> 
> Link: https://scan5.scan.coverity.com/#/project-view/10074/10063?selectedIssue=1646759
> Signed-off-by: I Hsin Cheng <richard120310@gmail.com>

Could you add a Fixes tag ?

Thanks,
Neil

> ---
> Changelog:
> 
> v1 -> v2:
> 	- Use 1000ULL instead of casting the type of "mode->clock"
> 	- Refine commit title and message
> 	- Fix the issue for the evaluation inside drm_mode_status
> 	  meson_encoder_hdmi_mode_valid() as well
> 
> Christophe,
> Thanks for your review and your suggestion, I think I should add a tag
> for you,too, but I'm not sure what should I add, if you would be so kind
> please let me know how should I tag you in the patch.
> 
> Best regards,
> I Hsin Cheng
> ---
>   drivers/gpu/drm/meson/meson_encoder_hdmi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 7752d8ac85f0..c08fa93e50a3 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> @@ -75,7 +75,7 @@ static void meson_encoder_hdmi_set_vclk(struct meson_encoder_hdmi *encoder_hdmi,
>   	unsigned long long venc_freq;
>   	unsigned long long hdmi_freq;
>   
> -	vclk_freq = mode->clock * 1000;
> +	vclk_freq = mode->clock * 1000ULL;
>   
>   	/* For 420, pixel clock is half unlike venc clock */
>   	if (encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> @@ -123,7 +123,7 @@ static enum drm_mode_status meson_encoder_hdmi_mode_valid(struct drm_bridge *bri
>   	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
>   	struct meson_drm *priv = encoder_hdmi->priv;
>   	bool is_hdmi2_sink = display_info->hdmi.scdc.supported;
> -	unsigned long long clock = mode->clock * 1000;
> +	unsigned long long clock = mode->clock * 1000ULL;
>   	unsigned long long phy_freq;
>   	unsigned long long vclk_freq;
>   	unsigned long long venc_freq;
Re: [PATCH v2] drm/meson: Use 1000ULL when operating with mode->clock
Posted by Martin Blumenstingl 7 months, 1 week ago
On Fri, May 9, 2025 at 5:35 PM <neil.armstrong@linaro.org> wrote:
>
> On 05/05/2025 20:43, I Hsin Cheng wrote:
> > Coverity scan reported the usage of "mode->clock * 1000" may lead to
> > integer overflow. Use "1000ULL" instead of "1000"
> > when utilizing it to avoid potential integer overflow issue.
> >
> > Link: https://scan5.scan.coverity.com/#/project-view/10074/10063?selectedIssue=1646759
> > Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
>
> Could you add a Fixes tag ?
Fixes: 1017560164b6 ("drm/meson: use unsigned long long / Hz for
frequency types")

Neil, can you add this while applying or do we need a v3?


Best regards,
Martin
Re: [PATCH v2] drm/meson: Use 1000ULL when operating with mode->clock
Posted by Neil Armstrong 7 months, 1 week ago
On 09/05/2025 20:27, Martin Blumenstingl wrote:
> On Fri, May 9, 2025 at 5:35 PM <neil.armstrong@linaro.org> wrote:
>>
>> On 05/05/2025 20:43, I Hsin Cheng wrote:
>>> Coverity scan reported the usage of "mode->clock * 1000" may lead to
>>> integer overflow. Use "1000ULL" instead of "1000"
>>> when utilizing it to avoid potential integer overflow issue.
>>>
>>> Link: https://scan5.scan.coverity.com/#/project-view/10074/10063?selectedIssue=1646759
>>> Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
>>
>> Could you add a Fixes tag ?
> Fixes: 1017560164b6 ("drm/meson: use unsigned long long / Hz for
> frequency types")
> 
> Neil, can you add this while applying or do we need a v3?

Will add while applying.

Thanks,
Neil

> 
> 
> Best regards,
> Martin

Re: [PATCH v2] drm/meson: Use 1000ULL when operating with mode->clock
Posted by Martin Blumenstingl 7 months, 1 week ago
On Mon, May 5, 2025 at 8:43 PM I Hsin Cheng <richard120310@gmail.com> wrote:
>
> Coverity scan reported the usage of "mode->clock * 1000" may lead to
> integer overflow. Use "1000ULL" instead of "1000"
> when utilizing it to avoid potential integer overflow issue.
>
> Link: https://scan5.scan.coverity.com/#/project-view/10074/10063?selectedIssue=1646759
> Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>


Thank you and best regards,
Martin