linux-hardening - [PATCH] [next] i915/gvt: Replace one-element array with flexible-array member

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y6Eu2604cqtryP4g@mail.google.com>
Date:   Tue, 20 Dec 2022 16:41:15 +1300
From:   Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@...il.com>
To:     Zhenyu Wang <zhenyuw@...ux.intel.com>,
        Zhi Wang <zhi.a.wang@...el.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        intel-gvt-dev@...ts.freedesktop.org,
        intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org
Cc:     linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org,
        paulo.miguel.almeida.rodenas@...il.com
Subject: [PATCH] [next] i915/gvt: Replace one-element array with
 flexible-array member

One-element arrays are deprecated, and we are replacing them with
flexible array members instead. So, replace one-element array with
flexible-array member in struct gvt_firmware_header and refactor the
rest of the code accordingly.

Additionally, previous implementation was allocating 8 bytes more than
required to represent firmware_header + cfg_space data + mmio data.

This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [1].

Link: https://github.com/KSPP/linux/issues/79
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@...il.com>
---
To make reviewing this patch easier, I'm pasting before/after struct
sizes.

pahole -C gvt_firmware_header before/drivers/gpu/drm/i915/gvt/firmware.o 
struct gvt_firmware_header {
	u64                        magic;                /*     0     8 */
	u32                        crc32;                /*     8     4 */
	u32                        version;              /*    12     4 */
	u64                        cfg_space_size;       /*    16     8 */
	u64                        cfg_space_offset;     /*    24     8 */
	u64                        mmio_size;            /*    32     8 */
	u64                        mmio_offset;          /*    40     8 */
	unsigned char              data[1];              /*    48     1 */

	/* size: 56, cachelines: 1, members: 8 */
	/* padding: 7 */
	/* last cacheline: 56 bytes */
};

pahole -C gvt_firmware_header after/drivers/gpu/drm/i915/gvt/firmware.o 
struct gvt_firmware_header {
	u64                        magic;                /*     0     8 */
	u32                        crc32;                /*     8     4 */
	u32                        version;              /*    12     4 */
	u64                        cfg_space_size;       /*    16     8 */
	u64                        cfg_space_offset;     /*    24     8 */
	u64                        mmio_size;            /*    32     8 */
	u64                        mmio_offset;          /*    40     8 */
	unsigned char              data[];               /*    48     0 */

	/* size: 48, cachelines: 1, members: 8 */
	/* last cacheline: 48 bytes */
};

As you can see the additional byte of the fake-flexible array (data[1])
forced the compiler to pad the struct but those bytes aren't actually used
as first & last bytes (of both cfg_space and mmio) are controlled by the
<>_size and <>_offset members present in the gvt_firmware_header struct.
---
 drivers/gpu/drm/i915/gvt/firmware.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c
index a683c22d5b64..dce93738e98a 100644
--- a/drivers/gpu/drm/i915/gvt/firmware.c
+++ b/drivers/gpu/drm/i915/gvt/firmware.c
@@ -45,7 +45,7 @@ struct gvt_firmware_header {
 	u64 cfg_space_offset;	/* offset in the file */
 	u64 mmio_size;
 	u64 mmio_offset;	/* offset in the file */
-	unsigned char data[1];
+	unsigned char data[];
 };
 
 #define dev_to_drm_minor(d) dev_get_drvdata((d))
@@ -77,7 +77,7 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt)
 	unsigned long size, crc32_start;
 	int ret;
 
-	size = sizeof(*h) + info->mmio_size + info->cfg_space_size;
+	size = offsetof(struct gvt_firmware_header, data) + info->mmio_size + info->cfg_space_size;
 	firmware = vzalloc(size);
 	if (!firmware)
 		return -ENOMEM;
-- 
2.38.1