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

add @filesystem and @filesystem-ops syscall filter groups #4537

Merged
merged 1 commit into from
Nov 22, 2016

Conversation

poettering
Copy link
Member

After discussions in #4483, this is a new, separate PR now for the @filesystem and @filesystem-ops syscall filter groups, as they need more discussion and should probably not be merged before v232 is out.

This PR is on top of #4483, hence that PR should really be merged first, and until then please ignore all commits in this PR except the last one.

@keszybz, @topimiettinen, @evverx could you summarize your suggestions regarding these groups here again, please?

@poettering poettering added the pid1 label Nov 2, 2016
@poettering poettering added this to the v233 milestone Nov 2, 2016
@keszybz
Copy link
Member

keszybz commented Nov 9, 2016

"@resources" looks good.

I don't like the split into "basic" and "advanced" — it's just too arbitrary. (The names "@filesystem" and "@filesystem-ops" also don't really explain the difference). I'd prefer to add new groups "@filesystem-read" and "@filesystem-write", even if if requires looking the arguments of some system calls, not just at the syscall number. @filesystemd-read should include the calls that don't modify the file system, and @filesystem-write the ones that do or may modify. The tricky syscalls would be open, mmap and such. Ultimately this would be more useful to users than a raw syscall list.

This PR needs rebasing after my systemd-analyze syscall-filter additions.

@poettering
Copy link
Member Author

OK, I force pushed a new rebased version of this. I merged the two groups into one now: "@filesystem" that is hopefully more acceptable?

I am not sure we could properly distuingish read/write access with seccomp for all cases really. And if we can I am not sure this belongs under SystemCallFilters= (which really just matches system calls so far, but doesn't try to grok semantics of a call), but probably should get its own call ReadOnlyFileAccess=yes|no or so...

("@resources" already got merged btw, as part of the original PR.)

@filesystem groups various file system operations, such as opening files and
directories for read/write and stat()ing them, plus renaming, deleting,
symlinking, hardlinking.
@keszybz
Copy link
Member

keszybz commented Nov 22, 2016

Yeah, let's merge this. If we ever decide to make this more fancy, we can add @file-system-read and @file-system-write or whatever other options.

The failure is the one we have seen before, probably unrelated:

- ['ifup@ens2.service loaded failed failed ifup for ens2']
+ []

@keszybz
Copy link
Member

keszybz commented Nov 22, 2016

And also

subprocess.CalledProcessError: Command '['/lib/systemd/systemd-networkd-wait-online', '--interface', 'test_eth42', '--timeout=15']' returned non-zero exit status 1

@keszybz keszybz merged commit 1a1b13c into systemd:master Nov 22, 2016
@lucaswerkmeister
Copy link
Member

Is it intentional that the group includes mmap but not munmap? Seems like a weird asymmetry.

poettering added a commit to poettering/systemd that referenced this pull request Feb 9, 2017
We added mmap() and mmap2(), but forgot munmap(). Fix that.

Pointed out by @lucaswerkmeister:

systemd#4537 (comment)
poettering added a commit to poettering/systemd that referenced this pull request Feb 9, 2017
We added mmap() and mmap2(), but forgot munmap(). Fix that.

Pointed out by @lucaswerkmeister:

systemd#4537 (comment)
keszybz pushed a commit that referenced this pull request Feb 10, 2017
We added mmap() and mmap2(), but forgot munmap(). Fix that.

Pointed out by @lucaswerkmeister:

#4537 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants