-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
apparmor: load profiles in parallel #28766
Conversation
0585ebb
to
dc89b6e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we encode the dependency on find
/xargs
?
fi | ||
done | ||
find /etc/apparmor.d -maxdepth 1 -type f ! -name '*.new-*_*' |\ | ||
xargs -d"\n" -I{} --max-procs="$(grep -c ^processor /proc/cpuinfo)" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's all in findutils (xargs and find) which are installed if you have |
dc89b6e
to
02e299d
Compare
done | ||
find /etc/apparmor.d -maxdepth 1 -type f ! -name '*.new-*_*' |\ | ||
xargs -d"\n" -I{} --max-procs="$(nproc)" \ | ||
sh -c "printf '* Load profile %s: %s\n' '($APPARMOR)' '{}'; apparmor_parser -a $AACOMPLAIN '{}'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allows command injection. I'm not considering this an attack vector, as the files are root writeable, nevertheless, avoid this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Command injection? You mean shell spawning or in general? I don't see command injection here. I mean, even $APPARMOR without anything else in a shell script it can be a command injection if you don't control the source.
Upstream has optimized code to do this ( Currently we fudge it up with our custom one-by-one loading. Now we are reimplementing? Can we deprecate the runit "complain" mode and direct users to the Minimal example of what using the upstream provided code would look like (they make you define log functions):
|
I perfectly agree with @CameronNemo on long term using the upstream version. |
apparmror_parser already parses profiles in parallel, just pass everything to it and forget about the printf. |
If we patch in our .new- suffix we can just pass the directory and it will be fast and simple. |
02e299d
to
d5d52ad
Compare
The last version removes the superfluous logging |
daea413
to
9dd2431
Compare
Just a quick mod for parallelizing apparmor profiles on load.
Cc: @Gottox @ericonr @CameronNemo