| Message ID | 20250327124739.2609656-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/gvt: fix unterminated-string-initialization warning | expand |
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 >>
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;