On Sun, 30 Mar 2025, Zhenyu Wang <[email protected]> wrote:
> On Thu, Mar 27, 2025 at 02:47:39PM +0200, Jani Nikula wrote:
>> Initializing const char opregion_signature[16] = OPREGION_SIGNATURE
>> (which is "IntelGraphicsMem") drops the NUL termination of the
>> string. This is intentional, but the compiler doesn't know this.
>>
>
> Indeed...
>
>> Switch to initializing header->signature directly from the string
>> litaral, with sizeof destination rather than source. We don't treat the
>> signature as a string other than for initialization; it's really just a
>> blob of binary data.
>>
>> Add a static assert for good measure to cross-check the sizes.
>>
>> Reported-by: Kees Cook <[email protected]>
>> Closes: https://lore.kernel.org/r/[email protected]
>> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13934
>> Tested-by: Nicolas Chauvet <[email protected]>
>> Tested-by: Damian Tometzki <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: Jani Nikula <[email protected]>
>> ---
>
> Reviewed-by: Zhenyu Wang <[email protected]>
Thanks for the review, pushed to din.
BR,
Jani.
>
>> drivers/gpu/drm/i915/gvt/opregion.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/opregion.c
>> b/drivers/gpu/drm/i915/gvt/opregion.c
>> index 509f9ccae3a9..dbad4d853d3a 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,10 @@ 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));
>> +
>> + static_assert(sizeof(header->signature) == sizeof(OPREGION_SIGNATURE) -
>> 1);
>> + memcpy(header->signature, OPREGION_SIGNATURE,
>> sizeof(header->signature));
>> +
>> header->size = 0x8;
>> header->opregion_ver = 0x02000000;
>> header->mboxes = MBOX_VBT;
>> --
>> 2.39.5
>>
--
Jani Nikula, Intel
