* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).