<div dir="ltr">Hi Helge:<div>I shipped a new patch which moves the check before the function call, please take a look and see if this one makes sense to you.</div><div>Modifying the type of function argument is a bit risky because fb_blank() has more than one caller and some of them passed in an integer.</div><div><br></div><div>Signed-off-by: Yizhuo Zhai <<a href="mailto:yzhai003@ucr.edu" target="_blank">yzhai003@ucr.edu</a>><br>---<br> drivers/video/fbdev/core/fbmem.c | 2 ++<br> 1 file changed, 2 insertions(+)<br><br>diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c<br>index 0fa7ede94fa6..991711bfd647 100644<br>--- a/drivers/video/fbdev/core/fbmem.c<br>+++ b/drivers/video/fbdev/core/fbmem.c<br>@@ -1162,6 +1162,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,<br>        case FBIOBLANK:<br>                console_lock();<br>                lock_fb_info(info);<br>+               if (arg > FB_BLANK_POWERDOWN)<br>+                       arg = FB_BLANK_POWERDOWN;<br>                ret = fb_blank(info, arg);<br>                /* might again call into fb_blank */<br>                fbcon_fb_blanked(info, arg);<font color="#888888"><br>--<span class="gmail-Apple-converted-space"> </span><br>2.25.1</font><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Feb 1, 2022 at 7:03 AM Helge Deller <<a href="mailto:deller@gmx.de">deller@gmx.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">On 1/31/22 07:57, Yizhuo Zhai wrote:<br>
> In function do_fb_ioctl(), the "arg" is the type of unsigned long,<br>
<br>
yes, because it comes from the ioctl framework...<br>
<br>
> and in "case FBIOBLANK:" this argument is casted into an int before<br>
> passig to fb_blank().<br>
<br>
which makes sense IMHO.<br>
<br>
> In fb_blank(), the comparision if (blank > FB_BLANK_POWERDOWN) would<br>
> be bypass if the original "arg" is a large number, which is possible<br>
> because it comes from the user input.<br>
<br>
The main problem I see with your patch is that you change the behaviour.<br>
Let's assume someone passes in -1UL.<br>
With your patch applied, this means the -1 (which is e.g. 0xffffffff on 32bit)<br>
is converted to a positive integer and will be capped to FB_BLANK_POWERDOWN.<br>
Since most blank functions just check and react on specific values, you changed<br>
the behaviour that the screen now gets blanked at -1, while it wasn't before.<br>
<br>
One could now argue, that it's undefined behaviour if people<br>
pass in wrong values, but anyway, it's different now.<br>
<br>
So, your patch isn't wrong. I'm just not sure if this is what we want...<br>
<br>
Helge<br>
<br>
<br>
> Signed-off-by: Yizhuo Zhai <<a href="mailto:yzhai003@ucr.edu" target="_blank">yzhai003@ucr.edu</a>><br>
> ---<br>
>  drivers/video/fbdev/core/fbmem.c | 2 +-<br>
>  1 file changed, 1 insertion(+), 1 deletion(-)<br>
><br>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c<br>
> index 0fa7ede94fa6..a5f71c191122 100644<br>
> --- a/drivers/video/fbdev/core/fbmem.c<br>
> +++ b/drivers/video/fbdev/core/fbmem.c<br>
> @@ -1064,7 +1064,7 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)<br>
>  EXPORT_SYMBOL(fb_set_var);<br>
><br>
>  int<br>
> -fb_blank(struct fb_info *info, int blank)<br>
> +fb_blank(struct fb_info *info, unsigned long blank)<br>
>  {<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><span style="font-size:14px">Kind Regards,</span><div style="font-size:14px"><br><div><font face="arial, helvetica, sans-serif" size="2"><b>Yizhuo Zhai</b></font></div></div><div style="font-size:14px"><br></div><div style="font-size:14px"><b>Computer Science, Graduate Student</b></div><div style="font-size:14px"><b>University of California, Riverside </b></div></div></div>