[PATCH] setrlimit/getrlimit: Add parameter check to prevent null pointer access
Adhemerval Zanella
adhemerval.zanella@linaro.org
Wed Dec 30 12:43:31 GMT 2020
On 30/12/2020 08:41, Xiaoming Ni wrote:
> Following sysdeps/mach/hurd/[gs]etrlimit.c.
> Add parameter check to prevent null pointer access in setrlimit().
> Using Macro Functions RETURN_IF_RLIMIT_EINVAL() to Avoid Duplicate Code Copy.
POSIX [1] does not define that EINVAL should be returned for invalid
argument, only for an invalid resource or if limit specified cannot
be lowered.
LTP used to check for a *Linux* specific semantic where it returned
EFAULT for such cases; but it was a wrong assumption since getrlimit
could be reimplemented through another syscall (such as currently
on glibc it calls prlimit).
I think we should mark the argument nonull instead and remove this
check from Hurd (it will need to check againt RLIMIT_NLIMITS
since it access a internal array).
[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/getrlimit.html
> ---
> resource/getrlimit64.c | 2 ++
> resource/setrlimit64.c | 2 ++
> resource/sys/resource.h | 8 ++++++++
> sysdeps/mach/hurd/getrlimit.c | 6 +-----
> sysdeps/mach/hurd/setrlimit.c | 6 +-----
> sysdeps/unix/sysv/linux/alpha/getrlimit64.c | 2 ++
> sysdeps/unix/sysv/linux/alpha/setrlimit64.c | 2 ++
> sysdeps/unix/sysv/linux/getrlimit64.c | 2 ++
> sysdeps/unix/sysv/linux/mips/getrlimit64.c | 2 ++
> sysdeps/unix/sysv/linux/mips/setrlimit64.c | 2 ++
> sysdeps/unix/sysv/linux/setrlimit.c | 2 ++
> 11 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/resource/getrlimit64.c b/resource/getrlimit64.c
> index 41b6fa01d2..84059a5db8 100644
> --- a/resource/getrlimit64.c
> +++ b/resource/getrlimit64.c
> @@ -26,6 +26,8 @@ __getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits)
> {
> struct rlimit rlimits32;
>
> + RETURN_IF_RLIMIT_EINVAL(resource, rlimits);
> +
> if (__getrlimit (resource, &rlimits32) < 0)
> return -1;
>
> diff --git a/resource/setrlimit64.c b/resource/setrlimit64.c
> index 0411e3ea19..687df56cb3 100644
> --- a/resource/setrlimit64.c
> +++ b/resource/setrlimit64.c
> @@ -27,6 +27,8 @@ setrlimit64 (enum __rlimit_resource resource, const struct rlimit64 *rlimits)
> {
> struct rlimit rlimits32;
>
> + RETURN_IF_RLIMIT_EINVAL(resource, rlimits);
> +
> if (rlimits->rlim_cur >= RLIM_INFINITY)
> rlimits32.rlim_cur = RLIM_INFINITY;
> else
> diff --git a/resource/sys/resource.h b/resource/sys/resource.h
> index 4edafb50d5..a98b7c8b4e 100644
> --- a/resource/sys/resource.h
> +++ b/resource/sys/resource.h
> @@ -82,6 +82,14 @@ extern int setrlimit64 (__rlimit_resource_t __resource,
> const struct rlimit64 *__rlimits) __THROW;
> #endif
>
> +#define RETURN_IF_RLIMIT_EINVAL(resource, rlimits) do { \
> + if ((rlimits) == NULL || (unsigned int) (resource) >= RLIMIT_NLIMITS) \
> + { \
> + errno = EINVAL; \
> + return -1; \
> + } \
> +} while(0)
> +
> /* Return resource usage information on process indicated by WHO
> and put it in *USAGE. Returns 0 for success, -1 for failure. */
> extern int getrusage (__rusage_who_t __who, struct rusage *__usage) __THROW;
> diff --git a/sysdeps/mach/hurd/getrlimit.c b/sysdeps/mach/hurd/getrlimit.c
> index 32d37c185d..904a9b7b9b 100644
> --- a/sysdeps/mach/hurd/getrlimit.c
> +++ b/sysdeps/mach/hurd/getrlimit.c
> @@ -27,11 +27,7 @@ __getrlimit (enum __rlimit_resource resource, struct rlimit *rlimits)
> {
> struct rlimit lim;
>
> - if (rlimits == NULL || (unsigned int) resource >= RLIMIT_NLIMITS)
> - {
> - errno = EINVAL;
> - return -1;
> - }
> + RETURN_IF_RLIMIT_EINVAL(resource, rlimits);
>
> HURD_CRITICAL_BEGIN;
> __mutex_lock (&_hurd_rlimit_lock);
> diff --git a/sysdeps/mach/hurd/setrlimit.c b/sysdeps/mach/hurd/setrlimit.c
> index e0f80bbb9c..a1b5615b93 100644
> --- a/sysdeps/mach/hurd/setrlimit.c
> +++ b/sysdeps/mach/hurd/setrlimit.c
> @@ -29,11 +29,7 @@ __setrlimit (enum __rlimit_resource resource, const struct rlimit *rlimits)
> {
> struct rlimit lim;
>
> - if (rlimits == NULL || (unsigned int) resource >= RLIMIT_NLIMITS)
> - {
> - errno = EINVAL;
> - return -1;
> - }
> + RETURN_IF_RLIMIT_EINVAL(resource, rlimits);
>
> lim = *rlimits;
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/getrlimit64.c b/sysdeps/unix/sysv/linux/alpha/getrlimit64.c
> index 96655ff77e..017a7c18d4 100644
> --- a/sysdeps/unix/sysv/linux/alpha/getrlimit64.c
> +++ b/sysdeps/unix/sysv/linux/alpha/getrlimit64.c
> @@ -35,6 +35,8 @@ __old_getrlimit64 (enum __rlimit_resource resource,
> {
> struct rlimit64 krlimits;
>
> + RETURN_IF_RLIMIT_EINVAL(resource, rlimits);
> +
> if (__getrlimit64 (resource, &krlimits) < 0)
> return -1;
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/setrlimit64.c b/sysdeps/unix/sysv/linux/alpha/setrlimit64.c
> index ae77b4127c..8c5ac6c76e 100644
> --- a/sysdeps/unix/sysv/linux/alpha/setrlimit64.c
> +++ b/sysdeps/unix/sysv/linux/alpha/setrlimit64.c
> @@ -35,6 +35,8 @@ __old_setrlimit64 (enum __rlimit_resource resource,
> {
> struct rlimit64 krlimits;
>
> + RETURN_IF_RLIMIT_EINVAL(resource, rlimits);
> +
> if (rlimits->rlim_cur == OLD_RLIM64_INFINITY)
> krlimits.rlim_cur = RLIM64_INFINITY;
> else
> diff --git a/sysdeps/unix/sysv/linux/getrlimit64.c b/sysdeps/unix/sysv/linux/getrlimit64.c
> index e06ffd1a16..e75d06a402 100644
> --- a/sysdeps/unix/sysv/linux/getrlimit64.c
> +++ b/sysdeps/unix/sysv/linux/getrlimit64.c
> @@ -66,6 +66,8 @@ __old_getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits)
> {
> struct rlimit rlimits32;
>
> + RETURN_IF_RLIMIT_EINVAL(resource, rlimits);
> +
> if (__new_getrlimit (resource, &rlimits32) < 0)
> return -1;
>
> diff --git a/sysdeps/unix/sysv/linux/mips/getrlimit64.c b/sysdeps/unix/sysv/linux/mips/getrlimit64.c
> index f42100f085..94008444a9 100644
> --- a/sysdeps/unix/sysv/linux/mips/getrlimit64.c
> +++ b/sysdeps/unix/sysv/linux/mips/getrlimit64.c
> @@ -45,6 +45,8 @@ __old_getrlimit64 (enum __rlimit_resource resource,
> {
> struct rlimit64 krlimits;
>
> + RETURN_IF_RLIMIT_EINVAL(resource, rlimits);
> +
> if (__getrlimit64 (resource, &krlimits) < 0)
> return -1;
>
> diff --git a/sysdeps/unix/sysv/linux/mips/setrlimit64.c b/sysdeps/unix/sysv/linux/mips/setrlimit64.c
> index 36f5d85378..10ae33bf37 100644
> --- a/sysdeps/unix/sysv/linux/mips/setrlimit64.c
> +++ b/sysdeps/unix/sysv/linux/mips/setrlimit64.c
> @@ -44,6 +44,8 @@ __old_setrlimit64 (enum __rlimit_resource resource,
> {
> struct rlimit64 krlimits;
>
> + RETURN_IF_RLIMIT_EINVAL(resource, rlimits);
> +
> if (rlimits->rlim_cur == OLD_RLIM64_INFINITY)
> krlimits.rlim_cur = RLIM64_INFINITY;
> else
> diff --git a/sysdeps/unix/sysv/linux/setrlimit.c b/sysdeps/unix/sysv/linux/setrlimit.c
> index 6648fad5c0..98659c4ced 100644
> --- a/sysdeps/unix/sysv/linux/setrlimit.c
> +++ b/sysdeps/unix/sysv/linux/setrlimit.c
> @@ -35,6 +35,8 @@ __setrlimit (enum __rlimit_resource resource, const struct rlimit *rlim)
> {
> struct rlimit64 rlim64;
>
> + RETURN_IF_RLIMIT_EINVAL(resource, rlim);
> +
> if (rlim->rlim_cur == RLIM_INFINITY)
> rlim64.rlim_cur = RLIM64_INFINITY;
> else
>
More information about the Libc-alpha
mailing list