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 capabilities, priority, and oom-score-adjustment support on Linux #404

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/meson_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ jobs:
if: ${{ matrix.arch == 'amd64' }}
run: |
apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install g++ meson m4 -y
DEBIAN_FRONTEND=noninteractive apt-get install g++ meson m4 libcap-dev -y
- name: Getting depends (i386)
if: ${{ matrix.arch == 'i386' }}
run: |
apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install meson m4:i386 g++:i386 -y
DEBIAN_FRONTEND=noninteractive apt-get install meson m4:i386 g++:i386 libcap-dev:i386 -y
- name: Setup
run: meson setup -Dunit-tests=true -Digr-tests=true dirbuild
- name: Build
Expand Down Expand Up @@ -130,7 +130,7 @@ jobs:
- name: Getting depends
run: |
apk update
apk add meson g++ m4
apk add meson g++ m4 libcap-dev
- name: Setup
run: meson setup -Dunit-tests=true -Digr-tests=true dirbuild
- name: Build
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/regular_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ jobs:
if: ${{ matrix.arch == 'amd64' }}
run: |
apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install g++ make m4 file -y
DEBIAN_FRONTEND=noninteractive apt-get install g++ make m4 libcap-dev file -y
- name: Getting depends (i386)
if: ${{ matrix.arch == 'i386' }}
run: |
apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install gcc:i386 make:i386 m4:i386 g++:i386 file -y
DEBIAN_FRONTEND=noninteractive apt-get install gcc:i386 make:i386 m4:i386 g++:i386 libcap-dev:i386 file -y
- name: Print g++ architecture
run: g++ -dumpmachine
- name: Build
Expand Down Expand Up @@ -123,7 +123,7 @@ jobs:
- name: Getting depends
run: |
apk update
apk add make g++ m4 file
apk add make g++ m4 file libcap-dev
- name: Print g++ architecture
run: g++ -dumpmachine
- name: Build
Expand Down
3 changes: 3 additions & 0 deletions build/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ includes/mconfig.h: ../mconfig tools/mconfig-gen.cc version.conf
DEFAULT_START_TIMEOUT=$(DEFAULT_START_TIMEOUT) \
DEFAULT_STOP_TIMEOUT=$(DEFAULT_STOP_TIMEOUT) \
$(if $(SUPPORT_CGROUPS),SUPPORT_CGROUPS=$(SUPPORT_CGROUPS),) \
$(if $(SUPPORT_CAPABILITIES),SUPPORT_CAPABILITIES=$(SUPPORT_CAPABILITIES),) \
$(if $(SUPPORT_IOPRIO),SUPPORT_IOPRIO=$(SUPPORT_IOPRIO),) \
$(if $(SUPPORT_OOM_ADJ),SUPPORT_OOM_ADJ=$(SUPPORT_OOM_ADJ),) \
$(if $(USE_UTMPX),USE_UTMPX=$(USE_UTMPX),) \
$(if $(USE_INITGROUPS),USE_INITGROUPS=$(USE_INITGROUPS),) > includes/mconfig.h

Expand Down
3 changes: 3 additions & 0 deletions build/mconfig.mesontemplate
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
#mesondefine USE_UTMPX
#mesondefine USE_INITGROUPS
#mesondefine SUPPORT_CGROUPS
#mesondefine SUPPORT_CAPABILITIES
#mesondefine SUPPORT_IOPRIO
#mesondefine SUPPORT_OOM_ADJ
#mesondefine DEFAULT_AUTO_RESTART
#mesondefine DEFAULT_START_TIMEOUT
#mesondefine DEFAULT_STOP_TIMEOUT
Expand Down
9 changes: 9 additions & 0 deletions build/tools/mconfig-gen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ int main(int argc, char **argv)
if (vars.find("SUPPORT_CGROUPS") != vars.end()) {
cout << "#define SUPPORT_CGROUPS " << vars["SUPPORT_CGROUPS"] << "\n";
}
if (vars.find("SUPPORT_CAPABILITIES") != vars.end()) {
cout << "#define SUPPORT_CAPABILITIES " << vars["SUPPORT_CAPABILITIES"] << "\n";
}
if (vars.find("SUPPORT_IOPRIO") != vars.end()) {
cout << "#define SUPPORT_IOPRIO " << vars["SUPPORT_IOPRIO"] << "\n";
}
if (vars.find("SUPPORT_OOM_ADJ") != vars.end()) {
cout << "#define SUPPORT_OOM_ADJ " << vars["SUPPORT_OOM_ADJ"] << "\n";
}
if (vars.find("DEFAULT_AUTO_RESTART") != vars.end()) {
cout << "#define DEFAULT_AUTO_RESTART " << vars["DEFAULT_AUTO_RESTART"] << "\n";
}
Expand Down
3 changes: 3 additions & 0 deletions configs/mconfig.Linux
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ TEST_LDFLAGS=$(TEST_LDFLAGS_BASE) $(TEST_CXXFLAGS)
# Features.

