2022-04-04 12:39:47

by Zheyu Ma

[permalink] [raw]
Subject: [PATCH 0/7] Fix divide errors in fbdev drivers

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/[email protected]/

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


2022-04-04 14:45:23

by Zheyu Ma

[permalink] [raw]
Subject: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero

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 <[email protected]>
---
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

2022-04-04 15:39:12

by Zheyu Ma

[permalink] [raw]
Subject: [PATCH 7/7] video: fbdev: s3fb: Error out if 'pixclock' equals zero

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 <[email protected]>
---
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

2022-04-05 00:00:33

by Zheyu Ma

[permalink] [raw]
Subject: [PATCH 2/7] video: fbdev: neofb: Fix the check of 'var->pixclock'

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 <[email protected]>
---
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

2022-04-05 01:25:56

by Zheyu Ma

[permalink] [raw]
Subject: [PATCH 4/7] video: fbdev: vt8623fb: Error out if 'pixclock' equals zero

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 <[email protected]>
---
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

2022-04-05 01:31:09

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 7/7] video: fbdev: s3fb: Error out if 'pixclock' equals zero

On Mon, Apr 4, 2022 at 3:50 PM Zheyu Ma <[email protected]> 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 <[email protected]>
> ---
> 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 -- [email protected]

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

2022-04-05 01:51:19

by Zheyu Ma

[permalink] [raw]
Subject: [PATCH 3/7] video: fbdev: kyro: Error out if 'lineclock' equals zero

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 <[email protected]>
---
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

2022-04-05 02:09:49

by Zheyu Ma

[permalink] [raw]
Subject: [PATCH 6/7] video: fbdev: arkfb: Error out if 'pixclock' equals zero

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 <[email protected]>
---
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

2022-04-05 03:11:07

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 6/7] video: fbdev: arkfb: Error out if 'pixclock' equals zero

On Mon, Apr 4, 2022 at 3:10 PM Zheyu Ma <[email protected]> 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 <[email protected]>

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 -- [email protected]

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

2022-04-05 03:16:21

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 4/7] video: fbdev: vt8623fb: Error out if 'pixclock' equals zero

Hi Zheyu,

On Mon, Apr 4, 2022 at 1:07 PM Zheyu Ma <[email protected]> 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 <[email protected]>

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 -- [email protected]

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

2022-04-05 03:27:16

by Zheyu Ma

[permalink] [raw]
Subject: [PATCH 5/7] video: fbdev: tridentfb: Error out if 'pixclock' equals zero

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 <[email protected]>
---
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

2022-04-07 21:28:11

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero

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 <[email protected]>

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;

2022-04-08 02:26:43

by Zheyu Ma

[permalink] [raw]
Subject: Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero

On Fri, Apr 8, 2022 at 3:50 AM Helge Deller <[email protected]> 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 <[email protected]>
>
> 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

2022-04-11 09:08:50

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero

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 <[email protected]> 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 <[email protected]>
>>>
>>> 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

2022-04-11 10:20:16

by Ondrej Zary

[permalink] [raw]
Subject: Re: [PATCH 1/7] video: fbdev: i740fb: Error out if 'pixclock' equals zero

On Friday 08 April 2022 03:58:10 Zheyu Ma wrote:
> On Fri, Apr 8, 2022 at 3:50 AM Helge Deller <[email protected]> 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 <[email protected]>
> >
> > 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