Patchwork [RFC,v2,09/20] sysdeps/getrlimit: Use prlimit64 if avaliable

login
register
mail settings
Submitter Alistair Francis
Date June 25, 2019, 12:09 a.m.
Message ID <895e17d7b3fe3d689f8803a5541c75e7fdfcbd59.1561421042.git.alistair.francis@wdc.com>
Download mbox | patch
Permalink /patch/33378/
State New
Headers show

Comments

Alistair Francis - June 25, 2019, 12:09 a.m.
If the prlimit64 syscall is avaliable let's use that instead of
ugetrlimit as it isn't always avaliable (they aren't avaliable
on RV32).

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 ChangeLog                           | 1 +
 sysdeps/unix/sysv/linux/getrlimit.c | 9 +++++++++
 2 files changed, 10 insertions(+)
Florian Weimer - June 25, 2019, 11:11 a.m.
* Alistair Francis:

> If the prlimit64 syscall is avaliable let's use that instead of
> ugetrlimit as it isn't always avaliable (they aren't avaliable
> on RV32).

> diff --git a/sysdeps/unix/sysv/linux/getrlimit.c b/sysdeps/unix/sysv/linux/getrlimit.c
> index 10c0176619..741b065b25 100644
> --- a/sysdeps/unix/sysv/linux/getrlimit.c
> +++ b/sysdeps/unix/sysv/linux/getrlimit.c
> @@ -35,7 +35,16 @@
>  int
>  __new_getrlimit (enum __rlimit_resource resource, struct rlimit *rlim)
>  {
> +#ifdef __ASSUME_RLIM64_SYSCALLS
> +  return INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
> +#else
> +# ifdef __NR_prlimit64
> +  long int ret = INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
> +  if (ret == 0 || errno != ENOSYS)
> +    return ret;
> +# endif
>    return INLINE_SYSCALL_CALL (ugetrlimit, resource, rlim);
> +#endif

I think that's not correct because with this change, defining
__ASSUME_RLIM64_SYSCALLS changes the struct expected by getrlimit.
I believe the expectation is that such definitions only alter the set of
system calls glibc assumes to be present.  They should not affect the
external glibc ABI exposed on its function call interface.

Thanks,
Florian
Alistair Francis - June 25, 2019, 8:45 p.m.
On Tue, Jun 25, 2019 at 4:11 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Alistair Francis:
>
> > If the prlimit64 syscall is avaliable let's use that instead of
> > ugetrlimit as it isn't always avaliable (they aren't avaliable
> > on RV32).
>
> > diff --git a/sysdeps/unix/sysv/linux/getrlimit.c b/sysdeps/unix/sysv/linux/getrlimit.c
> > index 10c0176619..741b065b25 100644
> > --- a/sysdeps/unix/sysv/linux/getrlimit.c
> > +++ b/sysdeps/unix/sysv/linux/getrlimit.c
> > @@ -35,7 +35,16 @@
> >  int
> >  __new_getrlimit (enum __rlimit_resource resource, struct rlimit *rlim)
> >  {
> > +#ifdef __ASSUME_RLIM64_SYSCALLS
> > +  return INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
> > +#else
> > +# ifdef __NR_prlimit64
> > +  long int ret = INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
> > +  if (ret == 0 || errno != ENOSYS)
> > +    return ret;
> > +# endif
> >    return INLINE_SYSCALL_CALL (ugetrlimit, resource, rlim);
> > +#endif
>
> I think that's not correct because with this change, defining
> __ASSUME_RLIM64_SYSCALLS changes the struct expected by getrlimit.

I'm not clear what you mean here, why is the struct expected by
getrlimit different?

> I believe the expectation is that such definitions only alter the set of
> system calls glibc assumes to be present.  They should not affect the
> external glibc ABI exposed on its function call interface.

I agree, I don't see how this changes the function call interface though.

Alistair

>
> Thanks,
> Florian
Florian Weimer - June 25, 2019, 9:10 p.m.
* Alistair Francis:

> On Tue, Jun 25, 2019 at 4:11 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Alistair Francis:
>>
>> > If the prlimit64 syscall is avaliable let's use that instead of
>> > ugetrlimit as it isn't always avaliable (they aren't avaliable
>> > on RV32).
>>
>> > diff --git a/sysdeps/unix/sysv/linux/getrlimit.c b/sysdeps/unix/sysv/linux/getrlimit.c
>> > index 10c0176619..741b065b25 100644
>> > --- a/sysdeps/unix/sysv/linux/getrlimit.c
>> > +++ b/sysdeps/unix/sysv/linux/getrlimit.c
>> > @@ -35,7 +35,16 @@
>> >  int
>> >  __new_getrlimit (enum __rlimit_resource resource, struct rlimit *rlim)
>> >  {
>> > +#ifdef __ASSUME_RLIM64_SYSCALLS
>> > +  return INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
>> > +#else
>> > +# ifdef __NR_prlimit64
>> > +  long int ret = INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
>> > +  if (ret == 0 || errno != ENOSYS)
>> > +    return ret;
>> > +# endif
>> >    return INLINE_SYSCALL_CALL (ugetrlimit, resource, rlim);
>> > +#endif
>>
>> I think that's not correct because with this change, defining
>> __ASSUME_RLIM64_SYSCALLS changes the struct expected by getrlimit.
>
> I'm not clear what you mean here, why is the struct expected by
> getrlimit different?

On current 32-bit architectures, ugetrlimit expects a 32-bit rlim_t
type.  But prlimit64 assumes a 64-bit rlim_t type.  Maybe this goes
wrong in either case, just by the existence of the prlimit64 system call
number, whether or not __ASSUME_RLIM64_SYSCALLS is defined.

But maybe I'm missing something?

Thanks,
Florian
Alistair Francis - June 25, 2019, 11:38 p.m.
On Tue, Jun 25, 2019 at 2:10 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Alistair Francis:
>
> > On Tue, Jun 25, 2019 at 4:11 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * Alistair Francis:
> >>
> >> > If the prlimit64 syscall is avaliable let's use that instead of
> >> > ugetrlimit as it isn't always avaliable (they aren't avaliable
> >> > on RV32).
> >>
> >> > diff --git a/sysdeps/unix/sysv/linux/getrlimit.c b/sysdeps/unix/sysv/linux/getrlimit.c
> >> > index 10c0176619..741b065b25 100644
> >> > --- a/sysdeps/unix/sysv/linux/getrlimit.c
> >> > +++ b/sysdeps/unix/sysv/linux/getrlimit.c
> >> > @@ -35,7 +35,16 @@
> >> >  int
> >> >  __new_getrlimit (enum __rlimit_resource resource, struct rlimit *rlim)
> >> >  {
> >> > +#ifdef __ASSUME_RLIM64_SYSCALLS
> >> > +  return INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
> >> > +#else
> >> > +# ifdef __NR_prlimit64
> >> > +  long int ret = INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
> >> > +  if (ret == 0 || errno != ENOSYS)
> >> > +    return ret;
> >> > +# endif
> >> >    return INLINE_SYSCALL_CALL (ugetrlimit, resource, rlim);
> >> > +#endif
> >>
> >> I think that's not correct because with this change, defining
> >> __ASSUME_RLIM64_SYSCALLS changes the struct expected by getrlimit.
> >
> > I'm not clear what you mean here, why is the struct expected by
> > getrlimit different?
>
> On current 32-bit architectures, ugetrlimit expects a 32-bit rlim_t
> type.  But prlimit64 assumes a 64-bit rlim_t type.  Maybe this goes
> wrong in either case, just by the existence of the prlimit64 system call
> number, whether or not __ASSUME_RLIM64_SYSCALLS is defined.

Ah, I see what you mean now. Yes they do end up being different rlimit structs.

I also just realised that I don't need this patch, as
__RLIM_T_MATCHES_RLIM64_T is now defined, I'm going to drop this
patch.


Alistair

>
> But maybe I'm missing something?
>
> Thanks,
> Florian

Patch

diff --git a/ChangeLog b/ChangeLog
index 9ed9bea8b1..1f1070ebc3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -8,6 +8,7 @@ 
 	* sysdeps/unix/sysv/linux/wait.c: Use __NR_waitid if avaliable.
 	* sysdeps/unix/sysv/linux/waitpid.c: Likewise.
 	* sysdeps/unix/sysv/linux/waitpid_nocancel.c: Likewise.
+	* sysdeps/unix/sysv/linux/getrlimit.c: Use __NR_prlimit64 if avaliable
 
 2019-06-20  Dmitry V. Levin  <ldv@altlinux.org>
 	    Florian Weimer  <fweimer@redhat.com>
diff --git a/sysdeps/unix/sysv/linux/getrlimit.c b/sysdeps/unix/sysv/linux/getrlimit.c
index 10c0176619..741b065b25 100644
--- a/sysdeps/unix/sysv/linux/getrlimit.c
+++ b/sysdeps/unix/sysv/linux/getrlimit.c
@@ -35,7 +35,16 @@ 
 int
 __new_getrlimit (enum __rlimit_resource resource, struct rlimit *rlim)
 {
+#ifdef __ASSUME_RLIM64_SYSCALLS
+  return INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
+#else
+# ifdef __NR_prlimit64
+  long int ret = INLINE_SYSCALL_CALL (prlimit64, 0, resource, rlim, NULL);
+  if (ret == 0 || errno != ENOSYS)
+    return ret;
+# endif
   return INLINE_SYSCALL_CALL (ugetrlimit, resource, rlim);
+#endif
 }
 weak_alias (__new_getrlimit, __getrlimit)
 hidden_weak (__getrlimit)