dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Fix divide errors in fbdev drivers
@ 2022-04-04  8:47 Zheyu Ma
  2022-04-04  8:47 ` [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero Zheyu Ma
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Zheyu Ma @ 2022-04-04  8:47 UTC (permalink / raw)
  To: deller; +Cc: linux-fbdev, Zheyu Ma, linux-kernel, dri-devel

None of these framebuffer drivers checks for 'pixclock', leading to many
divide errors, which we fix by checking the value of 'pixclock' in
*_check_var(). As discussed before, it is better to keep the check per
driver rather than in the caller.

https://lore.kernel.org/all/YPgbHMtLQqb1kP0l@ravnborg.org/

Zheyu Ma (7):
  video: fbdev: i740fb: Error out if 'pixclock' equals zero
  video: fbdev: neofb: Fix the check of 'var->pixclock'
  video: fbdev: kyro: Error out if 'lineclock' equals zero
  video: fbdev: vt8623fb: Error out if 'pixclock' equals zero
  video: fbdev: tridentfb: Error out if 'pixclock' equals zero
  video: fbdev: arkfb: Error out if 'pixclock' equals zero
  video: fbdev: s3fb: Error out if 'pixclock' equals zero

 drivers/video/fbdev/arkfb.c      | 3 +++
 drivers/video/fbdev/i740fb.c     | 3 +++
 drivers/video/fbdev/kyro/fbdev.c | 2 ++
 drivers/video/fbdev/neofb.c      | 2 +-
 drivers/video/fbdev/s3fb.c       | 3 +++
 drivers/video/fbdev/tridentfb.c  | 3 +++
 drivers/video/fbdev/vt8623fb.c   | 3 +++
 7 files changed, 18 insertions(+), 1 deletion(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero
  2022-04-04  8:47 [PATCH 0/7] Fix divide errors in fbdev drivers Zheyu Ma
@ 2022-04-04  8:47 ` Zheyu Ma
  2022-04-07 19:49   ` Helge Deller
  2022-04-04  8:47 ` [PATCH 2/7] video: fbdev: neofb: Fix the check of 'var->pixclock' Zheyu Ma
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Zheyu Ma @ 2022-04-04  8:47 UTC (permalink / raw)
  To: deller; +Cc: linux-fbdev, Zheyu Ma, linux-kernel, dri-devel

The userspace program could pass any values to the driver through
ioctl() interface. If the driver doesn't check the value of 'pixclock',
it may cause divide error.

Fix this by checking whether 'pixclock' is zero in the function
i740fb_check_var().

The following log reveals it:

