All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ziqi Zhao <astrajoan@yahoo.com>
To: syzbot+622bba18029bcde672e1@syzkaller.appspotmail.com,
	astrajoan@yahoo.com, jani.nikula@linux.intel.com,
	airlied@gmail.com, daniel@ffwll.ch,
	dri-devel@lists.freedesktop.org, ivan.orlov0322@gmail.com,
	maarten.lankhorst@linux.intel.com, mripard@kernel.org,
	skhan@linuxfoundation.org, tzimmermann@suse.de
Cc: netdev@vger.kernel.org, dsahern@kernel.org,
	syzkaller-bugs@googlegroups.com, linux-kernel@vger.kernel.org,
	edumazet@google.com, jiri@nvidia.com, jacob.e.keller@intel.com,
	kuba@kernel.org, pabeni@redhat.com, davem@davemloft.net
Subject: [PATCH v3] drm/modes: Fix division by zero due to overflow
Date: Wed,  2 Aug 2023 10:47:46 -0700	[thread overview]
Message-ID: <20230802174746.2256-1-astrajoan@yahoo.com> (raw)
In-Reply-To: <00000000000034cf5d05fea52dd4@google.com>

In the bug reported by Syzbot, the variable `den == (1 << 22)` and
`mode->vscan == (1 << 10)`, causing the multiplication to overflow and
accidentally make `den == 0`. To prevent any chance of overflow, we
replace `num` and `den` with 64-bit unsigned integers, and explicitly
check if the divisor `den` will overflow. If so, we employ full 64-bit
division with rounding; otherwise we keep the 64-bit to 32-bit division
that could potentially be better optimized.

In order to minimize the performance overhead, the overflow check for
`den` is wrapped with an `unlikely` condition. Please let me know if
this usage is appropriate.

Reported-by: syzbot+622bba18029bcde672e1@syzkaller.appspotmail.com
Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
---
V1 -> V2: address style comments suggested by Jani Nikula
<jani.nikula@linux.intel.com>
V2 -> V3: change title to include context on overflow causing the
division by zero

 drivers/gpu/drm/drm_modes.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ac9a406250c5..137101960690 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1285,13 +1285,13 @@ EXPORT_SYMBOL(drm_mode_set_name);
  */
 int drm_mode_vrefresh(const struct drm_display_mode *mode)
 {
-	unsigned int num, den;
+	u64 num, den;
 
 	if (mode->htotal == 0 || mode->vtotal == 0)
 		return 0;
 
-	num = mode->clock;
-	den = mode->htotal * mode->vtotal;
+	num = mul_u32_u32(mode->clock, 1000);
+	den = mul_u32_u32(mode->htotal, mode->vtotal);
 
 	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
 		num *= 2;
@@ -1300,7 +1300,10 @@ int drm_mode_vrefresh(const struct drm_display_mode *mode)
 	if (mode->vscan > 1)
 		den *= mode->vscan;
 
-	return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(num, 1000), den);
+	if (unlikely(den > UINT_MAX))
+		return DIV64_U64_ROUND_CLOSEST(num, den);
+
+	return DIV_ROUND_CLOSEST_ULL(num, (u32) den);
 }
 EXPORT_SYMBOL(drm_mode_vrefresh);
 
-- 
2.34.1


WARNING: multiple messages have this Message-ID (diff)
From: Ziqi Zhao <astrajoan@yahoo.com>
To: syzbot+622bba18029bcde672e1@syzkaller.appspotmail.com,
	astrajoan@yahoo.com, jani.nikula@linux.intel.com,
	airlied@gmail.com, daniel@ffwll.ch,
	dri-devel@lists.freedesktop.org, ivan.orlov0322@gmail.com,
	maarten.lankhorst@linux.intel.com, mripard@kernel.org,
	skhan@linuxfoundation.org, tzimmermann@suse.de
