On Thu, May 05, 2022 at 07:16:18PM -0400, Paul Moore wrote:

On Thu, May 5, 2022 at 2:39 PM Kees Cook [off-list ref] wrote:

quoted

On Wed, May 04, 2022 at 11:14:42PM -0400, Paul Moore wrote:

quoted

On Wed, May 4, 2022 at 7:34 PM Gustavo A. R. Silva
[off-list ref] wrote:

quoted

Hi Paul,

On Wed, May 04, 2022 at 06:57:28PM -0400, Paul Moore wrote:

quoted

On Tue, May 3, 2022 at 9:57 PM Kees Cook [off-list ref] wrote:

[..]

quoted

quoted

+++ b/include/uapi/linux/xfrm.h

@@ -31,9 +31,9 @@ struct xfrm_id {
 struct xfrm_sec_ctx {
        __u8    ctx_doi;
        __u8    ctx_alg;
-       __u16   ctx_len;
+       __DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(__u16, ctx_len);
        __u32   ctx_sid;
-       char    ctx_str[0];
+       __DECLARE_FLEX_ARRAY_ELEMENTS(char, ctx_str);
 };

While I like the idea of this in principle, I'd like to hear about the
testing you've done on these patches.  A previous flex array
conversion in the audit uapi headers ended up causing a problem with

I'm curious about which commit caused those problems...?

Commit ed98ea2128b6 ("audit: replace zero-length array with
flexible-array member"), however, as I said earlier, the problem was
actually with SWIG, it just happened to be triggered by the kernel
commit.  There was a brief fedora-devel mail thread about the problem,
see the link below:

* https://www.spinics.net/lists/fedora-devel/msg297991.html

Wow, that's pretty weird -- it looks like SWIG was scraping the headers
to build its conversions? I assume SWIG has been fixed now?

I honestly don't know, the audit userspace was hacking around it with
some header file duplication/munging last I heard, but I try to avoid
having to touch Steve's audit userspace code.

quoted

quoted

To reiterate, I'm supportive of changes like this, but I would like to
hear how it was tested to ensure there are no unexpected problems with
userspace.  If there are userspace problems it doesn't mean we can't
make changes like this, it just means we need to ensure that the
userspace issues are resolved first.

Well, as this is the first and only report of any problems with [0] -> []
conversions (in UAPI or anywhere) that I remember seeing, and they've
been underway since at least v5.9, I hadn't been doing any new testing.

... and for whatever it is worth, I wasn't expecting it to be a
problem either.  Surprise :)

quoted

So, for this case, I guess I should ask what tests you think would be
meaningful here? Anything using #include should be fine:
https://codesearch.debian.net/search?q=linux%2Fxfrm.h&literal=1&perpkg=1
Which leaves just this, which may be doing something weird:

libabigail_2.0-1/tests/data/test-diff-filter/test-PR27569-v0.abi
        </data-member>
        <data-member access="public" layout-offset-in-bits="128">
          <var-decl name="seq_hi" type-id="3f1a6b60" visibility="default" filepath="include/uapi/linux/xfrm.h" line="97" column="1"/>
        </data-member>
        <data-member access="public" layout-offset-in-bits="160">

But I see that SWIG doesn't show up in a search for linux/audit.h:
https://codesearch.debian.net/search?q=linux%2Faudit.h&literal=1&perpkg=1

So this may not be a sufficient analysis...

I think from a practical perspective ensuring that the major IPsec/IKE
tools, e.g. the various *SWANs, that know about labeled IPSec still
build and can set/get the SA/SPD labels correctly would be sufficient.
I seriously doubt there would be any problems, but who knows.

There are certainly some cases in which the transformation of
zero-length arrays into flexible-array members can bring some issues
to the surface[1][2]. This is the first time that we know of one of
them in user-space. However, we haven't transformed the arrays in
UAPI yet (with the exception of a couple of cases[3][4]). But that
is something that we are planning to try soon[5].

--
Gustavo

[1] https://github.com/KSPP/linux/issues?q=invalid+use+of+flexible+array
[2] https://github.com/KSPP/linux/issues?q=invalid+application+of+%E2%80%98sizeof%E2%80%99+to+incomplete+type
[3] https://git.kernel.org/linus/db243b796439c0caba47865564d8acd18a301d18
[4] https://git.kernel.org/linus/d6cdad870358128c1e753e6258e295ab8a5a2429
[5] https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?h=for-next/kspp-fam0-uapi