Hi Helge: 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. 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. Signed-off-by: Yizhuo Zhai --- 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..991711bfd647 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1162,6 +1162,8 @@ 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) + arg = FB_BLANK_POWERDOWN; ret = fb_blank(info, arg); /* might again call into fb_blank */ fbcon_fb_blanked(info, arg); -- 2.25.1 On Tue, Feb 1, 2022 at 7:03 AM Helge Deller wrote: > On 1/31/22 07:57, Yizhuo Zhai wrote: > > In function do_fb_ioctl(), the "arg" is the type of unsigned long, > > yes, because it comes from the ioctl framework... > > > and in "case FBIOBLANK:" this argument is casted into an int before > > passig to fb_blank(). > > which makes sense IMHO. > > > 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. > > The main problem I see with your patch is that you change the behaviour. > Let's assume someone passes in -1UL. > With your patch applied, this means the -1 (which is e.g. 0xffffffff on > 32bit) > is converted to a positive integer and will be capped to > FB_BLANK_POWERDOWN. > Since most blank functions just check and react on specific values, you > changed > the behaviour that the screen now gets blanked at -1, while it wasn't > before. > > One could now argue, that it's undefined behaviour if people > pass in wrong values, but anyway, it's different now. > > So, your patch isn't wrong. I'm just not sure if this is what we want... > > Helge > > > > Signed-off-by: Yizhuo Zhai > > --- > > drivers/video/fbdev/core/fbmem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/video/fbdev/core/fbmem.c > b/drivers/video/fbdev/core/fbmem.c > > index 0fa7ede94fa6..a5f71c191122 100644 > > --- a/drivers/video/fbdev/core/fbmem.c > > +++ b/drivers/video/fbdev/core/fbmem.c > > @@ -1064,7 +1064,7 @@ fb_set_var(struct fb_info *info, struct > fb_var_screeninfo *var) > > EXPORT_SYMBOL(fb_set_var); > > > > int > > -fb_blank(struct fb_info *info, int blank) > > +fb_blank(struct fb_info *info, unsigned long blank) > > { > -- Kind Regards, *Yizhuo Zhai* *Computer Science, Graduate Student* *University of California, Riverside *