Hello,
Thanks for this!
dnie...@gmail.com, le lun. 16 juin 2025 23:58:07 +0100, a ecrit:
> diff --git a/hurd/hurdrlimit.c b/hurd/hurdrlimit.c
> index 6cb5045bfe..6b0d8a26a3 100644
> --- a/hurd/hurdrlimit.c
> +++ b/hurd/hurdrlimit.c
> @@ -39,6 +39,11 @@ init_rlimit (void)
>
> for (i = 0; i < RLIM_NLIMITS; ++i)
> {
> +#ifdef HAVE_MACH_VM_GET_SIZE_LIMIT
> + if (i == RLIMIT_AS)
> + __vm_get_size_limit (__mach_task_self (),
> + &_hurd_rlimits[i].rlim_cur, &_hurd_rlimits[i].rlim_max);
> +#endif
> if (_hurd_rlimits[i].rlim_max == 0)
> _hurd_rlimits[i].rlim_max = RLIM_INFINITY;
> if (_hurd_rlimits[i].rlim_cur == 0)
You can as well put this code before the loop, we don't need to check
for i at each iteration, and instead directly put the result into
_hurd_rlimits[RLIMIT_AS].
> diff --git a/sysdeps/mach/configure.ac b/sysdeps/mach/configure.ac
> index 3a6f2443e2..237b8be937 100644
> --- a/sysdeps/mach/configure.ac
> +++ b/sysdeps/mach/configure.ac
> @@ -100,6 +100,10 @@ mach_RPC_CHECK(gnumach.defs, thread_set_name,
> HAVE_MACH_THREAD_SET_NAME)
> mach_RPC_CHECK(gnumach.defs, thread_get_name,
> HAVE_MACH_THREAD_GET_NAME)
> +mach_RPC_CHECK(gnumach.defs, vm_set_size_limit,
> + HAVE_MACH_VM_SET_SIZE_LIMIT)
> +mach_RPC_CHECK(gnumach.defs, vm_get_size_limit,
> + HAVE_MACH_VM_GET_SIZE_LIMIT)
>
> AC_CHECK_HEADER(mach/machine/ndr_def.h, [dnl
> DEFINES="$DEFINES -DNDR_DEF_HEADER='<mach/machine/ndr_def.h>'"], [dnl
> diff --git a/sysdeps/mach/hurd/setrlimit.c b/sysdeps/mach/hurd/setrlimit.c
> index cbc172ee75..ce01efbac8 100644
> --- a/sysdeps/mach/hurd/setrlimit.c
> +++ b/sysdeps/mach/hurd/setrlimit.c
> @@ -28,6 +28,8 @@ int
> __setrlimit (enum __rlimit_resource resource, const struct rlimit *rlimits)
> {
> struct rlimit lim;
> + error_t err;
You need to initialize it to 0 for the resource != RLIMIT_AS case.
> + mach_port_t host = MACH_PORT_NULL;
>
> if (rlimits == NULL || (unsigned int) resource >= RLIMIT_NLIMITS)
> return __hurd_fail (EINVAL);
> @@ -41,13 +43,66 @@ __setrlimit (enum __rlimit_resource resource, const
> struct rlimit *rlimits)
> if (lim.rlim_cur > lim.rlim_max)
> lim.rlim_cur = lim.rlim_max;
>
> +retry:
> HURD_CRITICAL_BEGIN;
> __mutex_lock (&_hurd_rlimit_lock);
> +
> +#ifdef HAVE_MACH_VM_SET_SIZE_LIMIT
> + if (resource == RLIMIT_AS)
> + {
> +
> + if (host == MACH_PORT_NULL)
> + {
> + /* XXX initialize err to 0 for skipping error handling in
> + * non-privileged host port branch.
> + */
> + err = 0;
You can as well put the error handling code inside this if:
> + /* Check whether the privileged host control port is required */
> + if (_hurd_rlimits[resource].rlim_max < lim.rlim_max)
> + err = __get_privileged_ports (&host, NULL);
> + else
> + host = __mach_host_self ();
> +
> + /* If we were interrupted, leave the error unchanged so cleanup
> code can retry */
> + if (err == EINTR)
> + goto fail;
> +
> + /* Handle any error getting the privileged ports by mapping them
> to EPERM */
I'd rather map only the exact kernel error that happens in
non-privileged processes, to translating odd errors into a mere EPERM,
possibly confusing developers trying to debug something.
> + if (err)
> + {
> + err = EPERM;
> + goto fail;
> + }
> + }
> +
> + err = __vm_set_size_limit (host, __mach_task_self (),
> + lim.rlim_cur, lim.rlim_max);
> +
> + if (err != MIG_BAD_ID)
> + {
> + if (err)
> + goto fail;
> + }
> + else /* XXX MIG_BAD_ID returned as kernel support is missing, clear
> error */
You can avoid the XXX tag. That tag would mean that there is still
something to fix here, while there is not, it's just the case that copes
with old kernels.
> + err = 0;
Perhaps making it simpler with
if (err == MIG_BAD_ID)
/* MIG_BAD_ID returned as kernel support is missing, clear error */
err = 0;
if (err)
goto fail;
> + }
> +#endif
> +
> _hurd_rlimits[resource] = lim;
> +
> +fail:
> __mutex_unlock (&_hurd_rlimit_lock);
> HURD_CRITICAL_END;
>
> - return 0;
> + if (err == EINTR)
> + /* Got a signal while inside an RPC of the critical section, retry */
> + goto retry;
> +
> + if (host != MACH_PORT_NULL && host != __mach_host_self ())
> + __mach_port_deallocate (__mach_task_self (), host);
> +
> + return err != 0 ? __hurd_fail (err) : 0;
You do not actually need to test err, __hurd_fail already behaves fine
when it's 0.
Samuel
