[SRU][F][PATCH 1/1] UBUNTU: SAUCE: selftests/seccomp: fix 'storage size of ‘md’ isn’t known' build issue

[Thadeu Lima de Souza Cascardo]

On Thu, Oct 28, 2021 at 10:17:26AM -0700, Luke Nowakowski-Krijger wrote:
> There is a way to change the search order of the headers and effectively
> replace the global ones. We would have to do this individually however
> because some headers in-tree reference system headers that are not in a
> previous release, so we cant just include the whole include/uapi/linux dir
> like we would want. A solution could be to say "we want X.h header in
> file.c to be from in tree", and then a script could hook that include
> somewhere. The only problem is if that header references a system header
> that isnt in the release.. then we have the the same problem. And like we
> talked about, all the headers are not self contained, which would make this
> a lot easier.
> 
> Also I'm not entirely sure how often replacing the header solves the
> problem, maybe in 99% of cases its fine and the in-tree system header
> references and the previous system headers don't differ by that much? In
> the case of ptrace.h it worked and would solve this build problem better,
> but IMO we should just solve it the hacky way and try to include
> definitions directly since from what I have seen it doesen't seem to happen
> that much and the nicer solution doesn't even solve the problem all the
> time.
> 
> -Luke

So, I dug deep into this issue recently and failed to update the bug with my
findings.

Basically, glibc doesn't define lots of those structures, but adds the defines
for the ptrace requests. And this really makes it a pain to do the right thing
here on this test.

So, I looked around searching for any real userspace users of this feature.
Because they should be able to build whatever glibc and kernel version you have
installed, right?

It turns out there aren't that many. I think I found only one that had the
seccomp_metadata structure and it was not strace or libseccomp, but criu.

https://sources.debian.org/src/criu/3.16-2/compel/include/uapi/ptrace.h/?hl=66#L59

So, I would go with a similar solution, adding a comment there, and I would
also go upstream and fix it there too. Who knows with what glibc those tests
are being tested with?

Cascardo.

> 
> On Wed, Oct 27, 2021 at 12:52 AM Stefan Bader <stefan.bader at canonical.com>
> wrote:
> 
> > On 26.10.21 19:19, Luke Nowakowski-Krijger wrote:
> > > BugLink: https://bugs.launchpad.net/bugs/1896420
> > >
> > > There is a build issue on Bionic/5.4 kernels due to
> > > PTRACE_SECCOMP_GET_METADATA being defined in glibc header sys/ptrace.h,
> > > which then stops struct seccomp_metadata from being defined leading to:
> > > seccomp_bpf.c:3028:26: error: storage size of ‘md’ isn’t known
> > >
> > > The solution here is to unconditonally define the seccomp_metadata
> > > definition that we need, and remove the linux/ptrace.h header where a
> > > definition of seccomp_metadata exists in Focal.
> > >
> > > Signed-off-by: Luke Nowakowski-Krijger <
> > luke.nowakowskikrijger at canonical.com>
> > > ---
> >
> > This is a general problem we have with HWE kernels. The global C headers
> > are
> > what the base kernel supports (4.15 for bionic). The way this is changed
> > would
> > lock the definition to something seperate from the 5.4 kernel itself.  I
> > wonder
> > whether one could change the search order for includes in general to make
> > those
> > headers defined in the same tree take precedence over the global ones.
> >
> > -Stefan
> > >   tools/testing/selftests/seccomp/seccomp_bpf.c | 3 +--
> > >   1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c
> > b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > > index e9a00d26666f..ab0da56ef7f1 100644
> > > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> > > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > > @@ -26,7 +26,6 @@
> > >   #include <sys/ptrace.h>
> > >   #include <sys/user.h>
> > >   #include <linux/prctl.h>
> > > -#include <linux/ptrace.h>
> > >   #include <linux/seccomp.h>
> > >   #include <pthread.h>
> > >   #include <semaphore.h>
> > > @@ -158,12 +157,12 @@ struct seccomp_data {
> > >
> > >   #ifndef PTRACE_SECCOMP_GET_METADATA
> > >   #define PTRACE_SECCOMP_GET_METADATA 0x420d
> > > +#endif
> > >
> > >   struct seccomp_metadata {
> > >       __u64 filter_off;       /* Input: which filter */
> > >       __u64 flags;             /* Output: filter's flags */
> > >   };
> > > -#endif
> > >
> > >   #ifndef SECCOMP_FILTER_FLAG_NEW_LISTENER
> > >   #define SECCOMP_FILTER_FLAG_NEW_LISTENER    (1UL << 3)
> > >
> >
> >
> >

> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team