mirrors/systemd
0
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2024-11-23 10:13:34 +08:00
Code Issues Packages Projects Releases Wiki Activity

pidref: add explicit concept of "remote" PidRef

Browse Source 
This PidRef just track some data, but cannot be used for any active
operation.

Background: for https://github.com/systemd/systemd/pull/34703 it makes
sense to track explicitly if some PidRef is not a local one, so that we
never attempt to for example "kill a remote process" and thus
acccidentally hit the wrong process (i.e. a local one by the same PID).
This commit is contained in:
Lennart Poettering 2024-10-11 11:33:42 +02:00
parent e1efa9d804
commit 7e3e540b88
3 changed files with 99 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
src/basic/pidref.c
View File

@ -40,6 +40,9 @@ int pidref_acquire_pidfd_id(PidRef *pidref) {
if (!pidref_is_set(pidref))
return -ESRCH;
if (pidref_is_remote(pidref))
return -EREMOTE;
if (pidref->fd < 0)
return -ENOMEDIUM;
@ -64,23 +67,36 @@ int pidref_acquire_pidfd_id(PidRef *pidref) {
bool pidref_equal(PidRef *a, PidRef *b) {
if (pidref_is_set(a)) {
if (!pidref_is_set(b))
if (!pidref_is_set(a))
return !pidref_is_set(b);
if (!pidref_is_set(b))
return false;
if (a->pid != b->pid)
return false;
if (pidref_is_remote(a)) {
/* If one is remote and the other isn't, they are not the same */
if (!pidref_is_remote(b))
return false;
if (a->pid != b->pid)
/* If both are remote, compare fd IDs if we have both, otherwise don't bother, and cut things short */
if (a->fd_id == 0 || b->fd_id == 0)
return true;
} else {
if (pidref_is_remote(b))
return false;
/* Try to compare pidfds using their inode numbers. This way we can ensure that we don't
* spuriously consider two PidRefs equal if the pid has been reused once. Note that we
* ignore all errors here, not only EOPNOTSUPP, as fstat() might fail due to many reasons. */
/* Try to compare pidfds using their inode numbers. This way we can ensure that we
* don't spuriously consider two PidRefs equal if the pid has been reused once. Note
* that we ignore all errors here, not only EOPNOTSUPP, as fstat() might fail due to
* many reasons. */
if (pidref_acquire_pidfd_id(a) < 0 || pidref_acquire_pidfd_id(b) < 0)
return true;
return a->fd_id == b->fd_id;
}
return !pidref_is_set(b);
return a->fd_id == b->fd_id;
}
int pidref_set_pid(PidRef *pidref, pid_t pid) {
@ -240,7 +256,9 @@ int pidref_copy(const PidRef *pidref, PidRef *dest) {
assert(dest);
if (pidref) {
if (pidref->fd >= 0) {
if (pidref_is_remote(pidref)) /* Propagate remote flag */
dup_fd = -EREMOTE;
else if (pidref->fd >= 0) {
dup_fd = fcntl(pidref->fd, F_DUPFD_CLOEXEC, 3);
if (dup_fd < 0) {
if (!ERRNO_IS_RESOURCE(errno))
@ -311,6 +329,9 @@ int pidref_kill(const PidRef *pidref, int sig) {
if (!pidref)
return -ESRCH;
if (pidref_is_remote(pidref))
return -EREMOTE;
if (pidref->fd >= 0)
return RET_NERRNO(pidfd_send_signal(pidref->fd, sig, NULL, 0));
@ -338,6 +359,9 @@ int pidref_sigqueue(const PidRef *pidref, int sig, int value) {
if (!pidref)
return -ESRCH;
if (pidref_is_remote(pidref))
return -EREMOTE;
if (pidref->fd >= 0) {
siginfo_t si;
@ -370,6 +394,9 @@ int pidref_verify(const PidRef *pidref) {
if (!pidref_is_set(pidref))
return -ESRCH;
if (pidref_is_remote(pidref))
return -EREMOTE;
if (pidref->pid == 1)
return 1; /* PID 1 can never go away, hence never be recycled to a different process → return 1 */
@ -387,6 +414,9 @@ bool pidref_is_self(const PidRef *pidref) {
if (!pidref)
return false;
if (pidref_is_remote(pidref))
return false;
return pidref->pid == getpid_cached();
}
@ -396,6 +426,9 @@ int pidref_wait(const PidRef *pidref, siginfo_t *ret, int options) {
if (!pidref_is_set(pidref))
return -ESRCH;
if (pidref_is_remote(pidref))
return -EREMOTE;
if (pidref->pid == 1 || pidref->pid == getpid_cached())
return -ECHILD;

37
src/basic/pidref.h
View File

@ -6,10 +6,35 @@ typedef struct PidRef PidRef;
#include "macro.h"
#include "process-util.h"
/* An embeddable structure carrying a reference to a process. Supposed to be used when tracking processes continuously. */
/* An embeddable structure carrying a reference to a process. Supposed to be used when tracking processes
* continuously. This combines a PID, a modern Linux pidfd and the 64bit inode number of the pidfd into one
* structure. Note that depending on kernel support the pidfd might not be initialized, and if it is
* initialized then fd_id might still not be initialized (because the concept was added to the kernel much
* later than pidfds themselves).
*
* There are three special states a PidRef can be in:
*
* 1. It can be *unset*. Use pidref_is_set() to detect this case. Most operations attempted on such a PidRef
* will fail with -ESRCH. Use PIDREF_NULL for initializing a PidRef in this state.
*
* 2. It can be marked as *automatic*. This is a special state indicating that a process reference is
* supposed to be derived automatically from the current context. This is used by the Varlink/JSON
* dispatcher as indication that a PidRef shall be derived from the connection peer, but might be
* otherwise used too. When marked *automatic* the PidRef will also be considered *unset*, hence most
* operations will fail with -ESRCH, as above.
*
* 3. It can be marked as *remote*. This is useful when deserializing a PidRef structure from an IPC message
* or similar, and it has been determined that the given PID definitely doesn't refer to a local
* process. In this case the PidRef logic will refrain from trying to acquire a pidfd for the
* process. Moreover, most operations will fail with -EREMOTE. Only PidRef structures that are not marked
* *unset* can be marked *remote*.
*/
struct PidRef {
pid_t pid; /* always valid */
int fd; /* only valid if pidfd are available in the kernel, and we manage to get an fd */
pid_t pid; /* > 0 if the PidRef is set, otherwise set to PID_AUTOMATIC if automatic mode is
* desired, or 0 otherwise. */
int fd; /* only valid if pidfd are available in the kernel, and we manage to get an fd. If we
* know that the PID is not from the local machine we set this to -EREMOTE, otherwise
* we use -EBADF as indicator the fd is invalid. */
uint64_t fd_id; /* the inode number of pidfd. only useful in kernel 6.9+ where pidfds live in
their own pidfs and each process comes with a unique inode number */
};
@ -31,6 +56,12 @@ static inline bool pidref_is_set(const PidRef *pidref) {
bool pidref_is_automatic(const PidRef *pidref);
static inline bool pidref_is_remote(const PidRef *pidref) {
/* If the fd is set to -EREMOTE we assume PidRef does not refer to a local PID, but on another
* machine (and we just got the PidRef initialized due to deserialization of some RPC message) */
return pidref_is_set(pidref) && pidref->fd == -EREMOTE;
}
int pidref_acquire_pidfd_id(PidRef *pidref);
bool pidref_equal(PidRef *a, PidRef *b);

22
src/test/test-pidref.c
View File

@ -240,4 +240,26 @@ TEST(pidref_is_automatic) {
assert_se(!pid_is_valid(PID_AUTOMATIC));
}
TEST(pidref_is_remote) {
assert_se(!pidref_is_remote(NULL));
assert_se(!pidref_is_remote(&PIDREF_NULL));
assert_se(!pidref_is_remote(&PIDREF_MAKE_FROM_PID(1)));
assert_se(!pidref_is_remote(&PIDREF_MAKE_FROM_PID(getpid_cached())));
assert_se(!pidref_is_remote(&PIDREF_AUTOMATIC));
static const PidRef p = {
.pid = 1,
.fd = -EREMOTE,
.fd_id = 4711,
};
assert_se(pidref_is_set(&p));
assert_se(pidref_is_remote(&p));
assert_se(!pidref_is_automatic(&p));
assert_se(pidref_kill(&p, SIGTERM) == -EREMOTE);
assert_se(pidref_kill_and_sigcont(&p, SIGTERM) == -EREMOTE);
assert_se(pidref_wait_for_terminate(&p, /* ret= */ NULL) == -EREMOTE);
assert_se(pidref_verify(&p) == -EREMOTE);
}
DEFINE_TEST_MAIN(LOG_DEBUG);