linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] fbdev: fbmem: Fix the implicit type casting
       [not found] <83e46d8d-ec7a-6cbd-010e-7f50f88dcf96@I-love.SAKURA.ne.jp>
@ 2022-02-02 23:33 ` Yizhuo Zhai
  2022-02-02 23:33   ` Yizhuo Zhai
  2022-02-02 23:45   ` Guenter Roeck
  0 siblings, 2 replies; 9+ messages in thread
From: Yizhuo Zhai @ 2022-02-02 23:33 UTC (permalink / raw)
  Cc: Yizhuo Zhai, Helge Deller, Daniel Vetter, Matthew Wilcox,
	Sam Ravnborg, Tetsuo Handa, Zhen Lei, Zheyu Ma, Alex Deucher,
	Xiyu Yang, Guenter Roeck, linux-fbdev, dri-devel, linux-kernel

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 <yzhai003@ucr.edu>
---
 drivers/video/fbdev/core/fbmem.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 0fa7ede94fa6..d5dec24c4d16 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1162,6 +1162,11 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
 	case FBIOBLANK:
 		console_lock();
 		lock_fb_info(info);
+		if (arg > FB_BLANK_POWERDOWN) {
+			unlock_fb_info(info);
+			console_unlock();
+			return -EINVAL;
+		}
 		ret = fb_blank(info, arg);
 		/* might again call into fb_blank */
 		fbcon_fb_blanked(info, arg);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH v5] fbdev: fbmem: Fix the implicit type casting
  2022-02-02 23:33 ` [PATCH v5] fbdev: fbmem: Fix the implicit type casting Yizhuo Zhai
@ 2022-02-02 23:33   ` Yizhuo Zhai
  2022-02-02 23:45   ` Guenter Roeck
  1 sibling, 0 replies; 9+ messages in thread
From: Yizhuo Zhai @ 2022-02-02 23:33 UTC (permalink / raw)
  Cc: Yizhuo Zhai, Helge Deller, Daniel Vetter, Sam Ravnborg,
	Matthew Wilcox, Guenter Roeck, William Kucharski, Tetsuo Handa,
	Zheyu Ma, Zhen Lei, Alex Deucher, Xiyu Yang, linux-fbdev,
	dri-devel, linux-kernel

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 <yzhai003@ucr.edu>
---
 drivers/video/fbdev/core/fbmem.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 0fa7ede94fa6..d5dec24c4d16 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1162,6 +1162,11 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
 	case FBIOBLANK:
 		console_lock();
 		lock_fb_info(info);
+		if (arg > FB_BLANK_POWERDOWN) {
+			unlock_fb_info(info);
+			console_unlock();
+			return -EINVAL;
+		}
 		ret = fb_blank(info, arg);
 		/* might again call into fb_blank */
 		fbcon_fb_blanked(info, arg);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH v5] fbdev: fbmem: Fix the implicit type casting
  2022-02-02 23:33 ` [PATCH v5] fbdev: fbmem: Fix the implicit type casting Yizhuo Zhai
  2022-02-02 23:33   ` Yizhuo Zhai
@ 2022-02-02 23:45   ` Guenter Roeck
  2022-02-02 23:58     ` [PATCH v6] " Yizhuo Zhai
  1 sibling, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2022-02-02 23:45 UTC (permalink / raw)
  To: Yizhuo Zhai
  Cc: Helge Deller, Daniel Vetter, Matthew Wilcox, Sam Ravnborg,
	Tetsuo Handa, Zhen Lei, Zheyu Ma, Alex Deucher, Xiyu Yang,
	linux-fbdev, dri-devel, linux-kernel

On 2/2/22 15:33, 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 <yzhai003@ucr.edu>
> ---
>   drivers/video/fbdev/core/fbmem.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 0fa7ede94fa6..d5dec24c4d16 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1162,6 +1162,11 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>   	case FBIOBLANK:
>   		console_lock();
>   		lock_fb_info(info);
> +		if (arg > FB_BLANK_POWERDOWN) {
> +			unlock_fb_info(info);
> +			console_unlock();
> +			return -EINVAL;
> +		}

Is it really necessary to perform this check within the lock ?
arg is a passed in value, and FB_BLANK_POWERDOWN.
It seems to me that

	case FBIOBLANK:
		if (arg > FB_BLANK_POWERDOWN)
			return -EINVAL;
		console_lock();
		...

should be sufficient.

Sorry if I missed some previous discussion; this is the first time
I looked at this patch.

Guenter

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v6] fbdev: fbmem: Fix the implicit type casting
  2022-02-02 23:45   ` Guenter Roeck
@ 2022-02-02 23:58     ` Yizhuo Zhai
  2022-02-02 23:58       ` Yizhuo Zhai
  2022-02-03  6:39       ` Sam Ravnborg
  0 siblings, 2 replies; 9+ messages in thread
