net: ethernet: rmnet: Replace zero-length array with DECLARE_FLEX_ARRAY() helper

PATCH In review Gustavo A. R. Silva 3 participants 5 messages Sep 26, 2022 – Sep 27, 2022

Subsystems (3)

Networking Network drivers Qualcomm rmnet driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2022-09-26 21:50 UTC #0 lore raw

PATCH 1/1 net: ethernet: rmnet: Replace zero-length array with DECLARE_FLEX_ARRAY() helper

Zero-length arrays are deprecated and we are moving towards adopting C99 flexible-array members, instead. So, replace zero-length arrays declarations in anonymous union with the new DECLARE_FLEX_ARRAY() helper macro.

This helper allows for flexible-array members in unions.

Link https://github.com/KSPP/linux/issues/193

Link https://github.com/KSPP/linux/issues/221

Link https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h +1 -1

index e5a0b38f7dbe..2b033060fc20 100644
@@ -19,7 +19,7 @@ struct rmnet_map_control_command {
 			__be16 flow_control_seq_num;
 			__be32 qos_id;
 		} flow_control;
-		u8 data[0];
+		DECLARE_FLEX_ARRAY(u8, data);
 	};
 }  __aligned(1);

Signature · 1 line

2.34.1

Kees Cook

Reviewed-by on v1 1/1

2022-09-26 22:03 UTC #1 lore raw

Quoted 11 lines · Gustavo A. R. Silva

Zero-length arrays are deprecated and we are moving towards adopting C99 flexible-array members, instead. So, replace zero-length arrays declarations in anonymous union with the new DECLARE_FLEX_ARRAY() helper macro.
This helper allows for flexible-array members in unions.
Link https://github.com/KSPP/linux/issues/193
Link https://github.com/KSPP/linux/issues/221
Link https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
Signed-off-by Gustavo A. R. Silva <[email protected]>

Reviewed-by Kees Cook <[email protected]>

Signature · 1 line

Kees Cook

Jakub Kicinski

maintainer

2022-09-27 00:26 UTC #2 lore raw

Quoted 6 lines · On Mon, 26 Sep 2022 15:02:48 -0700 Kees Cook wrote:

Quoted 5 lines · Gustavo A. R. Silva
Zero-length arrays are deprecated and we are moving towards adopting C99 flexible-array members, instead. So, replace zero-length arrays declarations in anonymous union with the new DECLARE_FLEX_ARRAY() helper macro.

Quoted 8 lines

Quoted 6 lines
This helper allows for flexible-array members in unions.
Link https://github.com/KSPP/linux/issues/193
Link https://github.com/KSPP/linux/issues/221
Link https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Kees Cook <[email protected]>

Not directly related to this patch, but I just had to look at pahole output for sk_buff and the struct_group() stuff makes is really painful to read :/ Offsets for the members are relative to the "group" and they are all repeated.

Is there any chance you could fix that? Before we sprinkle more pixie dust around, perhaps?

Kees Cook

2022-09-27 02:22 UTC #3 lore raw

Quoted 16 lines · Jakub Kicinski

Quoted 6 lines · On Mon, 26 Sep 2022 15:02:48 -0700 Kees Cook wrote:
Quoted 5 lines · Gustavo A. R. Silva
Zero-length arrays are deprecated and we are moving towards adopting C99 flexible-array members, instead. So, replace zero-length arrays declarations in anonymous union with the new DECLARE_FLEX_ARRAY() helper macro.
Quoted 8 lines
Quoted 6 lines
This helper allows for flexible-array members in unions.
Link https://github.com/KSPP/linux/issues/193
Link https://github.com/KSPP/linux/issues/221
Link https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Kees Cook <[email protected]>

Quoted 7 lines

Not directly related to this patch, but I just had to look at pahole output for sk_buff and the struct_group() stuff makes is really painful to read :/ Offsets for the members are relative to the "group" and they are all repeated.
Is there any chance you could fix that? Before we sprinkle more pixie dust around, perhaps?

Unfortunately I don't see a way around it until we can make changes to the C language spec, and that's measured in decades. :(

Perhaps we could add some kind of heuristic to pahole to "hide" one of the internal struct_group() copies, and to hide the empty flexible-array wrapper structs? (pahole already can't tell the difference between a 0-length array and a flexible-array.) Would that be workable?

Signature · 1 line

Kees Cook

Jakub Kicinski

maintainer

2022-09-27 14:07 UTC #4 lore raw

Quoted 10 lines · On Mon, 26 Sep 2022 19:22:30 -0700 Kees Cook wrote:

Quoted 7 lines
Not directly related to this patch, but I just had to look at pahole output for sk_buff and the struct_group() stuff makes is really painful to read :/ Offsets for the members are relative to the "group" and they are all repeated.
Is there any chance you could fix that? Before we sprinkle more pixie dust around, perhaps?
Unfortunately I don't see a way around it until we can make changes to the C language spec, and that's measured in decades. :(

I think BPF folks have had some success adding C extensions, like tagging. Some form of attribute would really look so much better than this DECLARE_FLEX_ARRAY() macro indirection. Maybe it's just me :(

Quoted 4 lines

Perhaps we could add some kind of heuristic to pahole to "hide" one of the internal struct_group() copies, and to hide the empty flexible-array wrapper structs? (pahole already can't tell the difference between a 0-length array and a flexible-array.) Would that be workable?

That'd be my knee-jerk fix, too. Or at least render the offsets for the anonymous side of the union as absolute rather than relative.