src/basic/missing_syscall: change #ifndef to #if ! (defined && > 0)

[ddstreet]

The #ifndef check used to work for missing _NR* syscall defines, but
unfortunately libseccomp now redefines missing syscall number to negative
numbers, in their public header file, e.g.:
https://github.com/seccomp/libseccomp/blob/master/include/seccomp.h.in#L801

When systemd is built, since it includes <seccomp.h>, it pulls in the
incorrect negative value for any _NR* syscall define that's included in
the seccomp.h header (for those syscalls that the kernel headers don't
yet define, e.g. when built with older/stable-distro kernels). This leads
to bugs like:
https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1821625

This changes the check so that it can override the negative number that
libseccomp defines, instead of trying to use the negative syscall number.
To avoid gcc warnings (which are failures with meson --werror), this checks
without generating a redefinition gcc warning.

I have no idea why libseccomp decided to define missing syscalls
to negative numbers inside their public header file, causing
problems like this.

[floppym]

I have no idea why libseccomp decided to define missing syscalls to negative numbers inside their public header file, causing problems like this.

Has this issue been brought to the attention of the libseccomp maintainers?

[poettering]

hmm, yeah, i would argue libseccomp should not do what it is doing there.

[poettering]

@pcmoore any chance we can covince you to revert that change in libseccomp? defining the __NR_xyz macros to values that are not the right ones sounds like something that is not just going to break systemd, but a lot of other stuff too?

[pcmoore]

Adding @drakenclimber to this thread.

The negative syscall numbers (what we call pseudo-syscalls) are important to libseccomp when it comes to translating syscalls across ABIs; for example, creating both 32-bit x86 and 64-bit x86_64 filters on the same system. Basically you need to have a __NR_x define for every possible syscall across all of the ABIs that libseccomp supports. It has been this way for some time and a "revert" isn't an option at this point in time.

One quick hack might be to modify the pseudo-syscall definition to check if the syscall is already defined on the host system and throw an error at build time. It isn't perfect, but it would at least help catch disconnects between the host system and the libseccomp syscall table.

We do have an issue open to look into moving away from the pseudo-syscalls, but so far no firm solution.

• RFE: consider removing negative (non-canonical) __NR_xyz defines seccomp/libseccomp#27

[drakenclimber]

Interesting. @pcmoore let's make sure to talk about this next week at LSS

[keszybz]

That's pretty ugly, but I don't think we have much choice. Even if libseccomp changes this, versions that do this are out there in the wild and we need to keep systemd compilable. Let's merge and hope for a nicer approach in the future.

[pcmoore]

@drakenclimber agreed.

@keszybz that seems like the best approach.

[amluto]

ISTM the idea that __NR_whatever in libseccomp means something other than what __NR_whatever means in Linux was and remains a mistake. I would suggest that libseccomp create a new set of defines named LIBSECCOMP_whatever or similar, deprecate the old ones, and release a new libseccomp major release that breaks compatibility. Then systemd could drop support for old libseccomp versions.

For a less incompatible approach, new libseccomp could ship a compatibility header. After all, the API break here has no effect on the ABI, just the C headers.

[pcmoore]

@amluto I suggest moving your comments to the libseccomp issue mentioned above.

[poettering]

Hmm. I'd prefer if this commit would include comments in the code itself explaining what these lines are supposed to do. Its very much non-obvious to the reader who doesnt know libseccomp's logic on this.

[ddstreet]

Hmm. I'd prefer if this commit would include comments in the code itself explaining what these lines are supposed to do. Its very much non-obvious to the reader who doesnt know libseccomp's logic on this.

sure, I opened #13323 to add comment lines referencing this PR