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.
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 kees@kernel.org Closes: https://lore.kernel.org/r/20250310222355.work.417-kees@kernel.org Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13934 Tested-by: Nicolas Chauvet kwizart@gmail.com Tested-by: Damian Tometzki damian@riscv-rocks.de Cc: stable@vger.kernel.org Signed-off-by: Jani Nikula jani.nikula@intel.com --- 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;
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 kees@kernel.org Closes: https://lore.kernel.org/r/20250310222355.work.417-kees@kernel.org Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13934 Tested-by: Nicolas Chauvet kwizart@gmail.com Tested-by: Damian Tometzki damian@riscv-rocks.de Cc: stable@vger.kernel.org Signed-off-by: Jani Nikula jani.nikula@intel.com
Reviewed-by: Zhenyu Wang zhenyuw.linux@gmail.com
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
On Sun, 30 Mar 2025, Zhenyu Wang zhenyuw.linux@gmail.com 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 kees@kernel.org Closes: https://lore.kernel.org/r/20250310222355.work.417-kees@kernel.org Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13934 Tested-by: Nicolas Chauvet kwizart@gmail.com Tested-by: Damian Tometzki damian@riscv-rocks.de Cc: stable@vger.kernel.org Signed-off-by: Jani Nikula jani.nikula@intel.com
Reviewed-by: Zhenyu Wang zhenyuw.linux@gmail.com
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
linux-stable-mirror@lists.linaro.org
-
Jani Nikula -
Zhenyu Wang