[PATCH v6] fbdev: fbmem: Fix the implicit type casting
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 --- 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
[PATCH v6] fbdev: fbmem: Fix the implicit type casting
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 --- 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
[PATCH v5] fbdev: fbmem: Fix the implicit type casting
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
---
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
[PATCH v5] fbdev: fbmem: Fix the implicit type casting
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
---
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
[PATCH v4] fbdev: fbmem: Fix the implicit type casting
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 --- 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..b3352e23caaa 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) + return -EINVAL; ret = fb_blank(info, arg); /* might again call into fb_blank */ fbcon_fb_blanked(info, arg); -- 2.25.1
[PATCH v4] fbdev: fbmem: Fix the implicit type casting
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 --- 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..b3352e23caaa 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) + return -EINVAL; ret = fb_blank(info, arg); /* might again call into fb_blank */ fbcon_fb_blanked(info, arg); -- 2.25.1
Re: [PATCH] fbdev: fbmem: Fix the implicit type casting
Not at all, I can draft the new patch and resend. Thanks for all the useful discussion : ) On Wed, Feb 2, 2022 at 9:36 AM Helge Deller wrote: > On 2/2/22 18:27, Sam Ravnborg wrote: > > Hi Helge, > > > > On Tue, Feb 01, 2022 at 04:02:40PM +0100, 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. 0x 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. > > > > We should just plug this hole and in case an illegal value is passed > > then return -EINVAL. > > > > Acceptable values are FB_BLANK_UNBLANK..FB_BLANK_POWERDOWN, > > anything less than or greater than should result in -EINVAL. > > Yes, that's the best solution. > Yizhuo Zhai, would you mind to resend with that solution? > > Helge > > > We miss this kind or early checks in many places, and we see the effect > > of this in some drivers where they do it locally for no good. > > > > Sam > -- Kind Regards, *Yizhuo Zhai* *Computer Science, Graduate Student* *University of California, Riverside *
Re: [PATCH] fbdev: fbmem: Fix the implicit type casting
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. 0x 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 *
[PATCH v3] fbdev: fbmem: Fix the implicit type casting
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 --- 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
[PATCH v3] fbdev: fbmem: Fix the implicit type casting
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 --- 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
[PATCH v2] fbdev: fbmem: Fix the implicit type casting
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 --- 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..f08326efff54 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 (blank > FB_BLANK_POWERDOWN) + blank = FB_BLANK_POWERDOWN; ret = fb_blank(info, arg); /* might again call into fb_blank */ fbcon_fb_blanked(info, arg); -- 2.25.1
[PATCH] fbdev: fbmem: Fix the implicit type casting
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.
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)
{
struct fb_event event;
int ret = -EINVAL;
--
2.25.1
[PATCH] drm/amd/display: Fix the uninitialized variable in enable_stream_features()
In function enable_stream_features(), the variable "old_downspread.raw"
could be uninitialized if core_link_read_dpcd() fails, however, it is
used in the later if statement, and further, core_link_write_dpcd()
may write random value, which is potentially unsafe.
Fixes: 6016cd9dba0f ("drm/amd/display: add helper for enabling mst stream
features")
Cc: sta...@vger.kernel.org
Signed-off-by: Yizhuo Zhai
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index c8457babfdea..fd5a0e7eb029 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -1844,6 +1844,8 @@ static void enable_stream_features(struct pipe_ctx
*pipe_ctx)
union down_spread_ctrl old_downspread;
union down_spread_ctrl new_downspread;
+ memset(&old_downspread, 0, sizeof(old_downspread));
+
core_link_read_dpcd(link, DP_DOWNSPREAD_CTRL,
&old_downspread.raw, sizeof(old_downspread));
--
2.25.1
Re: [PATCH] In function nvkm_ioctl_map(), the variable "type" could be uninitialized if "nvkm_object_map()" returns error code, however, it does not check the return value and directly use the "type"
Hi Lyude:
I appreciate your feedback and I misplaced the commit message to the
title, I have modified it and resend the patch.
I made my linux development tree a mess, so I sent a brandly new one
and cc you. Thanks again for your help: )
On Tue, Nov 16, 2021 at 1:18 PM Lyude Paul wrote:
>
> This is a very long patch name, it should probably be shorter and the
> details in the patch title moved into the actual commit description
> instead. Also a couple of things aren't formatted correctly:
>
> * Cc tag for stable is missing, see
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> * Fixes tag isn't formatted properly
>
> I generally recommend using `dim fixes` from
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> in order to get the correct stable kernel CC tag and Fixes: tag (you can
> drop any of the Ccs it gives you beyond the one to stable at vger dot
> kernel dot org.
>
> Also, if you could try to Cc: me on the next version - will help me
> respond faster :).
>
> On Mon, 2021-11-15 at 23:07 -0800, Yizhuo Zhai wrote:
> > Fixes:01326050391ce("drm/nouveau/core/object: allow arguments to
> > be passed to map function")
> > Signed-off-by: Yizhuo Zhai
> > ---
> > drivers/gpu/drm/nouveau/nvkm/core/ioctl.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
> > b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
> > index 735cb6816f10..4264d9d79783 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
> > @@ -266,6 +266,8 @@ nvkm_ioctl_map(struct nvkm_client *client,
> > ret = nvkm_object_map(object, data, size, &type,
> > &args->v0.handle,
> > &args->v0.length);
> > + if (ret)
> > + return ret;
> > if (type == NVKM_OBJECT_MAP_IO)
> > args->v0.type = NVIF_IOCTL_MAP_V0_IO;
> > else
>
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
>
--
Kind Regards,
Yizhuo Zhai
Computer Science, Graduate Student
University of California, Riverside
[PATCH] drm/nouveau/core/object: Fix the uninitialized use of "type"
In function nvkm_ioctl_map(), the variable "type" could be
uninitialized if "nvkm_object_map()" returns error code, however,
it does not check the return value and directly use the "type" in
the if statement, which is potentially unsafe.
Cc: sta...@vger.kernel.org
Fixes: 01326050391c ("drm/nouveau/core/object: allow arguments to be passed to
map function")
Signed-off-by: Yizhuo Zhai
---
drivers/gpu/drm/nouveau/nvkm/core/ioctl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
index 735cb6816f10..4264d9d79783 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
@@ -266,6 +266,8 @@ nvkm_ioctl_map(struct nvkm_client *client,
ret = nvkm_object_map(object, data, size, &type,
&args->v0.handle,
&args->v0.length);
+ if (ret)
+ return ret;
if (type == NVKM_OBJECT_MAP_IO)
args->v0.type = NVIF_IOCTL_MAP_V0_IO;
else
--
2.25.1
[PATCH] In function nvkm_ioctl_map(), the variable "type" could be uninitialized if "nvkm_object_map()" returns error code, however, it does not check the return value and directly use the "type" in t
Fixes:01326050391ce("drm/nouveau/core/object: allow arguments to
be passed to map function")
Signed-off-by: Yizhuo Zhai
---
drivers/gpu/drm/nouveau/nvkm/core/ioctl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
index 735cb6816f10..4264d9d79783 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
@@ -266,6 +266,8 @@ nvkm_ioctl_map(struct nvkm_client *client,
ret = nvkm_object_map(object, data, size, &type,
&args->v0.handle,
&args->v0.length);
+ if (ret)
+ return ret;
if (type == NVKM_OBJECT_MAP_IO)
args->v0.type = NVIF_IOCTL_MAP_V0_IO;
else
--
2.25.1
[PATCH] In function nvkm_ioctl_map(), the variable "type" could be uninitialized if "nvkm_object_map()" returns error code, however, it does not check the return value and directly use the "type" in t
Fixes:01326050391ce("drm/nouveau/core/object: allow arguments to
be passed to map function")
Signed-off-by: Yizhuo Zhai
---
drivers/gpu/drm/nouveau/nvkm/core/ioctl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
index 735cb6816f10..4264d9d79783 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
@@ -266,6 +266,8 @@ nvkm_ioctl_map(struct nvkm_client *client,
ret = nvkm_object_map(object, data, size, &type,
&args->v0.handle,
&args->v0.length);
+ if (ret)
+ return ret;
if (type == NVKM_OBJECT_MAP_IO)
args->v0.type = NVIF_IOCTL_MAP_V0_IO;
else
--
2.25.1
Re: [PATCH] drm/nouveau/core: fix the uninitialized use in nvkm_ioctl_map()
Hi Karol: Thanks for the feedback, the patch might be too old to apply to the latest code tree. Let me check and get back to you soon. On Sat, Nov 13, 2021 at 12:22 PM Karol Herbst wrote: > > something seems to have messed with the patch so it doesn't apply correctly. > > On Thu, Jun 17, 2021 at 9:39 AM Yizhuo Zhai wrote: > > > > In function nvkm_ioctl_map(), the variable "type" could be > > uninitialized if "nvkm_object_map()" returns error code, > > however, it does not check the return value and directly > > use the "type" in the if statement, which is potentially > > unsafe. > > > > Signed-off-by: Yizhuo > > --- > > drivers/gpu/drm/nouveau/nvkm/core/ioctl.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c > > b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c > > index d777df5a64e6..7f2e8482f167 100644 > > --- a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c > > +++ b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c > > @@ -266,6 +266,8 @@ nvkm_ioctl_map(struct nvkm_client *client, > > ret = nvkm_object_map(object, data, size, &type, > > &args->v0.handle, > > &args->v0.length); > > + if (ret) > > + return ret; > > if (type == NVKM_OBJECT_MAP_IO) > > args->v0.type = NVIF_IOCTL_MAP_V0_IO; > > else > > -- > > 2.17.1 > > > -- Kind Regards, Yizhuo Zhai Computer Science, Graduate Student University of California, Riverside
[PATCH] drm/nouveau/core: fix the uninitialized use in nvkm_ioctl_map()
In function nvkm_ioctl_map(), the variable "type" could be uninitialized if "nvkm_object_map()" returns error code, however, it does not check the return value and directly use the "type" in the if statement, which is potentially unsafe. Signed-off-by: Yizhuo --- drivers/gpu/drm/nouveau/nvkm/core/ioctl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c index d777df5a64e6..7f2e8482f167 100644 --- a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c +++ b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c @@ -266,6 +266,8 @@ nvkm_ioctl_map(struct nvkm_client *client, ret = nvkm_object_map(object, data, size, &type, &args->v0.handle, &args->v0.length); + if (ret) + return ret; if (type == NVKM_OBJECT_MAP_IO) args->v0.type = NVIF_IOCTL_MAP_V0_IO; else -- 2.17.1
Potential NULL pointer deference in drm/amdgpu
Hi All: drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c: The function to_amdgpu_fence() could return NULL, but callers in this file does not check the return value but directly dereference it, which seems potentially unsafe. Such callers include amdgpu_fence_get_timeline_name(), amdgpu_fence_enable_signaling() and amdgpu_fence_free(). -- Kind Regards, Yizhuo Zhai Computer Science, Graduate Student University of California, Riverside ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Uninitialized Variable Use in video: fbdev: pm3fb
Hi All: drivers/video/fbdev/pm3fb.c: Inside function pm3fb_write_mode(), variable "m" "n" "p" could leave uninitialized after pm3fb_calculate_clock(), however, they are used later in PM3_WRITE_DAC_REG, which is potentially unsafe. -- Kind Regards, Yizhuo Zhai Computer Science, Graduate Student University of California, Riverside