Cc: davem@davemloft.net, dsahern@kernel.org, edumazet@google.com,
	jacob.e.keller@intel.com, jiri@nvidia.com, kuba@kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	pabeni@redhat.com, syzkaller-bugs@googlegroups.com
Subject: [PATCH v3] drm/modes: Fix division by zero due to overflow
Date: Wed,  2 Aug 2023 10:47:46 -0700	[thread overview]
Message-ID: <20230802174746.2256-1-astrajoan@yahoo.com> (raw)
In-Reply-To: <00000000000034cf5d05fea52dd4@google.com>

In the bug reported by Syzbot, the variable `den == (1 << 22)` and
`mode->vscan == (1 << 10)`, causing the multiplication to overflow and
accidentally make `den == 0`. To prevent any chance of overflow, we
replace `num` and `den` with 64-bit unsigned integers, and explicitly
check if the divisor `den` will overflow. If so, we employ full 64-bit
division with rounding; otherwise we keep the 64-bit to 32-bit division
that could potentially be better optimized.

In order to minimize the performance overhead, the overflow check for
`den` is wrapped with an `unlikely` condition. Please let me know if
this usage is appropriate.

Reported-by: syzbot+622bba18029bcde672e1@syzkaller.appspotmail.com
Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
---
V1 -> V2: address style comments suggested by Jani Nikula
<jani.nikula@linux.intel.com>
V2 -> V3: change title to include context on overflow causing the
division by zero

 drivers/gpu/drm/drm_modes.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ac9a406250c5..137101960690 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1285,13 +1285,13 @@ EXPORT_SYMBOL(drm_mode_set_name);
  */
 int drm_mode_vrefresh(const struct drm_display_mode *mode)
 {
-	unsigned int num, den;
+	u64 num, den;
 
 	if (mode->htotal == 0 || mode->vtotal == 0)
 		return 0;
 
-	num = mode->clock;
-	den = mode->htotal * mode->vtotal;
+	num = mul_u32_u32(mode->clock, 1000);
+	den = mul_u32_u32(mode->htotal, mode->vtotal);
 
 	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
 		num *= 2;
@@ -1300,7 +1300,10 @@ int drm_mode_vrefresh(const struct drm_display_mode *mode)
 	if (mode->vscan > 1)
 		den *= mode->vscan;
 
-	return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(num, 1000), den);
+	if (unlikely(den > UINT_MAX))
+		return DIV64_U64_ROUND_CLOSEST(num, den);
+
+	return DIV_ROUND_CLOSEST_ULL(num, (u32) den);
 }
 EXPORT_SYMBOL(drm_mode_vrefresh);
 
-- 
2.34.1


WARNING: multiple messages have this Message-ID (diff)
From: Ziqi Zhao <astrajoan@yahoo.com>
To: syzbot+622bba18029bcde672e1@syzkaller.appspotmail.com,
	astrajoan@yahoo.com, jani.nikula@linux.intel.com,
	airlied@gmail.com, daniel@ffwll.ch,
	dri-devel@lists.freedesktop.org, ivan.orlov0322@gmail.com,
	maarten.lankhorst@linux.intel.com, mripard@kernel.org,
	skhan@linuxfoundation.org, tzimmermann@suse.de
Cc: netdev@vger.kernel.org, dsahern@kernel.org,
	syzkaller-bugs@googlegroups.com, linux-kernel@vger.kernel.org,
	edumazet@google.com, jiri@nvidia.com, jacob.e.keller@intel.com,
	kuba@kernel.org, pabeni@redhat.com, davem@davemloft.net
Subject: [Intel-gfx] [PATCH v3] drm/modes: Fix division by zero due to overflow
Date: Wed,  2 Aug 2023 10:47:46 -0700	[thread overview]
Message-ID: <20230802174746.2256-1-astrajoan@yahoo.com> (raw)
In-Reply-To: <00000000000034cf5d05fea52dd4@google.com>

