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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAPM=9twgrjQdNCrnK2gXMckqDHRjBAwnCKx4HwAfty-Q6VZrig@mail.gmail.com>
Date: Tue, 25 Feb 2025 11:16:07 +1000
From: Dave Airlie <airlied@...il.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>, Karol Herbst <kherbst@...hat.com>, 
	Lyude Paul <lyude@...hat.com>, Faith Ekstrand <faith.ekstrand@...labora.com>, 
	Simona Vetter <simona@...ll.ch>, dri-devel@...ts.freedesktop.org, 
	nouveau@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH][next] drm/nouveau: Avoid multiple -Wflex-array-member-not-at-end
 warnings

On Sat, 15 Feb 2025 at 00:34, Danilo Krummrich <dakr@...nel.org> wrote:
>
> On Wed, Feb 12, 2025 at 07:31:26PM +1030, Gustavo A. R. Silva wrote:
> > -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> > getting ready to enable it, globally.
> >
> > So, in order to avoid ending up with flexible-array members in the
> > middle of other structs, we use the `struct_group_tagged()` helper
> > to separate the flexible arrays from the rest of the members in the
> > flexible structures. We then use the newly created tagged `struct
> > nvif_ioctl_v0_hdr` and `struct nvif_ioctl_mthd_v0_hdr` to replace the
> > type of the objects causing trouble in multiple structures.
> >
> > We also want to ensure that when new members need to be added to the
> > flexible structures, they are always included within the newly created
> > tagged structs. For this, we use `static_assert()`. This ensures that the
> > memory layout for both the flexible structure and the new tagged struct
> > is the same after any changes.
> >
> > So, with these changes, fix the following warnings:
> > drivers/gpu/drm/nouveau/nvif/object.c:60:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:233:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:214:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:152:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:138:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:104:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nouveau_svm.c:83:35: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nouveau_svm.c:82:30: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> >
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
>
> @Faith, Karol: Can I get an ACK from mesa for this one?

If we do reimport this to userspace we will have to figure it out, but for now,

Acked-by: Dave Airlie <airlied@...hat.com>