divide error: 0000 [#1] PREEMPT SMP KASAN PTI
RIP: 0010:i740fb_decode_var drivers/video/fbdev/i740fb.c:444 [inline]
RIP: 0010:i740fb_set_par+0x272f/0x3bb0 drivers/video/fbdev/i740fb.c:739
Call Trace:
    fb_set_var+0x604/0xeb0 drivers/video/fbdev/core/fbmem.c:1036
    do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1112
    fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1191
    vfs_ioctl fs/ioctl.c:51 [inline]
    __do_sys_ioctl fs/ioctl.c:874 [inline]

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 drivers/video/fbdev/i740fb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/fbdev/i740fb.c b/drivers/video/fbdev/i740fb.c
index 52cce0db8bd3..b595437a5752 100644
--- a/drivers/video/fbdev/i740fb.c
+++ b/drivers/video/fbdev/i740fb.c
@@ -657,6 +657,9 @@ static int i740fb_decode_var(const struct fb_var_screeninfo *var,
 
 static int i740fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
+	if (!var->pixclock)
+		return -EINVAL;
+
 	switch (var->bits_per_pixel) {
 	case 8:
 		var->red.offset	= var->green.offset = var->blue.offset = 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/7] video: fbdev: neofb: Fix the check of 'var->pixclock'
  2022-04-04  8:47 [PATCH 0/7] Fix divide errors in fbdev drivers Zheyu Ma
  2022-04-04  8:47 ` [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero Zheyu Ma
@ 2022-04-04  8:47 ` Zheyu Ma
  2022-04-04  8:47 ` [PATCH 3/7] video: fbdev: kyro: Error out if 'lineclock' equals zero Zheyu Ma
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Zheyu Ma @ 2022-04-04  8:47 UTC (permalink / raw)
  To: deller; +Cc: linux-fbdev, Zheyu Ma, linux-kernel, dri-devel

The previous check against 'var->pixclock' doesn't return -EINVAL when
it equals zero, but the driver uses it again, causing the divide error.

Fix this by returning when 'var->pixclock' is zero.

The following log reveals it:

[   49.704574] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
[   49.704593] RIP: 0010:neofb_set_par+0x190f/0x49a0
[   49.704635] Call Trace:
[   49.704636]  <TASK>
[   49.704650]  fb_set_var+0x604/0xeb0
[   49.704702]  do_fb_ioctl+0x234/0x670
[   49.704745]  fb_ioctl+0xdd/0x130
[   49.704753]  do_syscall_64+0x3b/0x90

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 drivers/video/fbdev/neofb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/neofb.c b/drivers/video/fbdev/neofb.c
index 966df2a07360..28d32cbf496b 100644
--- a/drivers/video/fbdev/neofb.c
+++ b/drivers/video/fbdev/neofb.c
@@ -585,7 +585,7 @@ neofb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 
 	DBG("neofb_check_var");
 
-	if (var->pixclock && PICOS2KHZ(var->pixclock) > par->maxClock)
+	if (!var->pixclock || PICOS2KHZ(var->pixclock) > par->maxClock)
 		return -EINVAL;
 
 	/* Is the mode larger than the LCD panel? */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/7] video: fbdev: kyro: Error out if 'lineclock' equals zero
  2022-04-04  8:47 [PATCH 0/7] Fix divide errors in fbdev drivers Zheyu Ma
  2022-04-04  8:47 ` [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero Zheyu Ma
  2022-04-04  8:47 ` [PATCH 2/7] video: fbdev: neofb: Fix the check of 'var->pixclock' Zheyu Ma
@ 2022-04-04  8:47 ` Zheyu Ma
  2022-04-04  8:47 ` [PATCH 4/7] video: fbdev: vt8623fb: Error out if 'pixclock' " Zheyu Ma
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Zheyu Ma @ 2022-04-04  8:47 UTC (permalink / raw)
  To: deller; +Cc: linux-fbdev, Zheyu Ma, linux-kernel, dri-devel

The userspace program could pass any values to the driver through
ioctl() interface. If the driver doesn't check the value of 'lineclock',
it may cause divide error.

Fix this by checking whether 'lineclock' is zero.

The following log reveals it:

[   33.404918] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
[   33.404932] RIP: 0010:kyrofb_set_par+0x30d/0xd80
[   33.404976] Call Trace:
[   33.404978]  <TASK>
[   33.404987]  fb_set_var+0x604/0xeb0
[   33.405038]  do_fb_ioctl+0x234/0x670
[   33.405083]  fb_ioctl+0xdd/0x130
[   33.405091]  do_syscall_64+0x3b/0x90

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 drivers/video/fbdev/kyro/fbdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/video/fbdev/kyro/fbdev.c b/drivers/video/fbdev/kyro/fbdev.c
index 25801e8e3f74..d57772f96ad2 100644
--- a/drivers/video/fbdev/kyro/fbdev.c
+++ b/drivers/video/fbdev/kyro/fbdev.c
@@ -494,6 +494,8 @@ static int kyrofb_set_par(struct fb_info *info)
 				    info->var.hsync_len +
 				    info->var.left_margin)) / 1000;
 
+	if (!lineclock)
+		return -EINVAL;
 
 	/* time for a frame in ns (precision in 32bpp) */
 	frameclock = lineclock * (info->var.yres +
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/7] video: fbdev: vt8623fb: Error out if 'pixclock' equals zero
  2022-04-04  8:47 [PATCH 0/7] Fix divide errors in fbdev drivers Zheyu Ma
                   ` (2 preceding siblings ...)
  2022-04-04  8:47 ` [PATCH 3/7] video: fbdev: kyro: Error out if 'lineclock' equals zero Zheyu Ma
@ 2022-04-04  8:47 ` Zheyu Ma
  2022-04-04 11:47   ` Geert Uytterhoeven
  2022-04-04  8:47 ` [PATCH 5/7] video: fbdev: tridentfb: " Zheyu Ma
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Zheyu Ma @ 2022-04-04  8:47 UTC (permalink / raw)
  To: deller; +Cc: linux-fbdev, Zheyu Ma, linux-kernel, dri-devel

The userspace program could pass any values to the driver through
ioctl() interface. If the driver doesn't check the value of 'pixclock',
it may cause divide error.

Fix this by checking whether 'pixclock' is zero in the function
vt8623fb_check_var().

The following log reveals it:

[   47.778727] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
[   47.778803] RIP: 0010:vt8623fb_set_par+0xecd/0x2210
[   47.778870] Call Trace:
[   47.778872]  <TASK>
[   47.778909]  fb_set_var+0x604/0xeb0
[   47.778995]  do_fb_ioctl+0x234/0x670
[   47.779041]  fb_ioctl+0xdd/0x130
[   47.779048]  do_syscall_64+0x3b/0x90

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 drivers/video/fbdev/vt8623fb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
index 7a959e5ba90b..a92a8c670cf0 100644
--- a/drivers/video/fbdev/vt8623fb.c
+++ b/drivers/video/fbdev/vt8623fb.c
@@ -321,6 +321,9 @@ static int vt8623fb_check_var(struct fb_var_screeninfo *var, struct fb_info *inf
 {
 	int rv, mem, step;
 
+	if (!var->pixclock)
+		return -EINVAL;
+
 	/* Find appropriate format */
 	rv = svga_match_format (vt8623fb_formats, var, NULL);
 	if (rv < 0)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/7] video: fbdev: tridentfb: Error out if 'pixclock' equals zero
  2022-04-04  8:47 [PATCH 0/7] Fix divide errors in fbdev drivers Zheyu Ma
                   ` (3 preceding siblings ...)
  2022-04-04  8:47 ` [PATCH 4/7] video: fbdev: vt8623fb: Error out if 'pixclock' " Zheyu Ma
@ 2022-04-04  8:47 ` Zheyu Ma
  2022-04-04  8:47 ` [PATCH 6/7] video: fbdev: arkfb: " Zheyu Ma
  2022-04-04  8:47 ` [PATCH 7/7] video: fbdev: s3fb: " Zheyu Ma
  6 siblings, 0 replies; 15+ messages in thread
From: Zheyu Ma @ 2022-04-04  8:47 UTC (permalink / raw)
  To: deller; +Cc: linux-fbdev, Zheyu Ma, linux-kernel, dri-devel

The userspace program could pass any values to the driver through
ioctl() interface. If the driver doesn't check the value of 'pixclock',
it may cause divide error.

Fix this by checking whether 'pixclock' is zero.

The following log reveals it:

[   38.260715] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
[   38.260733] RIP: 0010:tridentfb_check_var+0x853/0xe60
[   38.260791] Call Trace:
[   38.260793]  <TASK>
[   38.260796]  fb_set_var+0x367/0xeb0
[   38.260879]  do_fb_ioctl+0x234/0x670
[   38.260922]  fb_ioctl+0xdd/0x130
[   38.260930]  do_syscall_64+0x3b/0x90

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 drivers/video/fbdev/tridentfb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 4d20cb557ff0..319131bd72cf 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -996,6 +996,9 @@ static int tridentfb_check_var(struct fb_var_screeninfo *var,
 	int ramdac = 230000; /* 230MHz for most 3D chips */
 	debug("enter\n");
 
+	if (!var->pixclock)
+		return -EINVAL;
+
 	/* check color depth */
 	if (bpp == 24)
 		bpp = var->bits_per_pixel = 32;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/7] video: fbdev: arkfb: Error out if 'pixclock' equals zero
  2022-04-04  8:47 [PATCH 0/7] Fix divide errors in fbdev drivers Zheyu Ma
                   ` (4 preceding siblings ...)
  2022-04-04  8:47 ` [PATCH 5/7] video: fbdev: tridentfb: " Zheyu Ma
@ 2022-04-04  8:47 ` Zheyu Ma
  2022-04-04 15:52   ` Geert Uytterhoeven
  2022-04-04  8:47 ` [PATCH 7/7] video: fbdev: s3fb: " Zheyu Ma
  6 siblings, 1 reply; 15+ messages in thread
From: Zheyu Ma @ 2022-04-04  8:47 UTC (permalink / raw)
  To: deller; +Cc: linux-fbdev, Zheyu Ma, linux-kernel, dri-devel

The userspace program could pass any values to the driver through
ioctl() interface. If the driver doesn't check the value of 'pixclock',
it may cause divide error.

Fix this by checking whether 'pixclock' is zero.

The following log reveals it:

[   76.603696] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
[   76.603712] RIP: 0010:arkfb_set_par+0x10fc/0x24f0
[   76.603762] Call Trace:
[   76.603764]  <TASK>
[   76.603773]  fb_set_var+0x604/0xeb0
[   76.603827]  do_fb_ioctl+0x234/0x670
[   76.603873]  fb_ioctl+0xdd/0x130
[   76.603881]  do_syscall_64+0x3b/0x90

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 drivers/video/fbdev/arkfb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index edf169d0816e..eb3e47c58c5f 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -566,6 +566,9 @@ static int arkfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
 	int rv, mem, step;
 
+	if (!var->pixclock)
+		return -EINVAL;
+
 	/* Find appropriate format */
 	rv = svga_match_format (arkfb_formats, var, NULL);
 	if (rv < 0)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/7] video: fbdev: s3fb: Error out if 'pixclock' equals zero
  2022-04-04  8:47 [PATCH 0/7] Fix divide errors in fbdev drivers Zheyu Ma
                   ` (5 preceding siblings ...)
  2022-04-04  8:47 ` [PATCH 6/7] video: fbdev: arkfb: " Zheyu Ma
@ 2022-04-04  8:47 ` Zheyu Ma
  2022-04-04 15:52   ` Geert Uytterhoeven
  6 siblings, 1 reply; 15+ messages in thread
From: Zheyu Ma @ 2022-04-04  8:47 UTC (permalink / raw)
  To: deller; +Cc: linux-fbdev, Zheyu Ma, linux-kernel, dri-devel

The userspace program could pass any values to the driver through
ioctl() interface. If the driver doesn't check the value of 'pixclock',
it may cause divide error.

Fix this by checking whether 'pixclock' is zero in s3fb_check_var().

The following log reveals it:

[  511.141561] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
[  511.141607] RIP: 0010:s3fb_check_var+0x3f3/0x530
[  511.141693] Call Trace:
[  511.141695]  <TASK>
[  511.141716]  fb_set_var+0x367/0xeb0
[  511.141815]  do_fb_ioctl+0x234/0x670
[  511.141876]  fb_ioctl+0xdd/0x130
[  511.141888]  do_syscall_64+0x3b/0x90

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 drivers/video/fbdev/s3fb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index 5c74253e7b2c..b93c8eb02336 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -549,6 +549,9 @@ static int s3fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 	int rv, mem, step;
 	u16 m, n, r;
 
+	if (!var->pixclock)
+		return -EINVAL;
+
 	/* Find appropriate format */
 	rv = svga_match_format (s3fb_formats, var, NULL);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/7] video: fbdev: vt8623fb: Error out if 'pixclock' equals zero
  2022-04-04  8:47 ` [PATCH 4/7] video: fbdev: vt8623fb: Error out if 'pixclock' " Zheyu Ma
@ 2022-04-04 11:47   ` Geert Uytterhoeven
  0 siblings, 0 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2022-04-04 11:47 UTC (permalink / raw)
  To: Zheyu Ma
  Cc: Helge Deller, Linux Fbdev development list, DRI Development,
	Linux Kernel Mailing List

Hi Zheyu,

On Mon, Apr 4, 2022 at 1:07 PM Zheyu Ma <zheyuma97@gmail.com> wrote:
> The userspace program could pass any values to the driver through
> ioctl() interface. If the driver doesn't check the value of 'pixclock',
> it may cause divide error.
>
> Fix this by checking whether 'pixclock' is zero in the function
> vt8623fb_check_var().
>
> The following log reveals it:
>
> [   47.778727] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
> [   47.778803] RIP: 0010:vt8623fb_set_par+0xecd/0x2210
> [   47.778870] Call Trace:
> [   47.778872]  <TASK>
> [   47.778909]  fb_set_var+0x604/0xeb0
> [   47.778995]  do_fb_ioctl+0x234/0x670
> [   47.779041]  fb_ioctl+0xdd/0x130
> [   47.779048]  do_syscall_64+0x3b/0x90
>
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>

Thanks for your patch!

> --- a/drivers/video/fbdev/vt8623fb.c
> +++ b/drivers/video/fbdev/vt8623fb.c
> @@ -321,6 +321,9 @@ static int vt8623fb_check_var(struct fb_var_screeninfo *var, struct fb_info *inf
>  {
>         int rv, mem, step;
>
> +       if (!var->pixclock)
> +               return -EINVAL;
> +

When passed an invalid value, .check_var() is supposed to
round up the invalid to a valid value, if possible.

>         /* Find appropriate format */
>         rv = svga_match_format (vt8623fb_formats, var, NULL);
>         if (rv < 0)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/7] video: fbdev: arkfb: Error out if 'pixclock' equals zero
  2022-04-04  8:47 ` [PATCH 6/7] video: fbdev: arkfb: " Zheyu Ma
@ 2022-04-04 15:52   ` Geert Uytterhoeven
  0 siblings, 0 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2022-04-04 15:52 UTC (permalink / raw)
  To: Zheyu Ma
  Cc: Helge Deller, Linux Fbdev development list, DRI Development,
	Linux Kernel Mailing List

On Mon, Apr 4, 2022 at 3:10 PM Zheyu Ma <zheyuma97@gmail.com> wrote:
> The userspace program could pass any values to the driver through
> ioctl() interface. If the driver doesn't check the value of 'pixclock',
> it may cause divide error.
>
> Fix this by checking whether 'pixclock' is zero.
>
> The following log reveals it:
>
> [   76.603696] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
> [   76.603712] RIP: 0010:arkfb_set_par+0x10fc/0x24f0
> [   76.603762] Call Trace:
> [   76.603764]  <TASK>
> [   76.603773]  fb_set_var+0x604/0xeb0
> [   76.603827]  do_fb_ioctl+0x234/0x670
> [   76.603873]  fb_ioctl+0xdd/0x130
> [   76.603881]  do_syscall_64+0x3b/0x90
>
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>

Thanks for your patch!

> --- a/drivers/video/fbdev/arkfb.c
> +++ b/drivers/video/fbdev/arkfb.c
> @@ -566,6 +566,9 @@ static int arkfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>  {
>         int rv, mem, step;
>
> +       if (!var->pixclock)
> +               return -EINVAL;

When passed an invalid value, .check_var() is supposed to
round up the invalid to a valid value, if possible.

> +
>         /* Find appropriate format */
>         rv = svga_match_format (arkfb_formats, var, NULL);
>         if (rv < 0)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 7/7] video: fbdev: s3fb: Error out if 'pixclock' equals zero
  2022-04-04  8:47 ` [PATCH 7/7] video: fbdev: s3fb: " Zheyu Ma
@ 2022-04-04 15:52   ` Geert Uytterhoeven
  0 siblings, 0 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2022-04-04 15:52 UTC (permalink / raw)
  To: Zheyu Ma
  Cc: Helge Deller, Linux Fbdev development list, DRI Development,
	Linux Kernel Mailing List

On Mon, Apr 4, 2022 at 3:50 PM Zheyu Ma <zheyuma97@gmail.com> wrote:
> The userspace program could pass any values to the driver through
> ioctl() interface. If the driver doesn't check the value of 'pixclock',
> it may cause divide error.
>
> Fix this by checking whether 'pixclock' is zero in s3fb_check_var().
>
> The following log reveals it:
>
> [  511.141561] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
> [  511.141607] RIP: 0010:s3fb_check_var+0x3f3/0x530
> [  511.141693] Call Trace:
> [  511.141695]  <TASK>
> [  511.141716]  fb_set_var+0x367/0xeb0
> [  511.141815]  do_fb_ioctl+0x234/0x670
> [  511.141876]  fb_ioctl+0xdd/0x130
> [  511.141888]  do_syscall_64+0x3b/0x90
>
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
> ---
>  drivers/video/fbdev/s3fb.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
> index 5c74253e7b2c..b93c8eb02336 100644
> --- a/drivers/video/fbdev/s3fb.c
> +++ b/drivers/video/fbdev/s3fb.c
> @@ -549,6 +549,9 @@ static int s3fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>         int rv, mem, step;
>         u16 m, n, r;
>
> +       if (!var->pixclock)
> +               return -EINVAL;
> +

When passed an invalid value, .check_var() is supposed to
round up the invalid value to a valid value, if possible.

>         /* Find appropriate format */
>         rv = svga_match_format (s3fb_formats, var, NULL);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero
  2022-04-04  8:47 ` [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero Zheyu Ma
@ 2022-04-07 19:49   ` Helge Deller
  2022-04-08  1:58     ` Zheyu Ma
  0 siblings, 1 reply; 15+ messages in thread
From: Helge Deller @ 2022-04-07 19:49 UTC (permalink / raw)
  To: Zheyu Ma; +Cc: linux-fbdev, linux-kernel, dri-devel

On 4/4/22 10:47, Zheyu Ma wrote:
> The userspace program could pass any values to the driver through
> ioctl() interface. If the driver doesn't check the value of 'pixclock',
> it may cause divide error.
>
> Fix this by checking whether 'pixclock' is zero in the function
> i740fb_check_var().
>
> The following log reveals it:
>
> divide error: 0000 [#1] PREEMPT SMP KASAN PTI
> RIP: 0010:i740fb_decode_var drivers/video/fbdev/i740fb.c:444 [inline]
> RIP: 0010:i740fb_set_par+0x272f/0x3bb0 drivers/video/fbdev/i740fb.c:739
> Call Trace:
>     fb_set_var+0x604/0xeb0 drivers/video/fbdev/core/fbmem.c:1036
>     do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1112
>     fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1191
>     vfs_ioctl fs/ioctl.c:51 [inline]
>     __do_sys_ioctl fs/ioctl.c:874 [inline]
>
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>

Hello Zheyu,

I've applied the patches #2-#7 of this series, but left
out this specific patch (for now).
As discussed on the mailing list we can try to come up with a
better fix (to round up the pixclock when it's invalid).
If not, I will apply this one later.

Thanks!
Helge


> ---
>  drivers/video/fbdev/i740fb.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/video/fbdev/i740fb.c b/drivers/video/fbdev/i740fb.c
> index 52cce0db8bd3..b595437a5752 100644
> --- a/drivers/video/fbdev/i740fb.c
> +++ b/drivers/video/fbdev/i740fb.c
> @@ -657,6 +657,9 @@ static int i740fb_decode_var(const struct fb_var_screeninfo *var,
>
>  static int i740fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>  {
> +	if (!var->pixclock)
> +		return -EINVAL;
> +
>  	switch (var->bits_per_pixel) {
>  	case 8:
>  		var->red.offset	= var->green.offset = var->blue.offset = 0;


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero
  2022-04-07 19:49   ` Helge Deller
@ 2022-04-08  1:58     ` Zheyu Ma
  2022-04-10  9:02       ` Ondrej Zary
  0 siblings, 1 reply; 15+ messages in thread
From: Zheyu Ma @ 2022-04-08  1:58 UTC (permalink / raw)
  To: Helge Deller
  Cc: Linux Fbdev development list, Linux Kernel Mailing List, DRI Development

On Fri, Apr 8, 2022 at 3:50 AM Helge Deller <deller@gmx.de> wrote:
>
> On 4/4/22 10:47, Zheyu Ma wrote:
> > The userspace program could pass any values to the driver through
> > ioctl() interface. If the driver doesn't check the value of 'pixclock',
> > it may cause divide error.
> >
> > Fix this by checking whether 'pixclock' is zero in the function
> > i740fb_check_var().
> >
> > The following log reveals it:
> >
> > divide error: 0000 [#1] PREEMPT SMP KASAN PTI
> > RIP: 0010:i740fb_decode_var drivers/video/fbdev/i740fb.c:444 [inline]
> > RIP: 0010:i740fb_set_par+0x272f/0x3bb0 drivers/video/fbdev/i740fb.c:739
> > Call Trace:
> >     fb_set_var+0x604/0xeb0 drivers/video/fbdev/core/fbmem.c:1036
> >     do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1112
> >     fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1191
> >     vfs_ioctl fs/ioctl.c:51 [inline]
> >     __do_sys_ioctl fs/ioctl.c:874 [inline]
> >
> > Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
>
> Hello Zheyu,
>
> I've applied the patches #2-#7 of this series, but left
> out this specific patch (for now).
> As discussed on the mailing list we can try to come up with a
> better fix (to round up the pixclock when it's invalid).
> If not, I will apply this one later.

I'm also looking forward to a more appropriate patch for this driver!

Thanks,
Zheyu Ma

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero
  2022-04-08  1:58     ` Zheyu Ma
@ 2022-04-10  9:02       ` Ondrej Zary
  2022-04-11  6:18         ` Helge Deller
  0 siblings, 1 reply; 15+ messages in thread
From: Ondrej Zary @ 2022-04-10  9:02 UTC (permalink / raw)
  To: Zheyu Ma
  Cc: Helge Deller, Linux Fbdev development list, DRI Development,
	Linux Kernel Mailing List

On Friday 08 April 2022 03:58:10 Zheyu Ma wrote:
> On Fri, Apr 8, 2022 at 3:50 AM Helge Deller <deller@gmx.de> wrote:
> >
> > On 4/4/22 10:47, Zheyu Ma wrote:
> > > The userspace program could pass any values to the driver through
> > > ioctl() interface. If the driver doesn't check the value of 'pixclock',
> > > it may cause divide error.
> > >
> > > Fix this by checking whether 'pixclock' is zero in the function
> > > i740fb_check_var().
> > >
> > > The following log reveals it:
> > >
> > > divide error: 0000 [#1] PREEMPT SMP KASAN PTI
> > > RIP: 0010:i740fb_decode_var drivers/video/fbdev/i740fb.c:444 [inline]
> > > RIP: 0010:i740fb_set_par+0x272f/0x3bb0 drivers/video/fbdev/i740fb.c:739
> > > Call Trace:
> > >     fb_set_var+0x604/0xeb0 drivers/video/fbdev/core/fbmem.c:1036
> > >     do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1112
> > >     fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1191
> > >     vfs_ioctl fs/ioctl.c:51 [inline]
> > >     __do_sys_ioctl fs/ioctl.c:874 [inline]
> > >
> > > Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
> >
> > Hello Zheyu,
> >
> > I've applied the patches #2-#7 of this series, but left
> > out this specific patch (for now).
> > As discussed on the mailing list we can try to come up with a
> > better fix (to round up the pixclock when it's invalid).
> > If not, I will apply this one later.
> 
> I'm also looking forward to a more appropriate patch for this driver!

I was not able to reproduce it at first but finally found it: the monitor must be unplugged. If a valid EDID is present, fb_validate_mode() call in i740fb_check_var() will refuse zero pixclock.

Haven't found any obvious way to correct zero pixclock value. Most other drivers simply return -EINVAL.

> Thanks,
> Zheyu Ma
> 


-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero
  2022-04-10  9:02       ` Ondrej Zary
@ 2022-04-11  6:18         ` Helge Deller
  0 siblings, 0 replies; 15+ messages in thread
From: Helge Deller @ 2022-04-11  6:18 UTC (permalink / raw)
  To: Ondrej Zary, Zheyu Ma
  Cc: Linux Fbdev development list, Linux Kernel Mailing List, DRI Development

On 4/10/22 11:02, Ondrej Zary wrote:
> On Friday 08 April 2022 03:58:10 Zheyu Ma wrote:
>> On Fri, Apr 8, 2022 at 3:50 AM Helge Deller <deller@gmx.de> wrote:
>>>
>>> On 4/4/22 10:47, Zheyu Ma wrote:
>>>> The userspace program could pass any values to the driver through
>>>> ioctl() interface. If the driver doesn't check the value of 'pixclock',
>>>> it may cause divide error.
>>>>
>>>> Fix this by checking whether 'pixclock' is zero in the function
>>>> i740fb_check_var().
>>>>
>>>> The following log reveals it:
>>>>
>>>> divide error: 0000 [#1] PREEMPT SMP KASAN PTI
>>>> RIP: 0010:i740fb_decode_var drivers/video/fbdev/i740fb.c:444 [inline]
>>>> RIP: 0010:i740fb_set_par+0x272f/0x3bb0 drivers/video/fbdev/i740fb.c:739
>>>> Call Trace:
>>>>     fb_set_var+0x604/0xeb0 drivers/video/fbdev/core/fbmem.c:1036
>>>>     do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1112
>>>>     fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1191
>>>>     vfs_ioctl fs/ioctl.c:51 [inline]
>>>>     __do_sys_ioctl fs/ioctl.c:874 [inline]
>>>>
>>>> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
>>>
>>> Hello Zheyu,
>>>
>>> I've applied the patches #2-#7 of this series, but left
>>> out this specific patch (for now).
>>> As discussed on the mailing list we can try to come up with a
>>> better fix (to round up the pixclock when it's invalid).
>>> If not, I will apply this one later.
>>
>> I'm also looking forward to a more appropriate patch for this driver!
>
> I was not able to reproduce it at first but finally found it: the
> monitor must be unplugged. If a valid EDID is present,
> fb_validate_mode() call in i740fb_check_var() will refuse zero
> pixclock.
>
> Haven't found any obvious way to correct zero pixclock value. Most other drivers simply return -EINVAL.

Thanks for checking, Ondrej!

So, I'll apply the EINVAL-patch from Zheyu for now.

Helge

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-04-11  6:19 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04  8:47 [PATCH 0/7] Fix divide errors in fbdev drivers Zheyu Ma
2022-04-04  8:47 ` [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero Zheyu Ma
2022-04-07 19:49   ` Helge Deller
2022-04-08  1:58     ` Zheyu Ma
2022-04-10  9:02       ` Ondrej Zary
2022-04-11  6:18         ` Helge Deller
2022-04-04  8:47 ` [PATCH 2/7] video: fbdev: neofb: Fix the check of 'var->pixclock' Zheyu Ma
2022-04-04  8:47 ` [PATCH 3/7] video: fbdev: kyro: Error out if 'lineclock' equals zero Zheyu Ma
2022-04-04  8:47 ` [PATCH 4/7] video: fbdev: vt8623fb: Error out if 'pixclock' " Zheyu Ma
2022-04-04 11:47   ` Geert Uytterhoeven
2022-04-04  8:47 ` [PATCH 5/7] video: fbdev: tridentfb: " Zheyu Ma
2022-04-04  8:47 ` [PATCH 6/7] video: fbdev: arkfb: " Zheyu Ma
2022-04-04 15:52   ` Geert Uytterhoeven
2022-04-04  8:47 ` [PATCH 7/7] video: fbdev: s3fb: " Zheyu Ma
2022-04-04 15:52   ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).