linux-hardening - Re: [PATCH][next] firewire: core: avoid -Wflex-array-member-not-at-end warning

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-prev] [day] [month] [year] [list]

Message-ID: <20250313000335.GA320863@workstation.local>
Date: Thu, 13 Mar 2025 09:03:35 +0900
From: Takashi Sakamoto <o-takashi@...amocchi.jp>
To: "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc: linux1394-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH][next] firewire: core: avoid
 -Wflex-array-member-not-at-end warning

Hi,

On Tue, Mar 11, 2025 at 07:53:02PM +1030, Gustavo A. R. Silva wrote:
> Use the `DEFINE_RAW_FLEX()` helper for an on-stack definition of
> a flexible structure where the size of the flexible-array member
> is known at compile-time, and refactor the rest of the code,
> accordingly.
> 
> So, with these changes, fix the following warning:
> 
> drivers/firewire/core-cdev.c:1141:38: 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>
> ---
>  drivers/firewire/core-cdev.c | 42 ++++++++++++++++--------------------
>  1 file changed, 19 insertions(+), 23 deletions(-)

Thanks for the improvement, however I think it includes a slight concern
about the calculation for the length of array.

> diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
> index b360dca2c69e..706b9037faac 100644
> --- a/drivers/firewire/core-cdev.c
> +++ b/drivers/firewire/core-cdev.c
> @@ -1137,10 +1137,7 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
>  	unsigned long payload, buffer_end, transmit_header_bytes = 0;
>  	u32 control;
>  	int count;
> -	struct {
> -		struct fw_iso_packet packet;
> -		u8 header[256];
> -	} u;
> +	DEFINE_RAW_FLEX(struct fw_iso_packet, u, header, 8);

The definition of 'struct fw_iso_packet' is in 'include/linux/firewire.h':

```
$ cat include/linux/firewire.h
...
460 struct fw_iso_packet {
461         u16 payload_length;     /* Length of indirect payload           */
462         u32 interrupt:1;        /* Generate interrupt on this packet    */
463         u32 skip:1;             /* tx: Set to not send packet at all    */
464                                 /* rx: Sync bit, wait for matching sy   */
465         u32 tag:2;              /* tx: Tag in packet header             */
466         u32 sy:4;               /* tx: Sy in packet header              */
467         u32 header_length:8;    /* Size of immediate header             */
468         u32 header[];           /* tx: Top of 1394 isoch. data_block    */
469 };
```

The size of element of 'header' array is 4 byte (= 32 / 8). The original code keeps
256 bytes storage following to the structure. Therefore the 'COUNT' argument of DEFINE_RAW_FLEX()
macro should be 64 (= 256 / 4).

Although the header field is not rarely used by userspace applications
actually and the reduction of kernel stack usage is preferable itself,
it is preferable to keep the compatibility.


Thanks

Takashi Sakamoto