From: Christian Goeschel Ndjomouo Date: Mon, 25 Aug 2025 18:04:19 +0000 (-0400) Subject: kill: add support for race-free process kills using pidfd inodes X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=39a42a53625c6cf2b6d8cb64394db97267962b44;p=thirdparty%2Futil-linux.git kill: add support for race-free process kills using pidfd inodes The 6.9 Linux kernel added support for pidfds which introduces inodes that stay unique for the system lifetime and can be used to reference a process with both its traditional pid and pid fd inode number. This enables race-free killing of processes and protects from false referencing due to pid rollovers. This patch adds a new way of addressing processes with the format 'pid:pidfd_inode'. When the new format is used, 'kill' assumes the user wishes to use a pidfd to signal a process, and will therefore use pidfd_* routines to complete the task if the provided inode matches the one from a previously acquired pidfd. Addresses: #3252 Signed-off-by: Christian Goeschel Ndjomouo --- diff --git a/include/Makemodule.am b/include/Makemodule.am index bdf87e221..59ecc793f 100644 --- a/include/Makemodule.am +++ b/include/Makemodule.am @@ -54,6 +54,7 @@ dist_noinst_HEADERS += \ include/partx.h \ include/path.h \ include/pathnames.h \ + include/pidutils.h \ include/pidfd-utils.h \ include/plymouth-ctrl.h \ include/procfs.h \ diff --git a/include/pidutils.h b/include/pidutils.h new file mode 100644 index 000000000..9d3257dfd --- /dev/null +++ b/include/pidutils.h @@ -0,0 +1,14 @@ +/* + * No copyright is claimed. This code is in the public domain; do with + * it what you wish. + * + * Authors: Christian Goeschel Ndjomouo [2025] + */ +#ifndef UTIL_LINUX_PIDUTILS_H +#define UTIL_LINUX_PIDUTILS_H + +#include + +extern int ul_parse_pid_str(char *pidstr, pid_t *pid_num, ino_t *pfd_ino); + +#endif /* UTIL_LINUX_PIDUTILS_H */ \ No newline at end of file diff --git a/lib/Makemodule.am b/lib/Makemodule.am index bf24b6bee..5f9cc5e44 100644 --- a/lib/Makemodule.am +++ b/lib/Makemodule.am @@ -30,6 +30,7 @@ libcommon_la_SOURCES = \ lib/mbsalign.c \ lib/mbsedit.c\ lib/md5.c \ + lib/pidutils.c \ lib/pwdutils.c \ lib/randutils.c \ lib/sha1.c \ diff --git a/lib/meson.build b/lib/meson.build index 25febbc19..866240a82 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -17,6 +17,7 @@ lib_common_sources = ''' mbsalign.c mbsedit.c md5.c + pidutils.c procfs.c pwdutils.c randutils.c diff --git a/lib/pidutils.c b/lib/pidutils.c new file mode 100644 index 000000000..5007cba3e --- /dev/null +++ b/lib/pidutils.c @@ -0,0 +1,49 @@ +/* + * No copyright is claimed. This code is in the public domain; do with + * it what you wish. + * + * Authors: Christian Goeschel Ndjomouo [2025] + */ +#include +#include +#include + +#include "strutils.h" +#include "pidutils.h" + +/* + * ul_parse_pid_str() - Parse a string and store the found pid and/or pidfd inode. + * + * @pidstr: string in format `pid:pidfd_inode` that is to be parsed + * @pid_num: stores pid number + * @pfd_ino: stores pidfd inode number + * + * If @pfd_ino is not destined to be set, pass it as NULL. + * + * Return: On success, 0 is returned. + * On failure, a negative errno number will be returned. + */ +int ul_parse_pid_str(char *pidstr, pid_t *pid_num, ino_t *pfd_ino) +{ + int rc; + char *end = NULL; + int64_t num = 0; + + if (!pidstr || !*pidstr || !pid_num) + return -EINVAL; + + num = strtoimax(pidstr, &end, 10); + if (errno == 0 && ((num && num < 1) || (num && num > SINT_MAX(pid_t)))) + return -ERANGE; + *pid_num = (pid_t) num; + + if (*end == ':' && pfd_ino) { + rc = ul_strtou64(++end, pfd_ino, 10); + if (rc != 0) + return -ERANGE; + *end = '\0'; + } + if (errno != 0 || ((end && *end != '\0') || pidstr >= end)) + return -EINVAL; + return 0; +} diff --git a/meson.build b/meson.build index e3038240a..be3e617cf 100644 --- a/meson.build +++ b/meson.build @@ -3897,6 +3897,16 @@ exe = executable( build_by_default: program_tests) exes += exe +exe = executable( + 'test_kill_pidfdino', + 'tests/helpers/test_kill_pidfdino.c', + include_directories : includes, + link_with : lib_common, + build_by_default: program_tests) +if not is_disabler(exe) + exes += exe +endif + if LINUX and lib_rt.found() exe = executable( 'test_mkfds', diff --git a/misc-utils/kill.1.adoc b/misc-utils/kill.1.adoc index 169f25866..ec5afed36 100644 --- a/misc-utils/kill.1.adoc +++ b/misc-utils/kill.1.adoc @@ -19,7 +19,9 @@ kill - terminate a process == SYNOPSIS -*kill* [**-**_signal_|*-s* _signal_|*-p*] [*-q* _value_] [*-a*] [*--timeout* _milliseconds_ _signal_] [*--*] _pid_|_name_... +*kill* [**-**_signal_|*-s* _signal_|*-p*] [*-q* _value_] [*-a*] [*--timeout* _milliseconds_ _signal_] [*--*] _pid_|_pid_:_pidfd_inode_|_name_... + +*kill* [**-**_signal_|*-s* _signal_] _pid_|_pid_:_pidfd_inode_... *kill* *-l* [_number_|``0x``_sigmask_] | *-L* @@ -50,6 +52,9 @@ All processes with a PID larger than 1 are signaled. **-**__n__;; where _n_ is larger than 1. All processes in process group _n_ are signaled. When an argument of the form '-n' is given, and it is meant to denote a process group, either a signal must be specified first, or the argument must be preceded by a '--' option, otherwise it will be taken as the signal to send. +_pid_:_pidfd_inode_:: +A process can be referenced by its _pid_ plus _pidfd_inode_ (pid:pidfd_inode), to uniquely identify it and perform race-free signalling. This works only for the options -s, --signal and -_signal_. Requires kernel version 6.9 and later. + _name_:: All processes invoked using this _name_ will be signaled. diff --git a/misc-utils/kill.c b/misc-utils/kill.c index fd947b923..5b73d24e7 100644 --- a/misc-utils/kill.c +++ b/misc-utils/kill.c @@ -42,10 +42,12 @@ * * Copyright (C) 2014 Sami Kerola * Copyright (C) 2014 Karel Zak + * Copyright (C) 2025 Christian Goeschel Ndjomouo */ #include /* for isdigit() */ #include +#include #include #include #include @@ -55,6 +57,7 @@ #include "c.h" #include "closestream.h" #include "nls.h" +#include "pidutils.h" #include "pidfd-utils.h" #include "procfs.h" #include "pathnames.h" @@ -69,6 +72,7 @@ #if defined(HAVE_PIDFD_OPEN) && defined(HAVE_PIDFD_SEND_SIGNAL) # define USE_KILL_WITH_TIMEOUT 1 +# define USE_KILL_WITH_PIDFDINO 1 #endif enum { @@ -89,6 +93,7 @@ struct timeouts { struct kill_control { char *arg; pid_t pid; + ino_t pidfd_ino; int numsig; #ifdef HAVE_SIGQUEUE union sigval sigdata; @@ -261,7 +266,7 @@ static void __attribute__((__noreturn__)) usage(void) { FILE *out = stdout; fputs(USAGE_HEADER, out); - fprintf(out, _(" %s [options] |...\n"), program_invocation_short_name); + fprintf(out, _(" %s [options] |:|...\n"), program_invocation_short_name); fputs(USAGE_SEPARATOR, out); fputs(_("Forcibly terminate a process.\n"), out); @@ -302,6 +307,9 @@ static void __attribute__((__noreturn__)) print_kill_version(void) #endif #ifdef USE_KILL_WITH_TIMEOUT "pidfd", +#endif +#ifdef USE_KILL_WITH_PIDFDINO + "pidfdino", #endif }; @@ -531,6 +539,22 @@ static int kill_with_timeout(const struct kill_control *ctl) } #endif +#ifdef USE_KILL_WITH_PIDFDINO +static int validate_pfd_ino(int pfd, ino_t pfd_ino) +{ + int rc; + struct stat f; + + rc = fstat(pfd, &f); + if (rc != 0) + return -EINVAL; + + if (f.st_ino != pfd_ino) + return -EINVAL; + return 0; +} +#endif + static int kill_verbose(const struct kill_control *ctl) { int rc = 0; @@ -551,7 +575,24 @@ static int kill_verbose(const struct kill_control *ctl) rc = sigqueue(ctl->pid, ctl->numsig, ctl->sigdata); else #endif - rc = kill(ctl->pid, ctl->numsig); +#ifdef USE_KILL_WITH_PIDFDINO + if ((ctl->pidfd_ino > 0)) { + int pfd; + pfd = pidfd_open(ctl->pid, 0); + if (pfd < 0) + err(EXIT_FAILURE, _("pidfd_open() failed: %d"), ctl->pid); + + rc = validate_pfd_ino(pfd, ctl->pidfd_ino); + if (rc < 0) + errx(EXIT_FAILURE, _("pidfd inode %"PRIu64" not found for pid %d: %s"), + ctl->pidfd_ino, ctl->pid, strerror(-rc)); + + rc = pidfd_send_signal(pfd, ctl->numsig, 0, 0); + if (rc < 0) + err(EXIT_FAILURE, _("pidfd_send_signal() failed")); + } else +#endif + rc = kill(ctl->pid, ctl->numsig); if (rc < 0) warn(_("sending signal to %s failed"), ctl->arg); @@ -587,7 +628,7 @@ static int check_signal_handler(const struct kill_control *ctl) int main(int argc, char **argv) { struct kill_control ctl = { .numsig = SIGTERM }; - int nerrs = 0, ct = 0; + int nerrs = 0, ct = 0, rc = 0; setlocale(LC_ALL, ""); bindtextdomain(PACKAGE, LOCALEDIR); @@ -601,11 +642,10 @@ int main(int argc, char **argv) /* The rest of the arguments should be process ids and names. */ for ( ; (ctl.arg = *argv) != NULL; argv++) { - char *ep = NULL; - errno = 0; - ctl.pid = strtol(ctl.arg, &ep, 10); - if (errno == 0 && ep && *ep == '\0' && ctl.arg < ep) { + + rc = ul_parse_pid_str(ctl.arg, &ctl.pid, &ctl.pidfd_ino); + if(errno == 0 && rc == 0) { if (check_signal_handler(&ctl) <= 0) continue; if (kill_verbose(&ctl) != 0) diff --git a/tests/commands.sh b/tests/commands.sh index fe989a541..f6dfc3790 100644 --- a/tests/commands.sh +++ b/tests/commands.sh @@ -63,6 +63,7 @@ TS_HELPER_MKFDS="${ts_helpersdir}test_mkfds" TS_HELPER_BLKID_FUZZ="${ts_helpersdir}test_blkid_fuzz" TS_HELPER_PROCFS="${ts_helpersdir}test_procfs" TS_HELPER_TIMEUTILS="${ts_helpersdir}test_timeutils" +TS_HELPER_KILL_PIDFDINO="${ts_helpersdir}test_kill_pidfdino" # paths to commands TS_CMD_ADDPART=${TS_CMD_ADDPART:-"${ts_commandsdir}addpart"} diff --git a/tests/expected/build-sys/config-all-devel b/tests/expected/build-sys/config-all-devel index da9afc1be..2667ba5df 100644 --- a/tests/expected/build-sys/config-all-devel +++ b/tests/expected/build-sys/config-all-devel @@ -102,7 +102,7 @@ setterm: libtinfo write: test_byteswap: test_md5: -test_pathnames: +test_pathnames: test_sysinfo: col: colcrt: @@ -113,4 +113,5 @@ line: more: libtinfo pg: libncursesw libtinfo rev: -ul: libtinfo +ul: libtinfo +test_kill_pidfdino: diff --git a/tests/expected/kill/pidfdino b/tests/expected/kill/pidfdino new file mode 100644 index 000000000..d48ce7299 --- /dev/null +++ b/tests/expected/kill/pidfdino @@ -0,0 +1 @@ +all ok diff --git a/tests/helpers/Makemodule.am b/tests/helpers/Makemodule.am index fac8608cd..f87675350 100644 --- a/tests/helpers/Makemodule.am +++ b/tests/helpers/Makemodule.am @@ -35,6 +35,10 @@ check_PROGRAMS += test_uuid_namespace test_uuid_namespace_SOURCES = tests/helpers/test_uuid_namespace.c \ libuuid/src/predefined.c libuuid/src/unpack.c libuuid/src/unparse.c +check_PROGRAMS += test_kill_pidfdino +test_kill_pidfdino_SOURCES = tests/helpers/test_kill_pidfdino.c +test_kill_pidfdino_LDADD = $(LDADD) libcommon.la + if LINUX check_PROGRAMS += test_mkfds test_mkfds_SOURCES = tests/helpers/test_mkfds.c tests/helpers/test_mkfds.h \ diff --git a/tests/helpers/test_kill_pidfdino.c b/tests/helpers/test_kill_pidfdino.c new file mode 100644 index 000000000..464373412 --- /dev/null +++ b/tests/helpers/test_kill_pidfdino.c @@ -0,0 +1,38 @@ +/* + * SPDX-License-Identifier: GPL-2.0-or-later + * + * test_kill_pidfdino - return a pidfd inode for a process using its pid + * + * Written by Christian Goeschel Ndjomouo [2025] + */ +#include +#include +#include +#include + +#include "c.h" +#include "exitcodes.h" +#include "strutils.h" +#include "pidfd-utils.h" + +int main(int argc, char **argv) +{ + int pfd, rc = 0; + pid_t pid; + struct stat f; + + if (argc != 2) + err(EXIT_FAILURE, "usage: %s PID", *argv); + + pid = strtopid_or_err(argv[1], "invalid pid"); + pfd = pidfd_open(pid, 0); + if (pfd < 0) + err_nosys(EXIT_FAILURE, "pidfd_open() failed %d", pid); + + rc = fstat(pfd, &f); + if (rc != 0) + err(EXIT_FAILURE, "fstat() failed: %d", pfd); + + printf("%"PRIu64"\n", f.st_ino); + return EXIT_SUCCESS; +} \ No newline at end of file diff --git a/tests/ts/kill/pidfdino b/tests/ts/kill/pidfdino new file mode 100755 index 000000000..047fcc42e --- /dev/null +++ b/tests/ts/kill/pidfdino @@ -0,0 +1,81 @@ +#!/bin/bash + +# This file is part of util-linux. +# +# This file is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This file is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# Copyright (C) 2025 Christian Goeschel Ndjomouo + +TS_TOPDIR="${0%/*}/../.." +TS_DESC="pidfdino" + +. "$TS_TOPDIR/functions.sh" +ts_init "$*" + +# make sure we do not use shell built-in command +if [ "$TS_USE_SYSTEM_COMMANDS" == "yes" ]; then + TS_CMD_KILL="$(which kill)" +fi + +# The pidfs was only introduced in kernel version 6.9, +# which means we cannot get a valid pidfd from earlier versions, +# so we can confidently anticipate the test failure. +if ts_kernel_ver_lt 6 9; then + TS_KNOWN_FAIL="yes" +fi + +ts_check_test_command "$TS_CMD_KILL" +ts_check_test_command "$TS_HELPER_SIGRECEIVE" +ts_check_test_command "$TS_HELPER_KILL_PIDFDINO" + +. "$TS_SELF/kill_functions.sh" + +all_ok=true + +HELPER_SYMLINK="$(mktemp "${TS_OUTDIR}/opXXXXXXXXXXXXX")" +ln -sf "$TS_HELPER_SIGRECEIVE" "$HELPER_SYMLINK" + +try_option() +{ + "$HELPER_SYMLINK" >> $TS_OUTPUT 2>> $TS_ERRLOG & + TEST_PID=$! + TEST_PIDFD_INO=$( "$TS_HELPER_KILL_PIDFDINO" ${TEST_PID} ) + ts_skip_exitcode_not_supported + + check_test_sigreceive "${TEST_PID}" + [ $? -eq 1 ] || echo "${HELPER_SYMLINK##*/} helper did not start" >> "$TS_OUTPUT" + + "$TS_CMD_KILL" "$@" "${TEST_PID}:${TEST_PIDFD_INO}" >> $TS_OUTPUT 2>> $TS_ERRLOG + if [ $? -ne 0 ]; then + echo "kill $* did not work" >> "$TS_OUTPUT" + all_ok=false + fi + wait $TEST_PID + if [ $? -ne 1 ]; then + echo "wait $TEST_PID for $* did not work" >> "$TS_OUTPUT" + all_ok=false + fi +} + +try_option -s 1 +try_option --signal 1 +try_option --signal HUP +try_option --signal SIGHUP +try_option -1 +try_option -HUP +try_option -SIGHUP + +if $all_ok; then + echo 'all ok' >> "$TS_OUTPUT" +fi +rm -f "$HELPER_SYMLINK" + +ts_finalize \ No newline at end of file