>
> > ---
> >  drivers/gpu/drm/nouveau/include/nvif/ioctl.h | 32 +++++++++++++-------
> >  drivers/gpu/drm/nouveau/nouveau_svm.c        |  4 +--
> >  drivers/gpu/drm/nouveau/nvif/object.c        | 12 ++++----
> >  3 files changed, 29 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/include/nvif/ioctl.h b/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> > index e825c8a1d9ca..00015412cb3e 100644
> > --- a/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> > +++ b/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> > @@ -3,25 +3,30 @@
> >  #define __NVIF_IOCTL_H__
> >
> >  struct nvif_ioctl_v0 {
> > -     __u8  version;
> > +     /* New members MUST be added within the struct_group() macro below. */
> > +     struct_group_tagged(nvif_ioctl_v0_hdr, __hdr,
> > +             __u8  version;
> >  #define NVIF_IOCTL_V0_SCLASS                                               0x01
> >  #define NVIF_IOCTL_V0_NEW                                                  0x02
> >  #define NVIF_IOCTL_V0_DEL                                                  0x03
> >  #define NVIF_IOCTL_V0_MTHD                                                 0x04
> >  #define NVIF_IOCTL_V0_MAP                                                  0x07
> >  #define NVIF_IOCTL_V0_UNMAP                                                0x08
> > -     __u8  type;
> > -     __u8  pad02[4];
> > +             __u8  type;
> > +             __u8  pad02[4];
> >  #define NVIF_IOCTL_V0_OWNER_NVIF                                           0x00
> >  #define NVIF_IOCTL_V0_OWNER_ANY                                            0xff
> > -     __u8  owner;
> > +             __u8  owner;
> >  #define NVIF_IOCTL_V0_ROUTE_NVIF                                           0x00
> >  #define NVIF_IOCTL_V0_ROUTE_HIDDEN                                         0xff
> > -     __u8  route;
> > -     __u64 token;
> > -     __u64 object;
> > +             __u8  route;
> > +             __u64 token;
> > +             __u64 object;
> > +     );
> >       __u8  data[];           /* ioctl data (below) */
> >  };
> > +static_assert(offsetof(struct nvif_ioctl_v0, data) == sizeof(struct nvif_ioctl_v0_hdr),
> > +           "struct member likely outside of struct_group()");
> >
> >  struct nvif_ioctl_sclass_v0 {
> >       /* nvif_ioctl ... */
> > @@ -51,12 +56,17 @@ struct nvif_ioctl_del {
> >  };
> >
> >  struct nvif_ioctl_mthd_v0 {
> > -     /* nvif_ioctl ... */
> > -     __u8  version;
> > -     __u8  method;
> > -     __u8  pad02[6];
> > +     /* New members MUST be added within the struct_group() macro below. */
> > +     struct_group_tagged(nvif_ioctl_mthd_v0_hdr, __hdr,
> > +             /* nvif_ioctl ... */
> > +             __u8  version;
> > +             __u8  method;
> > +             __u8  pad02[6];
> > +     );
> >       __u8  data[];           /* method data (class.h) */
> >  };
> > +static_assert(offsetof(struct nvif_ioctl_mthd_v0, data) == sizeof(struct nvif_ioctl_mthd_v0_hdr),
> > +           "struct member likely outside of struct_group()");
> >
> >  struct nvif_ioctl_map_v0 {
> >       /* nvif_ioctl ... */
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> > index b4da82ddbb6b..fc64c3d3169e 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> > @@ -79,8 +79,8 @@ struct nouveau_svm {
> >  #define SVM_ERR(s,f,a...) NV_WARN((s)->drm, "svm: "f"\n", ##a)
> >
> >  struct nouveau_pfnmap_args {
> > -     struct nvif_ioctl_v0 i;
> > -     struct nvif_ioctl_mthd_v0 m;
> > +     struct nvif_ioctl_v0_hdr i;
> > +     struct nvif_ioctl_mthd_v0_hdr m;
> >       struct nvif_vmm_pfnmap_v0 p;
> >  };
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvif/object.c b/drivers/gpu/drm/nouveau/nvif/object.c
> > index 0b87278ac0f8..70af63d70976 100644
> > --- a/drivers/gpu/drm/nouveau/nvif/object.c
> > +++ b/drivers/gpu/drm/nouveau/nvif/object.c
> > @@ -57,7 +57,7 @@ int
> >  nvif_object_sclass_get(struct nvif_object *object, struct nvif_sclass **psclass)
> >  {
> >       struct {
> > -             struct nvif_ioctl_v0 ioctl;
> > +             struct nvif_ioctl_v0_hdr ioctl;
> >               struct nvif_ioctl_sclass_v0 sclass;
> >       } *args = NULL;
> >       int ret, cnt = 0, i;
> > @@ -101,7 +101,7 @@ int
> >  nvif_object_mthd(struct nvif_object *object, u32 mthd, void *data, u32 size)
> >  {
> >       struct {
> > -             struct nvif_ioctl_v0 ioctl;
> > +             struct nvif_ioctl_v0_hdr ioctl;
> >               struct nvif_ioctl_mthd_v0 mthd;
> >       } *args;
> >       u32 args_size;
> > @@ -135,7 +135,7 @@ void
> >  nvif_object_unmap_handle(struct nvif_object *object)
> >  {
> >       struct {
> > -             struct nvif_ioctl_v0 ioctl;
> > +             struct nvif_ioctl_v0_hdr ioctl;
> >               struct nvif_ioctl_unmap unmap;
> >       } args = {
> >               .ioctl.type = NVIF_IOCTL_V0_UNMAP,
> > @@ -149,7 +149,7 @@ nvif_object_map_handle(struct nvif_object *object, void *argv, u32 argc,
> >                      u64 *handle, u64 *length)
> >  {
> >       struct {
> > -             struct nvif_ioctl_v0 ioctl;
> > +             struct nvif_ioctl_v0_hdr ioctl;
> >               struct nvif_ioctl_map_v0 map;
> >       } *args;
> >       u32 argn = sizeof(*args) + argc;
> > @@ -211,7 +211,7 @@ void
> >  nvif_object_dtor(struct nvif_object *object)
> >  {
> >       struct {
> > -             struct nvif_ioctl_v0 ioctl;
> > +             struct nvif_ioctl_v0_hdr ioctl;
> >               struct nvif_ioctl_del del;
> >       } args = {
> >               .ioctl.type = NVIF_IOCTL_V0_DEL,
> > @@ -230,7 +230,7 @@ nvif_object_ctor(struct nvif_object *parent, const char *name, u32 handle,
> >                s32 oclass, void *data, u32 size, struct nvif_object *object)
> >  {
> >       struct {
> > -             struct nvif_ioctl_v0 ioctl;
> > +             struct nvif_ioctl_v0_hdr ioctl;
> >               struct nvif_ioctl_new_v0 new;
> >       } *args;
> >       int ret = 0;
> > --
> > 2.43.0
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