SUPPORT_CGROUPS=1
SUPPORT_CAPABILITIES=1
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this requires a dependency (in libcap), I'd prefer the default here was SUPPORT_CAPABILITIES=0. It's fine to have it on-by-default in the configure script since we can check the dependency is available there (but that should be implemented).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it may add a dependency, but it's a dependency that's present on virtually any linux system (it's required by things like... udev), imo it makes much more sense to keep it on

Copy link
Owner

@davmac314 davmac314 Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The library will be present but not necessarily the headers, and people using a straight Makefile-based build are often just trying out Dinit for personal use. I'd like to make sure that this scenario still works as easily as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but those people can easily disable it; i don't think it's right to default to a configuration where capabilities are not supported, the default build should imo be one that is as distros should build it

it's trivial to disable for somebody just trying it (it's also trivial to just install libcap-devel or whatever, in 99% of scenarios it's easily doable, and you already need to install a development toolchain)

SUPPORT_IOPRIO=1
SUPPORT_OOM_ADJ=1


# Service defaults.
Expand Down
3 changes: 3 additions & 0 deletions configs/mconfig.Linux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ FEATURE_SETTINGS=$(
echo "# Feature settings"
echo ""
echo "SUPPORT_CGROUPS=1"
echo "SUPPORT_CAPABILITIES=1"
echo "SUPPORT_IOPRIO=1"
echo "SUPPORT_OOM_ADJ=1"
)

SERVICE_DEFAULTS=$(
Expand Down
18 changes: 18 additions & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ for var in PREFIX \
SHUTDOWN_PREFIX \
BUILD_SHUTDOWN \
SUPPORT_CGROUPS \
SUPPORT_CAPABILITIES \
SUPPORT_IOPRIO \
SUPPORT_OOM_ADJ \
USE_UTMPX \
USE_INITGROUPS \
SYSCONTROLSOCKET \
Expand Down Expand Up @@ -239,6 +242,12 @@ for arg in "$@"; do
--disable-shutdown|--enable-shutdown=no) BUILD_SHUTDOWN=no ;;
--enable-cgroups|--enable-cgroups=yes) SUPPORT_CGROUPS=1 ;;
--disable-cgroups|--enable-cgroups=no) SUPPORT_CGROUPS=0 ;;
--enable-capabilities|--enable-capabilities=yes) SUPPORT_CAPABILITIES=1 ;;
--disable-capabilities|--enable-capabilities=no) SUPPORT_CAPABILITIES=0 ;;
--enable-ioprio|--enable-ioprio=yes) SUPPORT_IOPRIO=1 ;;
--disable-ioprio|--enable-ioprio=no) SUPPORT_IOPRIO=0 ;;
--enable-oom-adj|--enable-oom-adj=yes) SUPPORT_OOM_ADJ=1 ;;
--disable-oom-adj|--enable-oom-adj=no) SUPPORT_OOM_ADJ=0 ;;
--enable-utmpx|--enable-utmpx=yes) USE_UTMPX=1 ;;
--disable-utmpx|--enable-utmpx=no) USE_UTMPX=0 ;;
--enable-initgroups|--enable-initgroups=yes) USE_INITGROUPS=1 ;;
Expand Down Expand Up @@ -278,10 +287,16 @@ done
if [ "$PLATFORM" = "Linux" ]; then
: "${BUILD_SHUTDOWN:="yes"}"
: "${SUPPORT_CGROUPS:="1"}"
: "${SUPPORT_CAPABILITIES:="1"}"
: "${SUPPORT_IOPRIO:="1"}"
: "${SUPPORT_OOM_ADJ:="1"}"
: "${SYSCONTROLSOCKET:="/run/dinitctl"}"
else
: "${BUILD_SHUTDOWN:="no"}"
: "${SUPPORT_CGROUPS:="0"}"
: "${SUPPORT_CAPABILITIES:="0"}"
: "${SUPPORT_IOPRIO:="0"}"
: "${SUPPORT_OOM_ADJ:="0"}"
: "${SYSCONTROLSOCKET:="/var/run/dinitctl"}"
fi

