uapi: net/ipv4: Use __DECLARE_FLEX_ARRAY() helper
Subsystems (2)
PATCH 1/1 uapi: net/ipv4: Use __DECLARE_FLEX_ARRAY() helper
We now have a cleaner way to keep compatibility with user-space (a.k.a. not breaking it) when we need to keep in place a one-element array (for its use in user-space) together with a flexible-array member (for its use in kernel-space) without making it hard to read at the source level. This is through the use of the new __DECLARE_FLEX_ARRAY() helper macro.
The size and memory layout of the structure is preserved after the changes. See below.
Before changes:
$ pahole -C ip_msfilter net/ipv4/igmp.o
struct ip_msfilter {
union {
struct {
__be32 imsf_multiaddr_aux; /* 0 4 */
__be32 imsf_interface_aux; /* 4 4 */
__u32 imsf_fmode_aux; /* 8 4 */
__u32 imsf_numsrc_aux; /* 12 4 */
__be32 imsf_slist[1]; /* 16 4 */
}; /* 0 20 */
struct {
__be32 imsf_multiaddr; /* 0 4 */
__be32 imsf_interface; /* 4 4 */
__u32 imsf_fmode; /* 8 4 */
__u32 imsf_numsrc; /* 12 4 */
__be32 imsf_slist_flex[0]; /* 16 0 */
}; /* 0 16 */
}; /* 0 20 *//* size: 20, cachelines: 1, members: 1 */ /* last cacheline: 20 bytes */ };
After changes:
$ pahole -C ip_msfilter net/ipv4/igmp.o
struct ip_msfilter {
struct {
__be32 imsf_multiaddr; /* 0 4 */
__be32 imsf_interface; /* 4 4 */
__u32 imsf_fmode; /* 8 4 */
__u32 imsf_numsrc; /* 12 4 */
union {
__be32 imsf_slist[1]; /* 16 4 */
struct {
struct {
} __empty_imsf_slist_flex; /* 16 0 */
__be32 imsf_slist_flex[0]; /* 16 0 */
}; /* 16 0 */
}; /* 16 4 */
}; /* 0 20 *//* size: 20, cachelines: 1, members: 1 */ /* last cacheline: 20 bytes */ };
In the past, we had to duplicate the whole original structure within a union, and update the names of all the members. Now, we just need to declare the flexible-array member to be used in kernel-space through the __DECLARE_FLEX_ARRAY() helper together with the one-element array, within a union. This makes the source code more clean and easier to read.
--- include/uapi/linux/in.h | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
include/uapi/linux/in.h +7 -13
@@ -188,20 +188,14 @@ struct ip_mreq_source { };struct ip_msfilter { -union { -struct { -__be32 imsf_multiaddr_aux; -__be32 imsf_interface_aux; -__u32 imsf_fmode_aux; -__u32 imsf_numsrc_aux; +struct { +__be32 imsf_multiaddr; +__be32 imsf_interface; +__u32 imsf_fmode; +__u32 imsf_numsrc; +union {__be32 imsf_slist[1 ]; - }; -struct { -__be32 imsf_multiaddr; -__be32 imsf_interface; -__u32 imsf_fmode; -__u32 imsf_numsrc; -__be32 imsf_slist_flex[]; + __DECLARE_FLEX_ARRAY(__be32, imsf_slist_flex); }; }; };
Signature · 1 line
2.34.1
Quoted 75 lines · Gustavo A. R. Silva
We now have a cleaner way to keep compatibility with user-space (a.k.a. not breaking it) when we need to keep in place a one-element array (for its use in user-space) together with a flexible-array member (for its use in kernel-space) without making it hard to read at the source level. This is through the use of the new
__DECLARE_FLEX_ARRAY()helper macro.The size and memory layout of the structure is preserved after the changes. See below.
Before changes:
$ pahole -C ip_msfilter net/ipv4/igmp.o struct ip_msfilter { union { struct { __be32 imsf_multiaddr_aux; /* 0 4 */ __be32 imsf_interface_aux; /* 4 4 */ __u32 imsf_fmode_aux; /* 8 4 */ __u32 imsf_numsrc_aux; /* 12 4 */ __be32 imsf_slist[1]; /* 16 4 */ }; /* 0 20 */ struct { __be32 imsf_multiaddr; /* 0 4 */ __be32 imsf_interface; /* 4 4 */ __u32 imsf_fmode; /* 8 4 */ __u32 imsf_numsrc; /* 12 4 */ __be32 imsf_slist_flex[0]; /* 16 0 */ }; /* 0 16 */ }; /* 0 20 *//* size: 20, cachelines: 1, members: 1 */ /* last cacheline: 20 bytes */ };After changes:
$ pahole -C ip_msfilter net/ipv4/igmp.o struct ip_msfilter { struct { __be32 imsf_multiaddr; /* 0 4 */ __be32 imsf_interface; /* 4 4 */ __u32 imsf_fmode; /* 8 4 */ __u32 imsf_numsrc; /* 12 4 */ union { __be32 imsf_slist[1]; /* 16 4 */ struct { struct { } __empty_imsf_slist_flex; /* 16 0 */ __be32 imsf_slist_flex[0]; /* 16 0 */ }; /* 16 0 */ }; /* 16 4 */ }; /* 0 20 *//* size: 20, cachelines: 1, members: 1 */ /* last cacheline: 20 bytes */ };In the past, we had to duplicate the whole original structure within a union, and update the names of all the members. Now, we just need to declare the flexible-array member to be used in kernel-space through the
__DECLARE_FLEX_ARRAY()helper together with the one-element array, within a union. This makes the source code more clean and easier to read.--- include/uapi/linux/in.h | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)include/uapi/linux/in.h +0
-0
include/uapi/linux/in.h struct ip_mreq_source { · line 191
Quoted 10 lines
include/uapi/linux/in.h +1
-6@@ -188,20 +188,14 @@ struct ip_mreq_source { }; struct ip_msfilter { - union { - struct { - __be32 imsf_multiaddr_aux; - __be32 imsf_interface_aux; - __u32 imsf_fmode_aux; - __u32 imsf_numsrc_aux; + struct {
I don't think this internal anonymous struct is needed any more?
Quoted 7 lines
include/uapi/linux/in.h +5
-1+__be32 imsf_multiaddr; +__be32 imsf_interface; +__u32 imsf_fmode; +__u32 imsf_numsrc; +union {__be32 imsf_slist[1 ]; - };
include/uapi/linux/in.h struct ip_mreq_source { · line 201
Quoted 10 lines
include/uapi/linux/in.h +1
-6-struct { -__be32 imsf_multiaddr; -__be32 imsf_interface; -__u32 imsf_fmode; -__u32 imsf_numsrc; -__be32 imsf_slist_flex[]; +__DECLARE_FLEX_ARRAY (__be32, imsf_slist_flex); }; }; };
I.e. can't this now just be:
struct ip_msfilter {
__be32 imsf_multiaddr;
__be32 imsf_interface;
__u32 imsf_fmode;
__u32 imsf_numsrc;
union {
__be32 imsf_slist[1];
__DECLARE_FLEX_ARRAY(__be32, imsf_slist_flex);
};
};Quoted 3 lines
Signature · 1 line
2.34.1
Signature · 1 line
Kees Cook
Quoted 10 lines · Kees Cook
include/uapi/linux/in.h struct ip_mreq_source { · line 191
Quoted 8 lines
struct
ip_msfilter{include/uapi/linux/in.h +1
-6-union { -struct { -__be32 imsf_multiaddr_aux; -__be32 imsf_interface_aux; -__u32 imsf_fmode_aux; -__u32 imsf_numsrc_aux; +struct {I don't think this internal anonymous struct is needed any more?
yes, aaargh... copy/paste error D:
I'll respin right away.
Thanks!
--
Gustavo