In the bug reported by Syzbot, the variable `den == (1 << 22)` and
`mode->vscan == (1 << 10)`, causing the multiplication to overflow and
accidentally make `den == 0`. To prevent any chance of overflow, we
replace `num` and `den` with 64-bit unsigned integers, and explicitly
check if the divisor `den` will overflow. If so, we employ full 64-bit
division with rounding; otherwise we keep the 64-bit to 32-bit division
that could potentially be better optimized.

In order to minimize the performance overhead, the overflow check for
`den` is wrapped with an `unlikely` condition. Please let me know if
this usage is appropriate.

Reported-by: syzbot+622bba18029bcde672e1@syzkaller.appspotmail.com
Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
---
V1 -> V2: address style comments suggested by Jani Nikula
<jani.nikula@linux.intel.com>
V2 -> V3: change title to include context on overflow causing the
division by zero

 drivers/gpu/drm/drm_modes.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ac9a406250c5..137101960690 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1285,13 +1285,13 @@ EXPORT_SYMBOL(drm_mode_set_name);
  */
 int drm_mode_vrefresh(const struct drm_display_mode *mode)
 {
-	unsigned int num, den;
+	u64 num, den;
 
 	if (mode->htotal == 0 || mode->vtotal == 0)
 		return 0;
 
-	num = mode->clock;
-	den = mode->htotal * mode->vtotal;
+	num = mul_u32_u32(mode->clock, 1000);
+	den = mul_u32_u32(mode->htotal, mode->vtotal);
 
 	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
 		num *= 2;
@@ -1300,7 +1300,10 @@ int drm_mode_vrefresh(const struct drm_display_mode *mode)
 	if (mode->vscan > 1)
 		den *= mode->vscan;
 
-	return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(num, 1000), den);
+	if (unlikely(den > UINT_MAX))
+		return DIV64_U64_ROUND_CLOSEST(num, den);
+
+	return DIV_ROUND_CLOSEST_ULL(num, (u32) den);
 }
 EXPORT_SYMBOL(drm_mode_vrefresh);
 
-- 
2.34.1


  parent reply	other threads:[~2023-08-02 17:47 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-21 15:11 [syzbot] [dri?] divide error in drm_mode_vrefresh syzbot
2023-07-09  1:12 ` [PATCH] drm/modes: Fix division by zero error Ziqi Zhao
2023-07-09  1:12   ` Ziqi Zhao
2023-07-09  4:09   ` [syzbot] [dri?] divide error in drm_mode_vrefresh syzbot
2023-07-21 16:07   ` [PATCH] drm/modes: Fix division by zero error Ziqi Zhao
2023-07-21 16:07     ` Ziqi Zhao
2023-08-01  9:30     ` Jani Nikula
2023-08-01 21:55 ` [PATCH v2] " Ziqi Zhao
2023-08-01 21:55   ` [Intel-gfx] " Ziqi Zhao
2023-08-01 21:55   ` Ziqi Zhao
2023-08-02 10:04   ` Jani Nikula
2023-08-02 10:04     ` Jani Nikula
2023-08-02 13:24 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2023-08-02 13:44 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-08-02 17:28 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2023-08-02 17:47 ` Ziqi Zhao [this message]
2023-08-02 17:47   ` [Intel-gfx] [PATCH v3] drm/modes: Fix division by zero due to overflow Ziqi Zhao
2023-08-02 17:47   ` Ziqi Zhao
2023-08-03  9:35   ` Jani Nikula
2023-08-03  9:35     ` Jani Nikula

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230802174746.2256-1-astrajoan@yahoo.com \
    --to=astrajoan@yahoo.com \
    --cc=airlied@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=davem@davemloft.net \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=ivan.orlov0322@gmail.com \
    --cc=jacob.e.keller@intel.com \
    --cc=jani.nikula@linux.intel.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=skhan@linuxfoundation.org \
    --cc=syzbot+622bba18029bcde672e1@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.