Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Q: seccomp_syscall_resolve_name_arch(...) is confusing, docs arguably wrong #104

Closed
sourcejedi opened this issue Jan 31, 2018 · 2 comments
Closed

Comments

@sourcejedi
Copy link

sourcejedi commented Jan 31, 2018

The seccomp_syscall_resolve_name(),
seccomp_syscall_resolve_name_arch(), and
seccomp_syscall_resolve_name_rewrite() functions resolve the commonly
used syscall name to the syscall number used by the kernel and the
rest of the libseccomp API

I've been down a bit of a rabbithole. This sentence fragment is wrong, impossible, or maybe just misleadingly incomplete. Correcting it might have helped me learn libseccomp quicker.

The numbers returned by resolve_name() and resolve_name_arch() are different. resolve_name_arch()'s result can be passed to resolve_num_arch(), using the same arch value. But it should not be passed to "the rest of the libseccomp API". (Unless you know arch is native - but then why are you using resolve_name_arch()? Or unless the return value was negative, but I'm not sure why you would ever rely on that).

Specifically, seccomp_rule_add() takes either a native or a pseudo syscall NR. So you should pass it the result of resolve_name(). You shouldn't need to use resolve_name_arch(), as seccomp_rule_add() will convert the NR to whatever arch(s) the filter is set to.

There's a second implication, which systemd got wrong and which is what put me on this journey :). seccomp_resolve_num_arch() should be passed SCMP_ARCH_NATIVE, unless the syscall number specifically came from seccomp_resolve_name_arch(). systemd got it wrong, but I didn't guess that line was wrong, and so my attempts to investigate the resulting problem led me to a wrong understanding of the libseccomp functions.

This gets a bit circular. It's not clear what you'd ever be doing with these non-native, non-pseudo syscall numbers... and hence why we don't have seccomp_resolve_num() instead of seccomp_resolve_num_arch().

The best solution might be to add seccomp_resolve_num(), and then move the documentation of the _arch() variants either in a separate page, or at least in a paragraph of their own. Including some little disclaimer about their obscurity.

@sourcejedi sourcejedi changed the title doc: resolve_name_arch() doc is wrong resolve_name_arch() is confusing, docs arguably wrong Jan 31, 2018
sourcejedi added a commit to sourcejedi/systemd that referenced this issue Jan 31, 2018
Booting with `systemd.log_level=debug` and looking in `dmesg -u` showed
messages like this:

    systemd[433]: Failed to add rule for system call n/a() / 156, ignoring:
    Numerical argument out of domain

This commit fixes it to:

    systemd[449]: Failed to add rule for system call _sysctl() / 156,
    ignoring: Numerical argument out of domain

Some of the messages could be even more misleading, e.g. we were reporting
that utimensat() / 320 was skipped as non-existent on x86, when actually
the syscall number 320 is kexec_file_load() on x86 .

The problem was that syscall NRs are looked up (and correctly passed to
libseccomp) as native syscall NRs.  But we forgot that when we tried to
go back from the syscall NR to the name.

I think the natural way to write this would be
seccomp_syscall_resolve_num(nr), however there is no such function.
I couldn't work out a short comment that would make this clearer.  FWIW
I wrote it up as a ticket for libseccomp instead.
seccomp/libseccomp#104
@pcmoore pcmoore changed the title resolve_name_arch() is confusing, docs arguably wrong Q: seccomp_syscall_resolve_name_arch(...) is confusing, docs arguably wrong Feb 2, 2018
@pcmoore
Copy link
Member

pcmoore commented Feb 2, 2018

While I'm a big proponent of documentation, I'm going to be honest and say that I'm definitely not a technical writer. I do my best, but there are plenty of times where the resulting message isn't very clear.

If you have suggestions - and it sounds like you do - on how to make the manpage more clear, or correct, I would strongly encourage you to submit a patch/PR, that's the quickest way to get it changed (and you earn my undying gratitude for a doc fix!).

keszybz pushed a commit to systemd/systemd-stable that referenced this issue Feb 9, 2018
Booting with `systemd.log_level=debug` and looking in `dmesg -u` showed
messages like this:

    systemd[433]: Failed to add rule for system call n/a() / 156, ignoring:
    Numerical argument out of domain

This commit fixes it to:

    systemd[449]: Failed to add rule for system call _sysctl() / 156,
    ignoring: Numerical argument out of domain

Some of the messages could be even more misleading, e.g. we were reporting
that utimensat() / 320 was skipped as non-existent on x86, when actually
the syscall number 320 is kexec_file_load() on x86 .

The problem was that syscall NRs are looked up (and correctly passed to
libseccomp) as native syscall NRs.  But we forgot that when we tried to
go back from the syscall NR to the name.

I think the natural way to write this would be
seccomp_syscall_resolve_num(nr), however there is no such function.
I couldn't work out a short comment that would make this clearer.  FWIW
I wrote it up as a ticket for libseccomp instead.
seccomp/libseccomp#104

(cherry picked from commit 5c19ff7)
@pcmoore
Copy link
Member

pcmoore commented Mar 31, 2023

Hi @sourcejedi, It's been a few years without any updates to this issue so I'm going to close this out, but my comment still stands: if you want to contribute any documentation/comment improvements we would be very happy to have them in the library!

@pcmoore pcmoore closed this as completed Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants