* [PATCH 0/3] Fix bugs in *_set_par() caused by user input
@ 2022-08-04 12:41 Zheyu Ma
2022-08-04 12:41 ` [PATCH 1/3] video: fbdev: vt8623fb: Check the size of screen before memset_io() Zheyu Ma
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Zheyu Ma @ 2022-08-04 12:41 UTC (permalink / raw)
To: deller, adaplas, santiago, akpm
Cc: linux-fbdev, dri-devel, linux-kernel, Zheyu Ma
In the function *_set_par(), the value of 'screen_size' is
calculated by the user input. If the user provides the improper value,
the value of 'screen_size' may larger than 'info->screen_size', which
may cause a bug in the memset_io().
Zheyu Ma (3):
video: fbdev: vt8623fb: Check the size of screen before memset_io()
video: fbdev: arkfb: Check the size of screen before memset_io()
video: fbdev: s3fb: Check the size of screen before memset_io()
drivers/video/fbdev/arkfb.c | 2 ++
drivers/video/fbdev/s3fb.c | 2 ++
drivers/video/fbdev/vt8623fb.c | 2 ++
3 files changed, 6 insertions(+)
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] video: fbdev: vt8623fb: Check the size of screen before memset_io()
2022-08-04 12:41 [PATCH 0/3] Fix bugs in *_set_par() caused by user input Zheyu Ma
@ 2022-08-04 12:41 ` Zheyu Ma
2022-08-04 12:41 ` [PATCH 2/3] video: fbdev: arkfb: " Zheyu Ma
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Zheyu Ma @ 2022-08-04 12:41 UTC (permalink / raw)
To: deller, adaplas, santiago, akpm
Cc: linux-fbdev, dri-devel, linux-kernel, Zheyu Ma
In the function vt8623fb_set_par(), the value of 'screen_size' is
calculated by the user input. If the user provides the improper value,
the value of 'screen_size' may larger than 'info->screen_size', which
may cause the following bug:
[ 583.339036] BUG: unable to handle page fault for address: ffffc90005000000
[ 583.339049] #PF: supervisor write access in kernel mode
[ 583.339052] #PF: error_code(0x0002) - not-present page
[ 583.339074] RIP: 0010:memset_orig+0x33/0xb0
[ 583.339110] Call Trace:
[ 583.339118] vt8623fb_set_par+0x11cd/0x21e0
[ 583.339146] fb_set_var+0x604/0xeb0
[ 583.339181] do_fb_ioctl+0x234/0x670
[ 583.339209] fb_ioctl+0xdd/0x130
Fix the this by checking the value of 'screen_size' before memset_io().
Fixes: 558b7bd86c32 ("vt8623fb: new framebuffer driver for VIA VT8623")
Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
drivers/video/fbdev/vt8623fb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
index a92a8c670cf0..4274c6efb249 100644
--- a/drivers/video/fbdev/vt8623fb.c
+++ b/drivers/video/fbdev/vt8623fb.c
@@ -507,6 +507,8 @@ static int vt8623fb_set_par(struct fb_info *info)
(info->var.vmode & FB_VMODE_DOUBLE) ? 2 : 1, 1,
1, info->node);
+ if (screen_size > info->screen_size)
+ screen_size = info->screen_size;
memset_io(info->screen_base, 0x00, screen_size);
/* Device and screen back on */
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] video: fbdev: arkfb: Check the size of screen before memset_io()
2022-08-04 12:41 [PATCH 0/3] Fix bugs in *_set_par() caused by user input Zheyu Ma
2022-08-04 12:41 ` [PATCH 1/3] video: fbdev: vt8623fb: Check the size of screen before memset_io() Zheyu Ma
@ 2022-08-04 12:41 ` Zheyu Ma
2022-08-04 12:41 ` [PATCH 3/3] video: fbdev: s3fb: " Zheyu Ma
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Zheyu Ma @ 2022-08-04 12:41 UTC (permalink / raw)
To: deller, adaplas, santiago, akpm
Cc: linux-fbdev, dri-devel, linux-kernel, Zheyu Ma
In the function arkfb_set_par(), the value of 'screen_size' is
calculated by the user input. If the user provides the improper value,
the value of 'screen_size' may larger than 'info->screen_size', which
may cause the following bug:
[ 659.399066] BUG: unable to handle page fault for address: ffffc90003000000
[ 659.399077] #PF: supervisor write access in kernel mode
[ 659.399079] #PF: error_code(0x0002) - not-present page
[ 659.399094] RIP: 0010:memset_orig+0x33/0xb0
[ 659.399116] Call Trace:
[ 659.399122] arkfb_set_par+0x143f/0x24c0
[ 659.399130] fb_set_var+0x604/0xeb0
[ 659.399161] do_fb_ioctl+0x234/0x670
[ 659.399189] fb_ioctl+0xdd/0x130
Fix the this by checking the value of 'screen_size' before memset_io().
Fixes: 681e14730c73 ("arkfb: new framebuffer driver for ARK Logic cards")
Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
drivers/video/fbdev/arkfb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index eb3e47c58c5f..de4002914f9e 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -792,6 +792,8 @@ static int arkfb_set_par(struct fb_info *info)
value = ((value * hmul / hdiv) / 8) - 5;
vga_wcrt(par->state.vgabase, 0x42, (value + 1) / 2);
+ if (screen_size > info->screen_size)
+ screen_size = info->screen_size;
memset_io(info->screen_base, 0x00, screen_size);
/* Device and screen back on */
svga_wcrt_mask(par->state.vgabase, 0x17, 0x80, 0x80);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] video: fbdev: s3fb: Check the size of screen before memset_io()
2022-08-04 12:41 [PATCH 0/3] Fix bugs in *_set_par() caused by user input Zheyu Ma
2022-08-04 12:41 ` [PATCH 1/3] video: fbdev: vt8623fb: Check the size of screen before memset_io() Zheyu Ma
2022-08-04 12:41 ` [PATCH 2/3] video: fbdev: arkfb: " Zheyu Ma
@ 2022-08-04 12:41 ` Zheyu Ma
2022-08-04 14:43 ` [PATCH 0/3] Fix bugs in *_set_par() caused by user input Ondrej Zajicek
2022-08-05 16:47 ` Helge Deller
4 siblings, 0 replies; 7+ messages in thread
From: Zheyu Ma @ 2022-08-04 12:41 UTC (permalink / raw)
To: deller, adaplas, santiago, akpm
Cc: linux-fbdev, dri-devel, linux-kernel, Zheyu Ma
In the function s3fb_set_par(), the value of 'screen_size' is
calculated by the user input. If the user provides the improper value,
the value of 'screen_size' may larger than 'info->screen_size', which
may cause the following bug:
[ 54.083733] BUG: unable to handle page fault for address: ffffc90003000000
[ 54.083742] #PF: supervisor write access in kernel mode
[ 54.083744] #PF: error_code(0x0002) - not-present page
[ 54.083760] RIP: 0010:memset_orig+0x33/0xb0
[ 54.083782] Call Trace:
[ 54.083788] s3fb_set_par+0x1ec6/0x4040
[ 54.083806] fb_set_var+0x604/0xeb0
[ 54.083836] do_fb_ioctl+0x234/0x670
Fix the this by checking the value of 'screen_size' before memset_io().
Fixes: a268422de8bf ("[PATCH] fbdev driver for S3 Trio/Virge")
Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
drivers/video/fbdev/s3fb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index b93c8eb02336..5069f6f67923 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -905,6 +905,8 @@ static int s3fb_set_par(struct fb_info *info)
value = clamp((htotal + hsstart + 1) / 2 + 2, hsstart + 4, htotal + 1);
svga_wcrt_multi(par->state.vgabase, s3_dtpc_regs, value);
+ if (screen_size > info->screen_size)
+ screen_size = info->screen_size;
memset_io(info->screen_base, 0x00, screen_size);
/* Device and screen back on */
svga_wcrt_mask(par->state.vgabase, 0x17, 0x80, 0x80);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] Fix bugs in *_set_par() caused by user input
2022-08-04 12:41 [PATCH 0/3] Fix bugs in *_set_par() caused by user input Zheyu Ma
` (2 preceding siblings ...)
2022-08-04 12:41 ` [PATCH 3/3] video: fbdev: s3fb: " Zheyu Ma
@ 2022-08-04 14:43 ` Ondrej Zajicek
2022-08-05 2:48 ` Zheyu Ma
2022-08-05 16:47 ` Helge Deller
4 siblings, 1 reply; 7+ messages in thread
From: Ondrej Zajicek @ 2022-08-04 14:43 UTC (permalink / raw)
To: Zheyu Ma; +Cc: deller, adaplas, akpm, linux-fbdev, dri-devel, linux-kernel
On Thu, Aug 04, 2022 at 08:41:22PM +0800, Zheyu Ma wrote:
> In the function *_set_par(), the value of 'screen_size' is
> calculated by the user input. If the user provides the improper value,
> the value of 'screen_size' may larger than 'info->screen_size', which
> may cause a bug in the memset_io().
Hi
I did not saw fbdev code in years, but should not this be already checked
by *_check_var() ?
arkfb_check_var():
...
/* Check whether have enough memory */
mem = ((var->bits_per_pixel * var->xres_virtual) >> 3) * var->yres_virtual;
if (mem > info->screen_size)
...
--
Elen sila lumenn' omentielvo
Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] Fix bugs in *_set_par() caused by user input
2022-08-04 14:43 ` [PATCH 0/3] Fix bugs in *_set_par() caused by user input Ondrej Zajicek
@ 2022-08-05 2:48 ` Zheyu Ma
0 siblings, 0 replies; 7+ messages in thread
From: Zheyu Ma @ 2022-08-05 2:48 UTC (permalink / raw)
To: Ondrej Zajicek
Cc: Helge Deller, adaplas, akpm, Linux Fbdev development list,
DRI Development, Linux Kernel Mailing List
Hello,
On Thu, Aug 4, 2022 at 10:43 PM Ondrej Zajicek <santiago@crfreenet.org> wrote:
>
> On Thu, Aug 04, 2022 at 08:41:22PM +0800, Zheyu Ma wrote:
> > In the function *_set_par(), the value of 'screen_size' is
> > calculated by the user input. If the user provides the improper value,
> > the value of 'screen_size' may larger than 'info->screen_size', which
> > may cause a bug in the memset_io().
>
> Hi
>
> I did not saw fbdev code in years, but should not this be already checked
> by *_check_var() ?
>
> arkfb_check_var():
>
> ...
> /* Check whether have enough memory */
> mem = ((var->bits_per_pixel * var->xres_virtual) >> 3) * var->yres_virtual;
> if (mem > info->screen_size)
> ...
Thanks for the reminder. But since the user can control all the
parameters of the ioctl system call, it is possible to assign
'var->bits_per_pixel' to be 0 and thus 'mem' will be 0, bypassing this
check. And in *_set_par(), when 'var->bits_per_pixel' is 0,
'screen_size' will be calculated as follows:
u32 bpp = info->var.bits_per_pixel;
if (bpp != 0) {
...
} else {
...
screen_size = (info->var.xres_virtual * info->var.yres_virtual) / 64;
}
resulting in a very large value.
regards,
Zheyu Ma
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] Fix bugs in *_set_par() caused by user input
2022-08-04 12:41 [PATCH 0/3] Fix bugs in *_set_par() caused by user input Zheyu Ma
` (3 preceding siblings ...)
2022-08-04 14:43 ` [PATCH 0/3] Fix bugs in *_set_par() caused by user input Ondrej Zajicek
@ 2022-08-05 16:47 ` Helge Deller
4 siblings, 0 replies; 7+ messages in thread
From: Helge Deller @ 2022-08-05 16:47 UTC (permalink / raw)
To: Zheyu Ma, adaplas, santiago, akpm; +Cc: linux-fbdev, dri-devel, linux-kernel
On 8/4/22 14:41, Zheyu Ma wrote:
> In the function *_set_par(), the value of 'screen_size' is
> calculated by the user input. If the user provides the improper value,
> the value of 'screen_size' may larger than 'info->screen_size', which
> may cause a bug in the memset_io().
>
> Zheyu Ma (3):
> video: fbdev: vt8623fb: Check the size of screen before memset_io()
> video: fbdev: arkfb: Check the size of screen before memset_io()
> video: fbdev: s3fb: Check the size of screen before memset_io()
applied all 3 patches to fbdev git tree.
Thanks!
Helge
>
> drivers/video/fbdev/arkfb.c | 2 ++
> drivers/video/fbdev/s3fb.c | 2 ++
> drivers/video/fbdev/vt8623fb.c | 2 ++
> 3 files changed, 6 insertions(+)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-05 16:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-04 12:41 [PATCH 0/3] Fix bugs in *_set_par() caused by user input Zheyu Ma
2022-08-04 12:41 ` [PATCH 1/3] video: fbdev: vt8623fb: Check the size of screen before memset_io() Zheyu Ma
2022-08-04 12:41 ` [PATCH 2/3] video: fbdev: arkfb: " Zheyu Ma
2022-08-04 12:41 ` [PATCH 3/3] video: fbdev: s3fb: " Zheyu Ma
2022-08-04 14:43 ` [PATCH 0/3] Fix bugs in *_set_par() caused by user input Ondrej Zajicek
2022-08-05 2:48 ` Zheyu Ma
2022-08-05 16:47 ` Helge Deller
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).