Re: [PATCH] alpha: correct handling of negative *rlimit() args besides -1

[matoro <matoro_mailinglist_glibc@matoro.tk>]

Hi Aurelien, I came up with the idea for this originally.  Matt noticed 
that it had stalled and asked me to check back in.

> On 2022-10-07 22:45, Matt Turner via Libc-alpha wrote:
> > The generic version of RLIM_INFINITY in Linux is equal to (rlim_t)-1,
> > which is equal to ULLONG_MAX.  On alpha however it is instead defined as
> > 0x7ffffffffffffffful.  This was special-cased in 0d0bc78 [BZ #22648] but
> > it specifically used an equality check.
> 
> I am not sure this commit is giving the full picture, commits around
> should also be checked to understand it.

Can you elaborate here?  This was my understanding based on what I read, 
but you are the original author, so your perspective will surely be more 
complete than mine.

> > There is a cpython test case test_prlimit_refcount which calls
> > setrlimit() with { -2, -2 } as arguments rather than the usual -1, it
> > therefore fails the equality test and is treated as a large arbitrary
> > positive value past the maximum of RLIM_INFINITY and fails with EPERM.
> > This patch changes the behavior of the *rlimit() calls to treat all
> > integers between 0x7ffffffffffffffful and (rlim_t)-1 as (rlim_t)-1,
> > i.e., RLIM_INFINITY.
> 
> Basically on alpha, the glibc API is now identical to the prlimit64 
> API,
> which means there is a dead zone with invalid values from
> 0x8000000000000000ul to 0xfffffffffffffffeul. The kernel returns EPERM
> for values in this range.
> 
> You suggestion is to consider values is this zone as infinity. I have
> mixed feeling about that. From the setrlimit() side it looks like the
> correct thing to do. But this breaks the assumption that calling
> getrlimit() after a successful setrlimit() call will return the same
> value.

Is this behavior specified one way or the other?  Alternatively, is the 
Python unit test making an assumption that is not guaranteed (that 
calling setrlimit() with a negative value behaves the same way as 
calling it specifically with RLIM_INFINITY)?  If this is Python's 
mistake, that can be corrected there.  The test in question:  
https://github.com/python/cpython/blob/main/Lib/test/test_resource.py#L163-L175

> > diff --git a/sysdeps/unix/sysv/linux/alpha/getrlimit64.c b/sysdeps/unix/sysv/linux/alpha/getrlimit64.c
> > index c195f5b55c..40f3e6bdff 100644
> > --- a/sysdeps/unix/sysv/linux/alpha/getrlimit64.c
> > +++ b/sysdeps/unix/sysv/linux/alpha/getrlimit64.c
> > @@ -38,11 +38,11 @@ __old_getrlimit64 (enum __rlimit_resource resource,
> >    if (__getrlimit64 (resource, &krlimits) < 0)
> >      return -1;
> >
> > -  if (krlimits.rlim_cur == RLIM64_INFINITY)
> > +  if (krlimits.rlim_cur >= OLD_RLIM64_INFINITY)
> >      rlimits->rlim_cur = OLD_RLIM64_INFINITY;
> >    else
> >      rlimits->rlim_cur = krlimits.rlim_cur;
> > -  if (krlimits.rlim_max == RLIM64_INFINITY)
> > +  if (krlimits.rlim_max >= OLD_RLIM64_INFINITY)
> >      rlimits->rlim_max = OLD_RLIM64_INFINITY;
> >    else
> >      rlimits->rlim_max = krlimits.rlim_max;
> 
> That said, I do not understand the change there. It is done on the
> *compat* symbol which still uses the old glibc API definition. There we
> want to keep doing the exact reverse operations as in the
> rlim_to_rlim64() kernel function.
> 
> > diff --git a/sysdeps/unix/sysv/linux/alpha/setrlimit64.c b/sysdeps/unix/sysv/linux/alpha/setrlimit64.c
> > index 421616ed20..4e88540a48 100644
> > --- a/sysdeps/unix/sysv/linux/alpha/setrlimit64.c
> > +++ b/sysdeps/unix/sysv/linux/alpha/setrlimit64.c
> > @@ -35,11 +35,11 @@ __old_setrlimit64 (enum __rlimit_resource resource,
> >  {
> >    struct rlimit64 krlimits;
> >
> > -  if (rlimits->rlim_cur == OLD_RLIM64_INFINITY)
> > +  if (rlimits->rlim_cur >= OLD_RLIM64_INFINITY)
> >      krlimits.rlim_cur = RLIM64_INFINITY;
> >    else
> >      krlimits.rlim_cur = rlimits->rlim_cur;
> > -  if (rlimits->rlim_max == OLD_RLIM64_INFINITY)
> > +  if (rlimits->rlim_max >= OLD_RLIM64_INFINITY)
> >      krlimits.rlim_max = RLIM64_INFINITY;
> >    else
> >      krlimits.rlim_max = rlimits->rlim_max;
> 
> Ditto here we want to do the reverse operations as the rlim64_to_rlim()
> kernel function.

I don't quite understand where else the change would go.  We don't want 
to be touching the generic implementations do we?  Or are you saying 
this should actually be going in the kernel and not glibc?