From: Helge Deller <deller@gmx.de> To: Yizhuo Zhai <yzhai003@ucr.edu> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, Sam Ravnborg <sam@ravnborg.org>, Matthew Wilcox <willy@infradead.org>, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>, Alex Deucher <alexander.deucher@amd.com>, Zhen Lei <thunder.leizhen@huawei.com>, Zheyu Ma <zheyuma97@gmail.com>, Xiyu Yang <xiyuyang19@fudan.edu.cn>, linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] fbdev: fbmem: Fix the implicit type casting Date: Tue, 1 Feb 2022 16:02:40 +0100 [thread overview] Message-ID: <b1c1f68d-4620-2429-66bd-33d806d31457@gmx.de> (raw) In-Reply-To: <20220131065719.1552958-1-yzhai003@ucr.edu> 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 <yzhai003@ucr.edu> > --- > 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) > {
WARNING: multiple messages have this Message-ID (diff)
From: Helge Deller <deller@gmx.de> To: Yizhuo Zhai <yzhai003@ucr.edu> Cc: linux-fbdev@vger.kernel.org, Xiyu Yang <xiyuyang19@fudan.edu.cn>, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>, Daniel Vetter <daniel.vetter@ffwll.ch>, Zheyu Ma <zheyuma97@gmail.com>, linux-kernel@vger.kernel.org, Matthew Wilcox <willy@infradead.org>, dri-devel@lists.freedesktop.org, Zhen Lei <thunder.leizhen@huawei.com>, Alex Deucher <alexander.deucher@amd.com>, Sam Ravnborg <sam@ravnborg.org> Subject: Re: [PATCH] fbdev: fbmem: Fix the implicit type casting Date: Tue, 1 Feb 2022 16:02:40 +0100 [thread overview] Message-ID: <b1c1f68d-4620-2429-66bd-33d806d31457@gmx.de> (raw) In-Reply-To: <20220131065719.1552958-1-yzhai003@ucr.edu> 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 <yzhai003@ucr.edu> > --- > 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) > {
next prev parent reply other threads:[~2022-02-01 15:03 UTC|newest] Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-31 6:57 [PATCH] fbdev: fbmem: Fix the implicit type casting Yizhuo Zhai 2022-01-31 6:57 ` Yizhuo Zhai 2022-01-31 10:55 ` kernel test robot 2022-01-31 10:55 ` kernel test robot 2022-01-31 10:55 ` kernel test robot 2022-01-31 11:36 ` kernel test robot 2022-01-31 11:36 ` kernel test robot 2022-01-31 11:36 ` kernel test robot 2022-01-31 12:27 ` kernel test robot 2022-01-31 12:27 ` kernel test robot 2022-01-31 12:27 ` kernel test robot 2022-02-01 2:35 ` [PATCH v2] " Yizhuo Zhai 2022-02-01 2:35 ` Yizhuo Zhai 2022-02-02 9:02 ` David Laight 2022-02-02 9:02 ` David Laight 2022-02-01 15:02 ` Helge Deller [this message] 2022-02-01 15:02 ` [PATCH] " Helge Deller 2022-02-01 22:37 ` Yizhuo Zhai 2022-02-02 17:27 ` Sam Ravnborg 2022-02-02 17:27 ` Sam Ravnborg 2022-02-02 17:36 ` Helge Deller 2022-02-02 17:36 ` Helge Deller 2022-02-02 22:58 ` Yizhuo Zhai 2022-02-02 23:16 ` [PATCH v4] " Yizhuo Zhai 2022-02-02 23:16 ` Yizhuo Zhai 2022-02-02 23:16 ` Yizhuo Zhai 2022-02-02 23:16 ` Yizhuo Zhai
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b1c1f68d-4620-2429-66bd-33d806d31457@gmx.de \ --to=deller@gmx.de \ --cc=alexander.deucher@amd.com \ --cc=daniel.vetter@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-fbdev@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=penguin-kernel@i-love.sakura.ne.jp \ --cc=sam@ravnborg.org \ --cc=thunder.leizhen@huawei.com \ --cc=willy@infradead.org \ --cc=xiyuyang19@fudan.edu.cn \ --cc=yzhai003@ucr.edu \ --cc=zheyuma97@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.