[PATCH 2/5] drm/i915/gvt: Do not copy the uninitialized pointer from fb_info

[Gimbitskii, Aleksei]

Thanks for you proposal. As you recommended, in next patch version I will initialize fb_info.obj in vgpu_get_plane_info(), it solves this issue in better manner and more consistent with other code.

- BR, Aleksei.

-----Original Message-----
From: Xu, Colin 
Sent: Monday, April 8, 2019 10:06 AM
To: Gimbitskii, Aleksei <aleksei.gimbitskii at intel.com>; intel-gvt-dev at lists.freedesktop.org
Cc: Wang, Zhi A <zhi.a.wang at intel.com>; Zhenyu Wang <zhenyuw at linux.intel.com>
Subject: Re: [PATCH 2/5] drm/i915/gvt: Do not copy the uninitialized pointer from fb_info


On 2019-04-08 13:54, Aleksei Gimbitskii wrote:
> In the code the memcpy() function copied uninitialized pointer in 
> fb_info to dmabuf_obj->info. Later the pointer in dmabuf_obj->info 
> will be initialized. To make the code aligned with requirements of the 
> klocwork static code analyzer, the uninitialized pointer should be 
> initialized before memcpy().
>
> This patch fixed the critical issue #632 reported by klockwork.
>
> Signed-off-by: Aleksei Gimbitskii <aleksei.gimbitskii at intel.com>
> Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang at intel.com>
> ---
>   drivers/gpu/drm/i915/gvt/dmabuf.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c 
> b/drivers/gpu/drm/i915/gvt/dmabuf.c
> index f6be97119968..d86055c144c2 100644
> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> @@ -418,9 +418,9 @@ int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args)
>   		ret = -ENOMEM;
>   		goto out_free_dmabuf;
>   	}
> -	memcpy(dmabuf_obj->info, &fb_info, sizeof(struct intel_vgpu_fb_info));
>   
> -	((struct intel_vgpu_fb_info *)dmabuf_obj->info)->obj = dmabuf_obj;
> +	fb_info.obj = dmabuf_obj;
> +	memcpy(dmabuf_obj->info, &fb_info, sizeof(struct 
> +intel_vgpu_fb_info));
>   

Since dmabuf_obj->info is cloned from fb_info, and obj is assigned the newly alloced dmabuf_obj in this same function, regardless what fb_info->obj original was, will it be better to initialize fb_info->obj in vgpu_get_plane_info() like other members?
Like a simple zero-out fb_info before assign members, or an explicit NULL to obj?

>   	dmabuf_obj->vgpu = vgpu;
>   

--
Best Regards,
Colin Xu

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.