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

[Colin Xu]

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