Re: [PATCH 1/3] Revert "drm/i915/gvt: Fix out-of-bounds buffer write into opregion->signature[]"

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 24 Mar 2025, Nicolas Chauvet <kwizart@gmail.com> wrote:
> This reverts commit ea26c96d59b27e878fe61e8ef0fed840d2281a2f.
>
> This fix truncates the OPREGION_SIGNATURE to fit into 16 chars instead of
> enlarging the target field, hence only moving the size missmatch to later.
>
> As shown with gcc-15:
> drivers/gpu/drm/i915/gvt/opregion.c: In function intel_vgpu_init_opregion:
> drivers/gpu/drm/i915/gvt/opregion.c:35:28: error: initializer-string for array of char is too long [-Werror=unterminated-string-initialization]
>    35 | #define OPREGION_SIGNATURE "IntelGraphicsMem"
>       |                            ^~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/i915/gvt/opregion.c:225:45: note: in expansion of macro OPREGION_SIGNATURE
>   225 |         const char opregion_signature[16] = OPREGION_SIGNATURE;
>       |                                             ^~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Cc: stable@vger.kernel.org
> Reported-by: Nicolas Chauvet <kwizart@gmail.com>
> Fixes: ea26c96d59 ("drm/i915/gvt: Fix out-of-bounds buffer write into opregion->signature[]")
> Signed-off-by: Nicolas Chauvet <kwizart@gmail.com>

This introduces a buffer overflow.

sizeof(OPREGION_SIGNATURE) == 17.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/gvt/opregion.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c
> index 509f9ccae3a9..9a8ead6039e2 100644
> --- a/drivers/gpu/drm/i915/gvt/opregion.c
> +++ b/drivers/gpu/drm/i915/gvt/opregion.c
> @@ -222,7 +222,6 @@ int intel_vgpu_init_opregion(struct intel_vgpu *vgpu)
>  	u8 *buf;
>  	struct opregion_header *header;
>  	struct vbt v;
> -	const char opregion_signature[16] = OPREGION_SIGNATURE;
>  
>  	gvt_dbg_core("init vgpu%d opregion\n", vgpu->id);
>  	vgpu_opregion(vgpu)->va = (void *)__get_free_pages(GFP_KERNEL |
> @@ -236,8 +235,8 @@ int intel_vgpu_init_opregion(struct intel_vgpu *vgpu)
>  	/* emulated opregion with VBT mailbox only */
>  	buf = (u8 *)vgpu_opregion(vgpu)->va;
>  	header = (struct opregion_header *)buf;
> -	memcpy(header->signature, opregion_signature,
> -	       sizeof(opregion_signature));
> +	memcpy(header->signature, OPREGION_SIGNATURE,
> +			sizeof(OPREGION_SIGNATURE));
>  	header->size = 0x8;
>  	header->opregion_ver = 0x02000000;
>  	header->mboxes = MBOX_VBT;

-- 
Jani Nikula, Intel