* [PATCH][next] uapi: net/ipv4: Use __DECLARE_FLEX_ARRAY() helper
@ 2022-08-30 17:24 Gustavo A. R. Silva
2022-08-31 18:17 ` Kees Cook
0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-30 17:24 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Gustavo A. R. Silva, linux-hardening
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.
Link: https://github.com/KSPP/linux/issues/193
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
include/uapi/linux/in.h | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h
index 14168225cecd..fa4dc8f8f081 100644
--- a/include/uapi/linux/in.h
+++ b/include/uapi/linux/in.h
@@ -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);
};
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH][next] uapi: net/ipv4: Use __DECLARE_FLEX_ARRAY() helper
2022-08-30 17:24 [PATCH][next] uapi: net/ipv4: Use __DECLARE_FLEX_ARRAY() helper Gustavo A. R. Silva
@ 2022-08-31 18:17 ` Kees Cook
2022-08-31 19:01 ` Gustavo A. R. Silva
0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2022-08-31 18:17 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, linux-kernel, linux-hardening
On Tue, Aug 30, 2022 at 12:24:44PM -0500, Gustavo A. R. Silva wrote:
> 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.
>
> Link: https://github.com/KSPP/linux/issues/193
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> include/uapi/linux/in.h | 20 +++++++-------------
> 1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h
> index 14168225cecd..fa4dc8f8f081 100644
> --- a/include/uapi/linux/in.h
> +++ b/include/uapi/linux/in.h
> @@ -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?
> + __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);
> };
> };
> };
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);
};
};
> --
> 2.34.1
>
--
Kees Cook
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH][next] uapi: net/ipv4: Use __DECLARE_FLEX_ARRAY() helper
2022-08-31 18:17 ` Kees Cook
@ 2022-08-31 19:01 ` Gustavo A. R. Silva
0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-31 19:01 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, linux-kernel, linux-hardening
On Wed, Aug 31, 2022 at 11:17:31AM -0700, Kees Cook wrote:
> > 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?
yes, aaargh... copy/paste error D:
I'll respin right away.
Thanks!
--
Gustavo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-31 19:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-30 17:24 [PATCH][next] uapi: net/ipv4: Use __DECLARE_FLEX_ARRAY() helper Gustavo A. R. Silva
2022-08-31 18:17 ` Kees Cook
2022-08-31 19:01 ` Gustavo A. R. Silva
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.