Expand Down Expand Up @@ -467,6 +482,9 @@ STRIPOPTS=$STRIPOPTS
# Feature settings
SUPPORT_CGROUPS=$SUPPORT_CGROUPS
USE_INITGROUPS=$USE_INITGROUPS
SUPPORT_CAPABILITIES=$SUPPORT_CAPABILITIES
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If SUPPORT_CAPABILITIES is 1 then -lcap needs to be added to LDFLAGS and TEST_LDFLAGS if they haven't been specified manually. See for example the tests under the heading "# Check whether sanitizers can be used for tests" (that's only dealing with TEST_LDFLAGS but the general principle should be the same for LDFLAGS).

SUPPORT_IOPRIO=$SUPPORT_IOPRIO
SUPPORT_OOM_ADJ=$SUPPORT_OOM_ADJ

# Optional settings
SHUTDOWN_PREFIX=${SHUTDOWN_PREFIX:-}
Expand Down
53 changes: 53 additions & 0 deletions doc/manpages/dinit-service.5.m4
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,12 @@ See the \fBRESOURCE LIMITS\fR section.
Note that some operating systems (notably, OpenBSD) do not support this limit; the
setting will be ignored on such systems.
.TP
\fBnice\fR = \fInice-value\fR
Specifies the CPU priority of the process.
When the given value is out of range for the operating system, it will be clamped to
supported range, but no error will be issued.
On Linux, this also sets the autogroup priority, assuming procfs is mounted.
.TP
\fBrun\-in\-cgroup\fR = \fIcgroup-path\fR
Run the service process(es) in the specified cgroup (see \fBcgroups\fR(7)).
The cgroup is specified as a path; if it has a leading slash, the remainder of the path is
Expand All @@ -557,6 +563,46 @@ The named cgroup must already exist prior to the service starting; it will not b
\fBdinit\fR.
.IP
This setting is only available if \fBdinit\fR was built with cgroups support.
.TP
\fBcapabilities\fR = \fIiab\fR
.TQ
\fBcapabilities\fR += \fIiab-addendum\fR
Run the service process(es) with capabilities specified by \fIiab\fR (see \fBcapabilities\fR(7)).
The syntax follows the regular capabilities IAB format, with comma-separated capabilities.
The append form of this setting will add to the previous IAB string, automatically adding
a comma to the previous string, so you do not need to add it manually.
.IP
This setting is only available if \fBdinit\fR was built with capabilities support.
.TP
\fBsecure\-bits\fR = \fIsecbits\fR
.TQ
\fBsecure\-bits\fR += \fIsecbits-addendum\fR
This is a companion option to \fBcapabilities\fR, specifying the secure bits for the
process.
Here, it is a space-separated list of keywords. The allowed keywords are \fIkeep-caps\fR,
\fIno-setuid-fixup\fR, \fInoroot\fR, and variants of the three with the \fI-locked\fR
suffix.
The append form can be used to add more secure bits, with everything being ORed together
at the end and used as an integer.
.IP
This setting is only available if \fBdinit\fR was built with capabilities support.
.TP
\fBioprio\fR = \fIioprio-value\fR
Specifies the I/O priority class and value for the process.
The permitted values are \fInone\fR, \fIidle\fR, \fIrealtime:PRIO\fR, and
\fIbest-effort:PRIO\fR, where \fIPRIO\fR is an integer value no less than 0
and no more than 7.
.IP
This setting is only available if \fBdinit\fR was built with ioprio support.
.TP
\fBoom-score-adj\fR = \fIadj-value\fR
Specifies the OOM killer score adjustment for the service.
The value is an integer no less than -1000 and no more than 1000.
.IP
This setting is only available if \fBdinit\fR was built with OOM score adjustment support.
.IP
This setting requires the proc filesystem to be mounted, and will result in a
service startup failure if that is not the case.
.\"
.SS OPTIONS
.\"
Expand Down Expand Up @@ -685,6 +731,13 @@ is suggested, i.e. every other service should either be a (possibly transitive)
dependent of the service with this option set.
.IP
This option can be used for scripted and internal services only.
.TP
\fBno\-new\-privs\fR
Normally, child processes can gain privileges that their parent did not have, such
as setuid or setgid and file capabilities. This option can be specified to prevent
the service from gaining such privileges.
.IP
This setting is only available if \fBdinit\fR was built with capabilities support.
.\"
.SS RESOURCE LIMITS
.\"
Expand Down
13 changes: 10 additions & 3 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ igr_tests = get_option('igr-tests')
fuzzer = get_option('fuzzer')
man_pages = get_option('man-pages')
support_cgroups = get_option('support-cgroups')
support_capabilities = get_option('support-capabilities')
support_ioprio = get_option('support-ioprio')
support_oom_adj = get_option('support-oom-adj')
use_utmpx = get_option('use-utmpx')
use_initgroups = get_option('use-initgroups')
default_auto_restart = get_option('default-auto-restart')
Expand All @@ -56,6 +59,9 @@ if platform == 'freebsd' and compiler.has_link_argument('-lrt')
add_project_link_arguments('-lrt', language : 'cpp')
endif

