In function do_fb_ioctl(), the "arg" is the type of unsigned long,
and in "case FBIOBLANK:" this argument is casted into an int before
passig to fb_blank(). In fb_blank(), the comparision
if (blank > FB_BLANK_POWERDOWN) would be bypass if the original
"arg" is a large number, which is possible because it comes from
the user input. Fix this by adding the check before the function
call.
Signed-off-by: Yizhuo Zhai <[email protected]>
---
drivers/video/fbdev/core/fbmem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 0fa7ede94fa6..13083ad8d751 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1160,6 +1160,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
ret = fbcon_set_con2fb_map_ioctl(argp);
break;
case FBIOBLANK:
+ if (arg > FB_BLANK_POWERDOWN)
+ return -EINVAL;
console_lock();
lock_fb_info(info);
ret = fb_blank(info, arg);
--
2.25.1
On Wed, Feb 02, 2022 at 03:58:09PM -0800, Yizhuo Zhai wrote:
> In function do_fb_ioctl(), the "arg" is the type of unsigned long,
> and in "case FBIOBLANK:" this argument is casted into an int before
> passig to fb_blank(). In fb_blank(), the comparision
> if (blank > FB_BLANK_POWERDOWN) would be bypass if the original
> "arg" is a large number, which is possible because it comes from
> the user input. Fix this by adding the check before the function
> call.
>
> Signed-off-by: Yizhuo Zhai <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
On a side note, change logs would be useful.
Guenter
> ---
> drivers/video/fbdev/core/fbmem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 0fa7ede94fa6..13083ad8d751 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1160,6 +1160,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> ret = fbcon_set_con2fb_map_ioctl(argp);
> break;
> case FBIOBLANK:
> + if (arg > FB_BLANK_POWERDOWN)
> + return -EINVAL;
> console_lock();
> lock_fb_info(info);
> ret = fb_blank(info, arg);
> --
> 2.25.1
>
On 2/3/22 07:39, Sam Ravnborg wrote:
> Hi Daniel,
>
> I assume you will take this.
>
> Patch is:
> Reviewed-by: Sam Ravnborg <[email protected]>
Acked-by: Helge Deller <[email protected]>
Helge
>
> Sam
>
> On Wed, Feb 02, 2022 at 03:58:08PM -0800, Yizhuo Zhai wrote:
>> In function do_fb_ioctl(), the "arg" is the type of unsigned long,
>> and in "case FBIOBLANK:" this argument is casted into an int before
>> passig to fb_blank(). In fb_blank(), the comparision
>> if (blank > FB_BLANK_POWERDOWN) would be bypass if the original
>> "arg" is a large number, which is possible because it comes from
>> the user input. Fix this by adding the check before the function
>> call.
>>
>> Signed-off-by: Yizhuo Zhai <[email protected]>
>> ---
>> drivers/video/fbdev/core/fbmem.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
>> index 0fa7ede94fa6..13083ad8d751 100644
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -1160,6 +1160,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>> ret = fbcon_set_con2fb_map_ioctl(argp);
>> break;
>> case FBIOBLANK:
>> + if (arg > FB_BLANK_POWERDOWN)
>> + return -EINVAL;
>> console_lock();
>> lock_fb_info(info);
>> ret = fb_blank(info, arg);
>> --
>> 2.25.1
On Thu, Feb 03, 2022 at 09:18:30AM +0100, Helge Deller wrote:
> On 2/3/22 07:39, Sam Ravnborg wrote:
> > Hi Daniel,
> >
> > I assume you will take this.
> >
> > Patch is:
> > Reviewed-by: Sam Ravnborg <[email protected]>
>
>
> Acked-by: Helge Deller <[email protected]>
Pushed to drm-misc-fixes, thanks for patch&review.
-Daniel
>
> Helge
>
> >
> > Sam
> >
> > On Wed, Feb 02, 2022 at 03:58:08PM -0800, Yizhuo Zhai wrote:
> >> In function do_fb_ioctl(), the "arg" is the type of unsigned long,
> >> and in "case FBIOBLANK:" this argument is casted into an int before
> >> passig to fb_blank(). In fb_blank(), the comparision
> >> if (blank > FB_BLANK_POWERDOWN) would be bypass if the original
> >> "arg" is a large number, which is possible because it comes from
> >> the user input. Fix this by adding the check before the function
> >> call.
> >>
> >> Signed-off-by: Yizhuo Zhai <[email protected]>
> >> ---
> >> drivers/video/fbdev/core/fbmem.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> >> index 0fa7ede94fa6..13083ad8d751 100644
> >> --- a/drivers/video/fbdev/core/fbmem.c
> >> +++ b/drivers/video/fbdev/core/fbmem.c
> >> @@ -1160,6 +1160,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> >> ret = fbcon_set_con2fb_map_ioctl(argp);
> >> break;
> >> case FBIOBLANK:
> >> + if (arg > FB_BLANK_POWERDOWN)
> >> + return -EINVAL;
> >> console_lock();
> >> lock_fb_info(info);
> >> ret = fb_blank(info, arg);
> >> --
> >> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch