| Message ID | YzIei3tLO1IWtMjs@work (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c87e4ad1d3a09504de0e573ff3a2ee3f04a24642 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [next] net: ethernet: rmnet: Replace zero-length array with DECLARE_FLEX_ARRAY() helper | expand |
On Mon, Sep 26, 2022 at 04:50:03PM -0500, Gustavo A. R. Silva wrote: > 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 <gustavoars@kernel.org> Reviewed-by: Kees Cook <keescook@chromium.org>
On Mon, 26 Sep 2022 15:02:48 -0700 Kees Cook wrote: > On Mon, Sep 26, 2022 at 04:50:03PM -0500, Gustavo A. R. Silva wrote: > > 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 <gustavoars@kernel.org> > > Reviewed-by: Kees Cook <keescook@chromium.org> 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?
On Mon, Sep 26, 2022 at 05:26:04PM -0700, Jakub Kicinski wrote: > On Mon, 26 Sep 2022 15:02:48 -0700 Kees Cook wrote: > > On Mon, Sep 26, 2022 at 04:50:03PM -0500, Gustavo A. R. Silva wrote: > > > 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 <gustavoars@kernel.org> > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > 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?
On Mon, 26 Sep 2022 19:22:30 -0700 Kees Cook wrote: > > 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 :( > 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.
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h index e5a0b38f7dbe..2b033060fc20 100644 --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h @@ -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);
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 <gustavoars@kernel.org> --- drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)