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

Submitted by Aleksei Gimbitskii on April 8, 2019, 5:54 a.m.

Details

Message ID 20190408055502.8474-2-aleksei.gimbitskii@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Aleksei Gimbitskii April 8, 2019, 5:54 a.m.
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@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/gvt/dmabuf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

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));
 
 	dmabuf_obj->vgpu = vgpu;
 

Comments

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@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@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;
>
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@intel.com>; intel-gvt-dev@lists.freedesktop.org
Cc: Wang, Zhi A <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@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@intel.com>

> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>

> Cc: Zhi Wang <zhi.a.wang@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.