From: Yizhuo Zhai @ 2022-02-02 23:58 UTC (permalink / raw)
  Cc: Yizhuo Zhai, Helge Deller, Daniel Vetter, Sam Ravnborg,
	Matthew Wilcox, Alex Deucher, Xin Tan, Xiyu Yang, Tetsuo Handa,
	Zhen Lei, Zheyu Ma, Guenter Roeck, linux-fbdev, dri-devel,
	linux-kernel

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 <yzhai003@ucr.edu>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH v6] fbdev: fbmem: Fix the implicit type casting
  2022-02-02 23:58     ` [PATCH v6] " Yizhuo Zhai
@ 2022-02-02 23:58       ` Yizhuo Zhai
  2022-02-03  1:07         ` Guenter Roeck
  2022-02-03  6:39       ` Sam Ravnborg
  1 sibling, 1 reply; 9+ messages in thread
From: Yizhuo Zhai @ 2022-02-02 23:58 UTC (permalink / raw)
  Cc: Yizhuo Zhai, Helge Deller, Daniel Vetter, Matthew Wilcox,
	Sam Ravnborg, Xiyu Yang, Guenter Roeck, Tetsuo Handa,
	Alex Deucher, Zhen Lei, Zheyu Ma, linux-fbdev, dri-devel,
	linux-kernel

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 <yzhai003@ucr.edu>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH v6] fbdev: fbmem: Fix the implicit type casting
  2022-02-02 23:58       ` Yizhuo Zhai
@ 2022-02-03  1:07         ` Guenter Roeck
  0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2022-02-03  1:07 UTC (permalink / raw)
  To: Yizhuo Zhai
  Cc: Helge Deller, Daniel Vetter, Matthew Wilcox, Sam Ravnborg,
	Xiyu Yang, Tetsuo Handa, Alex Deucher, Zhen Lei, Zheyu Ma,
	linux-fbdev, dri-devel, linux-kernel

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 <yzhai003@ucr.edu>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v6] fbdev: fbmem: Fix the implicit type casting
  2022-02-02 23:58     ` [PATCH v6] " Yizhuo Zhai
  2022-02-02 23:58       ` Yizhuo Zhai
@ 2022-02-03  6:39       ` Sam Ravnborg
  2022-02-03  8:18         ` Helge Deller
  1 sibling, 1 reply; 9+ messages in thread
From: Sam Ravnborg @ 2022-02-03  6:39 UTC (permalink / raw)
  To: Yizhuo Zhai, Daniel Vetter
  Cc: Helge Deller, Daniel Vetter, Matthew Wilcox, Alex Deucher,
	Xin Tan, Xiyu Yang, Tetsuo Handa, Zhen Lei, Zheyu Ma,
	Guenter Roeck, linux-fbdev, dri-devel, linux-kernel

Hi Daniel,

I assume you will take this.

Patch is:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	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 <yzhai003@ucr.edu>
> ---
>  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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v6] fbdev: fbmem: Fix the implicit type casting
  2022-02-03  6:39       ` Sam Ravnborg
@ 2022-02-03  8:18         ` Helge Deller
  2022-02-03 12:51           ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Helge Deller @ 2022-02-03  8:18 UTC (permalink / raw)
  To: Sam Ravnborg, Yizhuo Zhai, Daniel Vetter
  Cc: Matthew Wilcox, Alex Deucher, Xin Tan, Xiyu Yang, Tetsuo Handa,
	Zhen Lei, Zheyu Ma, Guenter Roeck, linux-fbdev, dri-devel,
	linux-kernel

On 2/3/22 07:39, Sam Ravnborg wrote:
> Hi Daniel,
>
> I assume you will take this.
>
> Patch is:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>


Acked-by: Helge Deller <deller@gmx.de>

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 <yzhai003@ucr.edu>
>> ---
>>  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


^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v6] fbdev: fbmem: Fix the implicit type casting
  2022-02-03  8:18         ` Helge Deller
@ 2022-02-03 12:51           ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2022-02-03 12:51 UTC (permalink / raw)
  To: Helge Deller
  Cc: Sam Ravnborg, Yizhuo Zhai, Daniel Vetter, Matthew Wilcox,
	Alex Deucher, Xin Tan, Xiyu Yang, Tetsuo Handa, Zhen Lei,
	Zheyu Ma, Guenter Roeck, linux-fbdev, dri-devel, linux-kernel

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 <sam@ravnborg.org>
> 
> 
> Acked-by: Helge Deller <deller@gmx.de>

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 <yzhai003@ucr.edu>
> >> ---
> >>  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

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-06  0:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <83e46d8d-ec7a-6cbd-010e-7f50f88dcf96@I-love.SAKURA.ne.jp>
2022-02-02 23:33 ` [PATCH v5] fbdev: fbmem: Fix the implicit type casting Yizhuo Zhai
2022-02-02 23:33   ` Yizhuo Zhai
2022-02-02 23:45   ` Guenter Roeck
2022-02-02 23:58     ` [PATCH v6] " Yizhuo Zhai
2022-02-02 23:58       ` Yizhuo Zhai
2022-02-03  1:07         ` Guenter Roeck
2022-02-03  6:39       ` Sam Ravnborg
2022-02-03  8:18         ` Helge Deller
2022-02-03 12:51           ` Daniel Vetter

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).