Hi Pádraig, Pádraig Brady <[email protected]> writes:
> On 09/11/2025 11:00, Pádraig Brady wrote: >> On 09/11/2025 07:17, Collin Funk wrote: >>> Collin Funk <[email protected]> writes: >>> >>>> That test program assumes that nice() does not clamp the niceness to the >>>> supported range as it is supposed to [1]. This is the case on GNU/Hurd, >>>> so I will submit a bug report for that later. >>>> >>>> The output is the same on a normal and privileged user: >>>> >>>> $ gcc main.c ./a.out >>>> starting nice: 0 >>>> minimum nice: 0 >>>> maximum nice: 38 >>>> minimum errno: EPERM >>>> maximum errno: ESRCH >>>> >>>> This patch clamps it to the supported range and adds some tests. Here is >>>> the corrected behavior: >>>> >>>> $ ./src/nice -n +100 ./src/nice >>>> 38 >>> >>> Actually my original patch did not work if 'nice' was invoked with a >>> niceness greater than zero. This v2 patch fixes that and adds more >>> tests. >> That's restricted to Hurd so looks safe to push. > > I should add that this should be reported to Hurd also, > so it's a layering violation to do the workaround in user space, > as we can't do it robustly because of races. Apologies for the delay. Was busy with work, so couldn't get to it until now. Samuel Thibault, who maintains Hurd, changed the maximum niceness to 39 instead of 38 [1]. That value is equal to (2 * NZERO - 1) which brings things more into alignment with other systems. He also made setpriority(2), and therefore nice(2), clamp the niceness to the supported range [2]. That change will be in glibc 2.43. Therefore, we can avoid the race with glibc 2.43 or newer. I also adjusted the tests a bit to allow for the differing maximum niceness. Otherwise the future Hurd release would fail. Pushed the attached patch with those changes. Collin [1] https://git.savannah.gnu.org/cgit/hurd/gnumach.git/commit/?id=4e19e045e5c9e2207c6dd2965180d55d1a8c73e9 [2] https://inbox.sourceware.org/libc-alpha/[email protected]/T/#m99c567494eec00dbac9298c0c38e16f7053996e6
>From fc93a44bbe9020cb1a5b03499f5fb2f7512d1551 Mon Sep 17 00:00:00 2001 Message-ID: <fc93a44bbe9020cb1a5b03499f5fb2f7512d1551.1762742977.git.collin.fu...@gmail.com> From: Collin Funk <[email protected]> Date: Sat, 8 Nov 2025 20:30:08 -0800 Subject: [PATCH v3] nice: clamp the niceness correctly on GNU/Hurd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * NEWS: Mention the bug fix. * src/nice.c (MIN_ADJUSTMENT): Set to 0 on the Hurd with glibc ≤ 2.42. (MAX_ADJUSTMENT): Set to (2 * NZERO - 2) on the Hurd with glibc ≤ 2.42. (main): Clamp the niceness to be greater or equal to MIN_ADJUSTMENT and less than or equal to MAX_ADJUSTMENT. * tests/nice/nice.sh: Add some tests for the Hurd's ranges. --- NEWS | 4 ++++ src/nice.c | 34 +++++++++++++++++++++++++++++++ tests/nice/nice.sh | 50 ++++++++++++++++++++++++++++++++++++---------- 3 files changed, 78 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 7e7dc6376..46df91b62 100644 --- a/NEWS +++ b/NEWS @@ -26,6 +26,10 @@ GNU coreutils NEWS -*- outline -*- will no longer always set a __CF_USER_TEXT_ENCODING environment variable. [bug introduced in coreutils-9.8] + 'nice' now limits the adjusted niceness value to its supported range on + GNU/Hurd. + [This bug was present in "the beginning".] + 'numfmt' no longer reads out-of-bounds memory with trailing blanks in input. [bug introduced with numfmt in coreutils-8.21] diff --git a/src/nice.c b/src/nice.c index 1c3f2a038..1187607c9 100644 --- a/src/nice.c +++ b/src/nice.c @@ -167,12 +167,46 @@ main (int argc, char **argv) /* If the requested adjustment is outside the valid range, silently bring it to just within range; this mimics what "setpriority" and "nice" do. */ +#if (defined __gnu_hurd__ \ + && (__GLIBC__ < 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 43))) + /* GNU/Hurd's nice(2) only supported 0 to (2 * NZERO - 2) niceness + until glibc 2.43. */ + enum { MIN_ADJUSTMENT = 0, MAX_ADJUSTMENT = 2 * NZERO - 2 }; +#else enum { MIN_ADJUSTMENT = 1 - 2 * NZERO, MAX_ADJUSTMENT = 2 * NZERO - 1 }; +#endif long int tmp; if (LONGINT_OVERFLOW < xstrtol (adjustment_given, nullptr, 10, &tmp, "")) error (EXIT_CANCELED, 0, _("invalid adjustment %s"), quote (adjustment_given)); +#if (defined __gnu_hurd__ \ + && (__GLIBC__ < 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 43))) + /* GNU/Hurd's nice(2) also did not clamp the new niceness into the + supported range until glibc 2.43. + See <https://sourceware.org/PR33614>. */ + errno = 0; + current_niceness = GET_NICENESS (); + if (current_niceness == -1 && errno != 0) + error (EXIT_CANCELED, errno, _("cannot get niceness")); + if (tmp < 0) + { + int sum; + if (ckd_add (&sum, current_niceness, tmp) || sum < MIN_ADJUSTMENT) + adjustment = MIN_ADJUSTMENT - current_niceness; + else + adjustment = tmp; + } + else + { + int sum; + if (ckd_add (&sum, current_niceness, tmp) || MAX_ADJUSTMENT < sum) + adjustment = MAX_ADJUSTMENT - current_niceness; + else + adjustment = tmp; + } +#else adjustment = MAX (MIN_ADJUSTMENT, MIN (tmp, MAX_ADJUSTMENT)); +#endif } if (i == argc) diff --git a/tests/nice/nice.sh b/tests/nice/nice.sh index 7e274ca0d..70be3eb98 100755 --- a/tests/nice/nice.sh +++ b/tests/nice/nice.sh @@ -18,6 +18,7 @@ . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src print_ver_ nice +getlimits_ tests=' 0 empty 10 @@ -71,16 +72,45 @@ done # Test negative niceness - command must be run whether or not change happens. if test x$(nice -n -1 nice 2> /dev/null) = x0 ; then - # unprivileged user - warn about failure to change - nice -n -1 true 2> err || fail=1 - compare /dev/null err && fail=1 - mv err exp || framework_failure_ - nice --1 true 2> err || fail=1 - compare exp err || fail=1 - # Failure to write advisory message is fatal. Buggy through coreutils 8.0. - if test -w /dev/full && test -c /dev/full; then - returns_ 125 nice -n -1 nice > out 2> /dev/full || fail=1 - compare /dev/null out || fail=1 + # GNU/Hurd does not allow negative niceness even if we are a privileged user. + if test "$(uname)" = GNU; then + max_nice=$(nice -n "$INT_MAX" nice) || framework_failure_ + # Check that the lowest niceness is 0. + nice -n -1 nice > out || fail=1 + echo '0' > exp || framework_failure_ + compare exp out || fail=1 + # Exceeding the max niceness would lead to the program not being executed on + # GNU/Hurd with coreutils 9.8 and earlier. + nice -n $(("$max_nice" + 1)) nice > out || fail=1 + echo "$max_nice" > exp || framework_failure_ + compare exp out || fail=1 + # GNU/Hurd's nice(2) with glibc 2.42 and earlier does not clamp the + # niceness to the supported range. Check that we workaround the bug. + # See <https://sourceware.org/PR33614>. + nice -n 1 nice -n $(("$max_nice" - 2)) nice > out|| fail=1 + echo $(("$max_nice" - 1)) > exp || framework_failure_ + compare exp out || fail=1 + nice -n 1 nice -n $(("$max_nice" + 1)) nice > out || fail=1 + echo "$max_nice" > exp || framework_failure_ + compare exp out || fail=1 + nice -n 2 nice -n -1 nice > out || fail=1 + echo '1' > exp || framework_failure_ + compare exp out || fail=1 + nice -n 2 nice -n -3 nice > out || fail=1 + echo '0' > exp || framework_failure_ + compare exp out || fail=1 + else + # unprivileged user - warn about failure to change + nice -n -1 true 2> err || fail=1 + compare /dev/null err && fail=1 + mv err exp || framework_failure_ + nice --1 true 2> err || fail=1 + compare exp err || fail=1 + # Failure to write advisory message is fatal. Buggy through coreutils 8.0. + if test "$(uname)" != GNU && test -w /dev/full && test -c /dev/full; then + returns_ 125 nice -n -1 nice > out 2> /dev/full || fail=1 + compare /dev/null out || fail=1 + fi fi else # superuser - change succeeds -- 2.51.1
