[PATCH 3/5] drm/i915/gvt: Use snprintf() to prevent possible buffer overflow.

[Gimbitskii, Aleksei]

Thanks for your review. The buffer in new patch version is increased.

BR, Aleksei.

-----Original Message-----
From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On Behalf Of Colin Xu
Sent: Monday, April 8, 2019 10:17 AM
To: Gimbitskii, Aleksei <aleksei.gimbitskii at intel.com>; intel-gvt-dev at lists.freedesktop.org
Cc: Zhenyu Wang <zhenyuw at linux.intel.com>; Wang, Zhi A <zhi.a.wang at intel.com>
Subject: Re: [PATCH 3/5] drm/i915/gvt: Use snprintf() to prevent possible buffer overflow.


On 2019-04-08 13:55, Aleksei Gimbitskii wrote:
> For printing the intel_vgpu->id, a buffer with fixed length is 
> allocated on the stack. But if vgpu->id is greater than 6 characters, 
> the buffer overflow will happen. Even the string of the amount of max 
> vgpu is less that the length buffer right now, it's better to replace 
> sprintf() with snprintf().
>
> This patch fixed the critical issue #673 reported by klocwork.
>
> 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/debugfs.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/debugfs.c 
> b/drivers/gpu/drm/i915/gvt/debugfs.c
> index 2ec89bcb59f1..51b2705018d5 100644
> --- a/drivers/gpu/drm/i915/gvt/debugfs.c
> +++ b/drivers/gpu/drm/i915/gvt/debugfs.c
> @@ -198,7 +198,7 @@ int intel_gvt_debugfs_add_vgpu(struct intel_vgpu *vgpu)
>   	struct dentry *ent;
>   	char name[10] = "";
>   
> -	sprintf(name, "vgpu%d", vgpu->id);
> +	snprintf(name, 10, "vgpu%d", vgpu->id);

Secure function like snprintf is always preferred over non-secure variants.
Current implementation defines intel_vgpu->id as int, which indicates although
type8 is the max number of vgpu that gvt allows, we could increase up limit to more than 8. In that case buf size of 10 will be insufficient.
In addition to use snprintf, it will be better to increase the buf size that could hold max possible value of intel_vgpu->id type.

>   	vgpu->debugfs = debugfs_create_dir(name, vgpu->gvt->debugfs_root);
>   	if (!vgpu->debugfs)
>   		return -ENOMEM;

--
Best Regards,
Colin Xu

_______________________________________________
intel-gvt-dev mailing list
intel-gvt-dev at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
---------------------------------------------------------------------
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.