-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally.
There is currently a local structure `f` that is using a flexible
`struct file_handle` as header for an on-stack place-holder for the
flexible-array member `unsigned char f_handle[];`.
struct {
struct file_handle handle;
u8 pad[MAX_HANDLE_SZ];
} f;
However, we are deprecating flexible arrays in the middle of another
struct. So, in order to avoid this, we use the `struct_group_tagged()`
helper to separate the flexible array from the rest of the members in
the flexible structure:
struct file_handle {
struct_group_tagged(file_handle_hdr, hdr,
... the rest of the members
);
unsigned char f_handle[];
};
With the change described above, we can now declare an object of the
type of the tagged struct, without embedding the flexible array in the
middle of another struct:
struct {
struct file_handle_hdr handle;
u8 pad[MAX_HANDLE_SZ];
} f;
We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure, through which the flexible-array member can be
accessed, as in this case.
So, with these changes, fix the following warning:
fs/notify/fdinfo.c: In function ‘show_mark_fhandle’:
fs/notify/fdinfo.c:45:36: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
45 | struct file_handle handle;
| ^~~~~~
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
fs/notify/fdinfo.c | 8 +++++---
include/linux/fs.h | 6 ++++--
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
index 5c430736ec12..740f5e68b397 100644
--- a/fs/notify/fdinfo.c
+++ b/fs/notify/fdinfo.c
@@ -42,15 +42,17 @@ static void show_fdinfo(struct seq_file *m, struct file *f,
static void show_mark_fhandle(struct seq_file *m, struct inode *inode)
{
struct {
- struct file_handle handle;
+ struct file_handle_hdr handle;
u8 pad[MAX_HANDLE_SZ];
} f;
+ struct file_handle *handle = container_of(&f.handle,
+ struct file_handle, hdr);
int size, ret, i;
f.handle.handle_bytes = sizeof(f.pad);
size = f.handle.handle_bytes >> 2;
- ret = exportfs_encode_fid(inode, (struct fid *)f.handle.f_handle, &size);
+ ret = exportfs_encode_fid(inode, (struct fid *)handle->f_handle, &size);
if ((ret == FILEID_INVALID) || (ret < 0)) {
WARN_ONCE(1, "Can't encode file handler for inotify: %d\n", ret);
return;
@@ -63,7 +65,7 @@ static void show_mark_fhandle(struct seq_file *m, struct inode *inode)
f.handle.handle_bytes, f.handle.handle_type);
for (i = 0; i < f.handle.handle_bytes; i++)
- seq_printf(m, "%02x", (int)f.handle.f_handle[i]);
+ seq_printf(m, "%02x", (int)handle->f_handle[i]);
}
#else
static void show_mark_fhandle(struct seq_file *m, struct inode *inode)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 00fc429b0af0..7c131bcd948f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1030,8 +1030,10 @@ struct file {
__attribute__((aligned(4))); /* lest something weird decides that 2 is OK */
struct file_handle {
- __u32 handle_bytes;
- int handle_type;
+ struct_group_tagged(file_handle_hdr, hdr,
+ __u32 handle_bytes;
+ int handle_type;
+ );
/* file identifier */
unsigned char f_handle[];
};
--
2.34.1
On Tue, Mar 05, 2024 at 04:18:46PM -0600, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> ready to enable it globally.
>
> There is currently a local structure `f` that is using a flexible
> `struct file_handle` as header for an on-stack place-holder for the
> flexible-array member `unsigned char f_handle[];`.
>
> struct {
> struct file_handle handle;
> u8 pad[MAX_HANDLE_SZ];
> } f;
This code pattern is "put a flex array struct on the stack", but we have
a macro for this now:
DEFINE_FLEX(struct file_handle, handle, f_handle, MAX_HANDLE_SZ);
And you can even include the initializer:
_DEFINE_FLEX(struct file_handle, handle, f_handle, MAX_HANDLE_SZ,
= { .handle_bytes = MAX_HANDLE_SZ });
I think this would be a simpler conversion.
Also, this could use a __counted_by tag...
I need to improve the DEFINE_FLEX macro a bit, though, to take advantage
of __counted_by.
--
Kees Cook
On Wed, Mar 6, 2024 at 1:52 AM Kees Cook <[email protected]> wrote:
>
> On Tue, Mar 05, 2024 at 04:18:46PM -0600, Gustavo A. R. Silva wrote:
> > -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> > ready to enable it globally.
> >
> > There is currently a local structure `f` that is using a flexible
> > `struct file_handle` as header for an on-stack place-holder for the
> > flexible-array member `unsigned char f_handle[];`.
> >
> > struct {
> > struct file_handle handle;
> > u8 pad[MAX_HANDLE_SZ];
> > } f;
>
> This code pattern is "put a flex array struct on the stack", but we have
> a macro for this now:
>
> DEFINE_FLEX(struct file_handle, handle, f_handle, MAX_HANDLE_SZ);
>
> And you can even include the initializer:
>
> _DEFINE_FLEX(struct file_handle, handle, f_handle, MAX_HANDLE_SZ,
> = { .handle_bytes = MAX_HANDLE_SZ });
>
Indeed that looks much nicer.
Thanks,
Amir.
On 3/5/24 17:52, Kees Cook wrote:
> On Tue, Mar 05, 2024 at 04:18:46PM -0600, Gustavo A. R. Silva wrote:
>> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
>> ready to enable it globally.
>>
>> There is currently a local structure `f` that is using a flexible
>> `struct file_handle` as header for an on-stack place-holder for the
>> flexible-array member `unsigned char f_handle[];`.
>>
>> struct {
>> struct file_handle handle;
>> u8 pad[MAX_HANDLE_SZ];
>> } f;
>
> This code pattern is "put a flex array struct on the stack", but we have
> a macro for this now:
>
> DEFINE_FLEX(struct file_handle, handle, f_handle, MAX_HANDLE_SZ);
>
> And you can even include the initializer:
>
> _DEFINE_FLEX(struct file_handle, handle, f_handle, MAX_HANDLE_SZ,
> = { .handle_bytes = MAX_HANDLE_SZ });
>
> I think this would be a simpler conversion.
>
> Also, this could use a __counted_by tag...
>
> I need to improve the DEFINE_FLEX macro a bit, though, to take advantage
> of __counted_by.
>
Yep, I like it.
I'll go and hunt down all those on-stack -Wflex-array-member-not-at-end
issues with this helper. :)
Thanks
--
Gustavo
On 3/6/24 01:36, Amir Goldstein wrote:
> On Wed, Mar 6, 2024 at 1:52 AM Kees Cook <[email protected]> wrote:
>>
>> On Tue, Mar 05, 2024 at 04:18:46PM -0600, Gustavo A. R. Silva wrote:
>>> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
>>> ready to enable it globally.
>>>
>>> There is currently a local structure `f` that is using a flexible
>>> `struct file_handle` as header for an on-stack place-holder for the
>>> flexible-array member `unsigned char f_handle[];`.
>>>
>>> struct {
>>> struct file_handle handle;
>>> u8 pad[MAX_HANDLE_SZ];
>>> } f;
>>
>> This code pattern is "put a flex array struct on the stack", but we have
>> a macro for this now:
>>
>> DEFINE_FLEX(struct file_handle, handle, f_handle, MAX_HANDLE_SZ);
>>
>> And you can even include the initializer:
>>
>> _DEFINE_FLEX(struct file_handle, handle, f_handle, MAX_HANDLE_SZ,
>> = { .handle_bytes = MAX_HANDLE_SZ });
>>
>
> Indeed that looks much nicer.
Yeah, I'll probably wait for this to land before I send a v2:
https://lore.kernel.org/linux-hardening/[email protected]/
Thanks
--
Gustavo