| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
| |
The glibc API is behind the wrapper is called "secure_getenv()", hence
our wrapper really should keep the order too, otherwise things are just
too confusing.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
FORK_DEATHSIG_SIGTERM
Sometimes it makes sense to hard kill a client if we die. Let's hence
add a third FORK_DEATHSIG flag for this purpose: FORK_DEATHSIG_SIGKILL.
To make things less confusing this also renames FORK_DEATHSIG to
FORK_DEATHSIG_SIGTERM to make clear it sends SIGTERM. We already had
FORK_DEATHSIG_SIGINT, hence this makes things nicely symmetric.
A bunch of users are switched over for FORK_DEATHSIG_SIGKILL where we
know it's safe to abort things abruptly. This should make some kernel
cases more robust, since we cannot get confused by signal masks or such.
While we are at it, also fix a bunch of bugs where we didn't take
FORK_DEATHSIG_SIGINT into account in safe_fork()
|
|
|
|
|
|
|
|
| |
We use it for more than just pipe() arrays. For example also for
socketpair(). Hence let's give it a generic name.
Also add EBADF_TRIPLET to mirror this for things like
stdin/stdout/stderr arrays, which we use a bunch of times.
|
|
|
|
|
| |
Also drop do_poll as the use case is covered
by timeout.
|
|
|
|
| |
And make safe_fork_full() takes fds to be assigned to stdio.
|
| |
|
|
|
|
| |
In some places, initialization is dropped when unnecesary.
|
|
|
|
|
| |
util.h is now about logarithms only, so we can rename it. Many files included
util.h for no apparent reason… Those includes are dropped.
|
|
|
|
|
|
|
|
| |
I had a strange failure where the pager was hanging on invocation (gdm crashed
and the kernel got into a strange state where it was hanging on some tasks).
Based on the logs from 'SYSTEMCTL_LOG_LEVEL=debug journalctl', I couldn't even
tell which pager binary we're executing. So let's shorten the function a bit and
provide a bit more detail.
|
|
|
|
|
|
|
|
|
|
|
| |
Same idea as 03677889f0ef42cdc534bf3b31265a054b20a354.
No functional change intended. The type of the iterator is generally changed to
be 'const char*' instead of 'char*'. Despite the type commonly used, modifying
the string was not allowed.
I adjusted the naming of some short variables for clarity and reduced the scope
of some variable declarations in code that was being touched anyway.
|
|
|
|
| |
Avoid doing stack allocations on environment variables.
|
|\
| |
| | |
A coding style tweak and checking of sd_notify() calls and voidification of pager_open()
|
| | |
|
|/ |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Let's define two helpers strdupa_safe() + strndupa_safe() which do the
same as their non-safe counterparts, except that they abort if called
with allocations larger than ALLOCA_MAX.
This should ensure that all our alloca() based allocations are subject
to this limit.
afaics glibc offers three alloca() based APIs: alloca() itself,
strndupa() + strdupa(). With this we have now replacements for all of
them, that take the limit into account.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Clean up ignore_signals() + default_signals() + sigaction_many() a bit:
make it unnecessary to explicitly terminate the signal list with -1.
Merge all three calls into a single function that is just called with
slightly different parameters. And eliminate an unnecessary extra
iteration in its inner for() loop.
No change in behaviour.
|
| |
|
| |
|
|
|
|
|
|
|
|
| |
I can't think of any real vulnerability about this, but it still feels
better to check a variable with "secure" in its name with
secure_getenv() rather than plain getenv().
Paranoia FTW!
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The variable is renamed to SYSTEMD_PAGERSECURE (because it's not just about
less now), and we automatically enable secure mode in certain cases, but not
otherwise.
This approach is more nuanced, but should provide a better experience for
users:
- Previusly we would set LESSSECURE=1 and trust the pager to make use of
it. But this has an effect only on less. We need to not start pagers which
are insecure when in secure mode. In particular more is like that and is a
very popular pager.
- We don't enable secure mode always, which means that those other pagers can
reasonably used.
- We do the right thing by default, but the user has ultimate control by
setting SYSTEMD_PAGERSECURE.
Fixes #5666.
v2:
- also check $PKEXEC_UID
v3:
- use 'sd_pid_get_owner_uid() != geteuid()' as the condition
|
|
|
|
|
|
|
|
|
| |
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.
Prompted by #5666
|
|
|
|
|
|
|
|
|
|
|
|
| |
"less" doesn't properly reset its terminal on SIGTERM, it does so only
on SIGINT. Let's thus configure SIGINT instead of SIGTERM.
I think this is something less should fix too, and clean up things
correctly on SIGTERM, too. However, given that we explicitly enable
SIGINT behaviour by passing "K" to $LESS I figure it makes sense if we
also send SIGINT instead of SIGTERM to match it.
Fixes: #16084
|
| |
|
| |
|
|
|
|
|
|
|
| |
The parent is waiting for an EOF on the pipe transmitting the pager name
before starting to send data. With external pagers this happens due to
execlp() CLOEXEC'ing the pipe, so the internal pager needs to close it
manually.
|
|
|
|
|
| |
This means we need to include many more headers in various files that simply
included util.h before, but it seems cleaner to do it this way.
|
| |
|
|
|
|
|
|
| |
This reverts commit 55844aebb60b42f56335eb147dd395269d8fb686.
As `exe_name_pipe` are closed by safe_fork().
|
| |
|
|
|
|
|
|
| |
The code so far logged about some errors but was silent on others. Let's
stream-line that and make the function fully self-logging on all error
conditions.
|
|
|
|
|
|
|
| |
Similar to the previous commit: in many cases no further fd processing
needs to be done in forked of children before execve() or any of its
flavours are called. In those case we can use FORK_RLIMIT_NOFILE_SAFE
instead.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Whenever we invoke external, foreign code from code that has
RLIMIT_NOFILE's soft limit bumped to high values, revert it to 1024
first. This is a safety precaution for compatibility with programs using
select() which cannot operate with fds > 1024.
This commit adds the call to rlimit_nofile_safe() to all invocations of
exec{v,ve,l}() and friends that either are in code that we know runs
with RLIMIT_NOFILE bumped up (which is PID 1 and all journal code for
starters) or that is part of shared code that might end up there.
The calls are placed as early as we can in processes invoking a flavour
of execve(), but after the last time we do fd manipulations, so that we
can still take benefit of the high fd limits for that.
|
|
|
|
|
| |
This is high-level functionality, and fits better in shared/ (which is for
our executables), than in basic/ (which is also for libraries).
|
|
|
|
|
|
|
|
|
| |
pager.[ch] doesn't use any APIs from src/libsystemd/ or src/shared/
hence there's no reason for it to be in src/shared/, let's move it to
src/basic/ instead.
This enables us to use pager.[ch] APIs from other code in src/basic/,
for example pager_have() and suchlike.
|
|
|
|
|
|
|
|
|
|
| |
Files which are installed as-is (any .service and other unit files, .conf
files, .policy files, etc), are left as is. My assumption is that SPDX
identifiers are not yet that well known, so it's better to retain the
extended header to avoid any doubt.
I also kept any copyright lines. We can probably remove them, but it'd nice to
obtain explicit acks from all involved authors before doing that.
|
|
|
|
|
|
|
|
|
|
|
|
| |
"noreturn" is reserved and can be used in other header files we include:
[ 16s] In file included from /usr/include/gcrypt.h:30:0,
[ 16s] from ../src/journal/journal-file.h:26,
[ 16s] from ../src/journal/journal-vacuum.c:31:
[ 16s] /usr/include/gpg-error.h:1544:46: error: expected ‘,’ or ‘;’ before ‘)’ token
[ 16s] void gpgrt_log_bug (const char *fmt, ...) GPGRT_ATTR_NR_PRINTF(1,2);
Here we include grcrypt.h (which in turns include gpg-error.h) *after* we
"noreturn" was defined in macro.h.
|
|
|
|
|
| |
let's keep static variables together, and before the function
definitions.
|
|
|
|
|
|
| |
We maintain static process-wide variables in these subsystems without
locking, hence let's refuse operation unless we are called from the main
thread (which we do anyway) just as a safety precaution.
|
|
|
|
|
|
| |
Using wait_for_terminate_and_check() instead of wait_for_terminate()
let's us simplify, shorten and unify the return value checking and
logging of waitid(). Hence, let's use it all over the place.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds a new safe_fork() wrapper around fork() and makes use of it
everywhere. The new wrapper does a couple of things we previously did
manually and separately in a safer, more correct and automatic way:
1. Optionally resets signal handlers/mask in the child
2. Sets a name on all processes we fork off right after forking off (and
the patch assigns useful names for all processes we fork off now,
following a systematic naming scheme: always enclosed in () – in order
to indicate that these are not proper, exec()ed processes, but only
forked off children, and if the process is long-running with only our
own code, without execve()'ing something else, it gets am "sd-" prefix.)
3. Optionally closes all file descriptors in the child
4. Optionally sets a PR_SET_DEATHSIG to SIGTERM in the child, in a safe
way so that the parent dying before this happens being handled
safely.
5. Optionally reopens the logs
6. Optionally connects stdin/stdout/stderr to /dev/null
7. Debug logs about the forked off processes.
|
|
|
|
|
| |
This follows what the kernel is doing, c.f.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5fd54ace4721fc5ce2bb5aef6318fcf17f421460.
|
|
|
|
| |
Not that we need it, but let's do this as matter of completeness.
|
|
|
|
|
|
|
|
| |
We make copies (without O_CLOEXEC) of the fds anyway before using them,
hence let's be safe and create them with O_CLOEXEC first, so that we
don't run into issues should pager_open() be called in a threaded
environment where another thread fork()s at the wrong time and ends up
with fds not marked O_CLOEXEC.
|
|
|
|
|
|
|
| |
This moves pretty much all uses of getpid() over to getpid_raw(). I
didn't specifically check whether the optimization is worth it for each
replacement, but in order to keep things simple and systematic I
switched over everything at once.
|
| |
|
|
|
|
| |
- result of dup2 should be compared against 0
- making parenthesis around comparisons just to be sure
|
| |
|
| |
|