On Wed, 16 Nov 2022 19:20:51 -0600 Gustavo A. R. Silva wrote:

On 11/16/22 19:05, Jakub Kicinski wrote:

quoted

quoted

This seems to be a sensible change. In general, it's not a good idea
to have variable length objects (flex-array members) in structures used
as headers, and that we know will ultimately be followed by more objects
when embedded inside other structures.  

Meaning we should go back to zero-length arrays instead?  

No.

I was asking based on your own commit 1e6e9d0f4859 ("uapi: revert
flexible-array conversions"). This is uAPI as well.

Since we can't prevent user space from wrapping structures seems
like adding a flex member to an existing struct should never be
permitted in uAPI headers? We can just wrap things locally, I guess:

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 9ebdf3262015..2af2f8de4043 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c

@@ -2479,7 +2479,10 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 {
        struct sk_buff *skb;
        struct nlmsghdr *rep;
-       struct nlmsgerr *errmsg;
+       struct hashtag_silly {
+               struct nlmsgerr err;
+               u8 data[];
+       } *errmsg;
        size_t payload = sizeof(*errmsg);
        struct netlink_sock *nlk = nlk_sk(NETLINK_CB(in_skb).sk);
        unsigned int flags = 0;

@@ -2507,15 +2510,14 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
        if (!rep)
                goto err_bad_put;
        errmsg = nlmsg_data(rep);
-       errmsg->error = err;
-       errmsg->msg = *nlh;
+       errmsg->err.error = err;
+       errmsg->err.msg = *nlh;
 
        if (!(flags & NLM_F_CAPPED)) {
                if (!nlmsg_append(skb, nlmsg_len(nlh)))
                        goto err_bad_put;
 
-               memcpy(errmsg->msg.nlmsg_data, nlh->nlmsg_data,
-                      nlmsg_len(nlh));
+               memcpy(errmsg->data, nlmsg_data(nlh), nlmsg_len(nlh));
        }
 
        if (tlvlen)

In this particular case, tho, we're probably better off giving up 
on the flex array and doing nlmsg_data() on both src and dst.

quoted

Is there something in the standard that makes flexible array
at the end of an embedded struct a problem?  

I haven't seen any problems ss long as the flex-array appears last:

struct foo {
	... members
	struct boo {
		... members
		char flex[];
	};
};

struct complex {
	... members
	struct foo embedded;
};

However, the GCC docs[1] mention this:

"A structure containing a flexible array member [..] may not be a
member of a structure [..] (However, these uses are permitted by GCC
as extensions.)"

And in this case it seems that's the reason why GCC doesn't complain?

Seems so, clang's warning is called -Wgnu-variable-sized-type-not-at-end