[PATCH 0/7] Fix divide errors in fbdev drivers

[PATCH 0/7] Fix divide errors in fbdev drivers

[Zheyu Ma]

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

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

[Zheyu Ma]

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

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

[Helge Deller]

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;

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

[Zheyu Ma]

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

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

[Ondrej Zary]

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

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

[Helge Deller]

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

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

[Zheyu Ma]

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

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

[Zheyu Ma]

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

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

[Zheyu Ma]

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

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

[Geert Uytterhoeven]

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

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

[Zheyu Ma]

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

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

[Zheyu Ma]

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

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

[Geert Uytterhoeven]

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

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

[Zheyu Ma]

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

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

[Geert Uytterhoeven]

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