| Message ID | 20230314-vmware-wuninitialized-v1-1-1bb4b0989758@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vmwgfx: Fix uninitialized use of dst_pitch in vmw_stdu_bo_cpu_commit() | expand |
On Tue, 2023-03-14 at 08:38 -0700, Nathan Chancellor wrote: > Clang warns (or errors with CONFIG_WERROR): > > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:509:29: error: variable 'dst_pitch' is > uninitialized when used here [-Werror,-Wuninitialized] > src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; > ^~~~~~~~~ > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:492:26: note: initialize the variable > 'dst_pitch' to silence this warning > s32 src_pitch, dst_pitch; > ^ > = 0 > 1 error generated. > > The assignments were switched around in such a way that dst_pitch was > used before it was assigned. Swap the pitch assignments to fix the issue > and make it clear which section they are used in. > > Fixes: 39985eea5a6d ("drm/vmwgfx: Abstract placement selection") > Link: > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F1811&data=05%7C01%7Czackr%40vmware.com%7C37398ac97246436c2efc08db24a226d5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C638144051125910171%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HtZTNLdmCAzHfRoEG6vsWcSul4YRSZ56BO2ZUHnUCj4%3D&reserved=0 > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > --- > I am not sure if this is the right fix, as it was not entirely clear to > me that src_pitch and dst_pitch were being used in the right assignments > but this is the obvious fix otherwise. Consider this a bug report if > not :) > --- > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > index d79a6eccfaa4..030e977c68e2 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > @@ -504,11 +504,11 @@ static void vmw_stdu_bo_cpu_commit(struct vmw_kms_dirty > *dirty) > return; > > /* Assume we are blitting from Guest (bo) to Host (display_srf) */ > - src_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp; > + dst_pitch = ddirty->pitch; > src_bo = &stdu->display_srf->res.guest_memory_bo->tbo; > src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; > > - dst_pitch = ddirty->pitch; > + src_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp; > dst_bo = &ddirty->buf->tbo; > dst_offset = ddirty->fb_top * src_pitch + ddirty->fb_left * stdu->cpp; Thanks Nathan! In this case we just want to switch the usage instead of what those variables are. I'll send a patch out in a second. z
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c index d79a6eccfaa4..030e977c68e2 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c @@ -504,11 +504,11 @@ static void vmw_stdu_bo_cpu_commit(struct vmw_kms_dirty *dirty) return; /* Assume we are blitting from Guest (bo) to Host (display_srf) */ - src_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp; + dst_pitch = ddirty->pitch; src_bo = &stdu->display_srf->res.guest_memory_bo->tbo; src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; - dst_pitch = ddirty->pitch; + src_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp; dst_bo = &ddirty->buf->tbo; dst_offset = ddirty->fb_top * src_pitch + ddirty->fb_left * stdu->cpp;
Clang warns (or errors with CONFIG_WERROR): drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:509:29: error: variable 'dst_pitch' is uninitialized when used here [-Werror,-Wuninitialized] src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; ^~~~~~~~~ drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:492:26: note: initialize the variable 'dst_pitch' to silence this warning s32 src_pitch, dst_pitch; ^ = 0 1 error generated. The assignments were switched around in such a way that dst_pitch was used before it was assigned. Swap the pitch assignments to fix the issue and make it clear which section they are used in. Fixes: 39985eea5a6d ("drm/vmwgfx: Abstract placement selection") Link: https://github.com/ClangBuiltLinux/linux/issues/1811 Signed-off-by: Nathan Chancellor <nathan@kernel.org> --- I am not sure if this is the right fix, as it was not entirely clear to me that src_pitch and dst_pitch were being used in the right assignments but this is the obvious fix otherwise. Consider this a bug report if not :) --- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- base-commit: c87e859cdeb5d106cb861326e3135c606d61f88d change-id: 20230314-vmware-wuninitialized-28666fb5a62b Best regards,