## Dependencies
libcap_dep = dependency('libcap', required: support_capabilities)
q66 marked this conversation as resolved.
Show resolved Hide resolved

## Prepare mconfig.h
mconfig_data.set_quoted('DINIT_VERSION', version)
mconfig_data.set_quoted('SYSCONTROLSOCKET', dinit_control_socket_path)
Expand All @@ -65,9 +71,10 @@ mconfig_data.set('DEFAULT_AUTO_RESTART', default_auto_restart)
mconfig_data.set('DEFAULT_START_TIMEOUT', default_start_timeout)
mconfig_data.set('DEFAULT_STOP_TIMEOUT', default_stop_timeout)
mconfig_data.set10('USE_INITGROUPS', use_initgroups)
if support_cgroups.auto() and platform == 'linux' or support_cgroups.enabled()
mconfig_data.set('SUPPORT_CGROUPS', '1')
endif
mconfig_data.set10('SUPPORT_CGROUPS', support_cgroups.auto() and platform == 'linux' or support_cgroups.enabled())
mconfig_data.set10('SUPPORT_CAPABILITIES', libcap_dep.found() and not support_capabilities.disabled())
mconfig_data.set10('SUPPORT_IOPRIO', support_ioprio.auto() and platform == 'linux' or support_ioprio.enabled())
mconfig_data.set10('SUPPORT_OOM_ADJ', support_oom_adj.auto() and platform == 'linux' or support_oom_adj.enabled())
if use_utmpx.enabled() or (use_utmpx.auto() and compiler.has_header_symbol('utmpx.h', '_PATH_UTMPX') and
compiler.has_header_symbol('utmpx.h', '_PATH_WTMPX'))
mconfig_data.set('USE_UTMPX', '1')
Expand Down
20 changes: 19 additions & 1 deletion meson_options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,25 @@ option(
'support-cgroups',
type : 'feature',
value : 'auto',
description : 'Enable Cgroups supprot.'
description : 'Enable Cgroups support.'
)
option(
'support-capabilities',
type : 'feature',
value : 'auto',
description : 'Enable capabilities support.'
)
option(
'support-ioprio',
type : 'feature',
value : 'auto',
description : 'Enable ioprio support.'
)
option(
'support-oom-adj',
type : 'feature',
value : 'auto',
description : 'Enable OOM score adjustment support.'
)
option(
'build-shutdown',
Expand Down
4 changes: 4 additions & 0 deletions src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ ifeq ($(BUILD_SHUTDOWN),yes)
SHUTDOWN=$(SHUTDOWN_PREFIX)shutdown
endif

ifeq ($(SUPPORT_CAPABILITIES),1)
ALL_LDFLAGS+=-lcap
endif
Comment on lines +12 to +14
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should already be LDFLAGS if it's needed (see review comments elsewhere for further details).


dinit_objects = dinit.o load-service.o service.o proc-service.o baseproc-service.o control.o dinit-log.o \
dinit-main.o run-child-proc.o options-processing.o dinit-env.o settings.o

Expand Down
14 changes: 14 additions & 0 deletions src/baseproc-service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,23 @@ bool base_process_service::start_ps_process(const std::vector<const char *> &cmd
run_params.env_file = env_file.c_str();
run_params.output_fd = log_output_fd;
run_params.input_fd = input_fd;
run_params.nice_is_set = nice_is_set;
run_params.nice = nice;
#if SUPPORT_CGROUPS
run_params.run_in_cgroup = run_in_cgroup.c_str();
#endif
#if SUPPORT_CAPABILITIES
run_params.cap_iab = cap_iab.get();
run_params.secbits = secbits;
run_params.no_new_privs = onstart_flags.no_new_privs;
#endif
#if SUPPORT_IOPRIO
run_params.ioprio = ioprio;
#endif
#if SUPPORT_OOM_ADJ
run_params.oom_adj_is_set = oom_adj_is_set;
run_params.oom_adj = oom_adj;
#endif
run_child_proc(run_params);
}
else {
Expand Down
Loading