| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABr+WTkggOTDDzgPFmnJo3Dab4QYxLRt=_g7in3bgr0z6jXf+g@mail.gmail.com>
Date: Mon, 24 Mar 2025 22:19:23 +0100
From: Nicolas Chauvet <kwizart@...il.com>
To: Jani Nikula <jani.nikula@...ux.intel.com>
Cc: Damian Tometzki <damian@...cv-rocks.de>, Kees Cook <kees@...nel.org>,
Zhenyu Wang <zhenyuw@...ux.intel.com>, Zhi Wang <zhi.wang.linux@...il.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>, Rodrigo Vivi <rodrigo.vivi@...el.com>,
Tvrtko Ursulin <tursulin@...ulin.net>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, intel-gvt-dev@...ts.freedesktop.org,
intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] drm/i915/gvt: Add __nonstring annotations for
unterminated strings
Le lun. 24 mars 2025 à 13:54, Jani Nikula
<jani.nikula@...ux.intel.com> a écrit :
>
> On Sun, 23 Mar 2025, Damian Tometzki <damian@...cv-rocks.de> wrote:
> > On Mon, 10. Mar 15:23, Kees Cook wrote:
> >> When a character array without a terminating NUL character has a static
> >> initializer, GCC 15's -Wunterminated-string-initialization will only
> >> warn if the array lacks the "nonstring" attribute[1]. Mark the arrays
> >> with __nonstring to and correctly identify the char array as "not a C
> >> string" and thereby eliminate the warning.
> >>
> >> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117178 [1]
> >> Cc: Zhenyu Wang <zhenyuw@...ux.intel.com>
> >> Cc: Zhi Wang <zhi.wang.linux@...il.com>
> >> Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>
> >> Cc: Rodrigo Vivi <rodrigo.vivi@...el.com>
> >> Cc: Tvrtko Ursulin <tursulin@...ulin.net>
> >> Cc: David Airlie <airlied@...il.com>
> >> Cc: Simona Vetter <simona@...ll.ch>
> >> Cc: intel-gvt-dev@...ts.freedesktop.org
> >> Cc: intel-gfx@...ts.freedesktop.org
> >> Cc: dri-devel@...ts.freedesktop.org
> >> Signed-off-by: Kees Cook <kees@...nel.org>
> >> ---
> >> drivers/gpu/drm/i915/gvt/opregion.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c
> >> index 509f9ccae3a9..f701638d3145 100644
> >> --- a/drivers/gpu/drm/i915/gvt/opregion.c
> >> +++ b/drivers/gpu/drm/i915/gvt/opregion.c
> >> @@ -43,7 +43,7 @@
> >> #define DEVICE_TYPE_EFP4 0x10
> >>
> >> struct opregion_header {
> >> - u8 signature[16];
> >> + u8 signature[16] __nonstring;
>
> Why would this annotation be needed? It's not treated as a string
> anywhere, and it's u8 not char.
>
> >> u32 size;
> >> u32 opregion_ver;
> >> u8 bios_ver[32];
> >> @@ -222,7 +222,7 @@ 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;
> >> + const char opregion_signature[16] __nonstring = OPREGION_SIGNATURE;
> >>
> >> gvt_dbg_core("init vgpu%d opregion\n", vgpu->id);
> >> vgpu_opregion(vgpu)->va = (void *)__get_free_pages(GFP_KERNEL |
> >> --
> >> 2.34.1
> >>
> > Hello together,
> >
> > it doesnt resolve the build issue with gcc15 gcc (GCC) 15.0.1 20250228
> >
> > CC [M] drivers/gpu/drm/i915/gvt/scheduler.o
> > /home/damian/kernel/linux/drivers/gpu/drm/i915/gvt/opregion.c: In function ‘intel_vgpu_init_opregion’:
> > /home/damian/kernel/linux/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"
> > | ^~~~~~~~~~~~~~~~~~
> > /home/damian/kernel/linux/drivers/gpu/drm/i915/gvt/opregion.c:225:57: note: in expansion of macro ‘OPREGION_SIGNATURE’
> > 225 | const char opregion_signature[16] __nonstring = OPREGION_SIGNATURE;
> > | ^~~~~~~~~~~~~~~~~~
> > CC [M] drivers/gpu/drm/i915/gvt/trace_points.o
> > cc1: all warnings being treated as errors
> > make[7]: *** [/home/damian/kernel/linux/scripts/Makefile.build:207: drivers/gpu/drm/i915/gvt/opregion.o] Error 1
> > make[7]: *** Waiting for unfinished jobs....
> > CC [M] drivers/gpu/drm/i915/gvt/vgpu.o
> > make[6]: *** [/home/damian/kernel/linux/scripts/Makefile.build:465: drivers/gpu/drm/i915] Error 2
> > make[5]: *** [/home/damian/kernel/linux/s
>
> What about this?
>
> IMO it's anyway good practice to use sizeof(dest) rather than
> sizeof(src) for memcpy.
>
>
> 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;
>
>
>
> --
> Jani Nikula, Intel
This patch does solve the build issue with gcc-15 on Fedora-42
(gcc-15.0.1-0.9.fc42.x86_64).
https://copr.fedorainfracloud.org/coprs/kwizart/kernel-longterm-6.12/build/8812754/
Thanks.
Powered by blists - more mailing lists