From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from matoro.tk (unknown [IPv6:2600:1700:4b10:9d80::2]) by sourceware.org (Postfix) with ESMTPS id 8050F3858C30 for ; Tue, 14 Feb 2023 19:38:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8050F3858C30 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=matoro.tk Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=matoro.tk DKIM-Signature: a=rsa-sha256; bh=DGLyMn0Bq91pwSpOn+vt/Spwx5nGhKBdVglezOa4TDM=; c=relaxed/relaxed; d=matoro.tk; h=Subject:Subject:Sender:To:To:Cc:Cc:From:From:Date:Date:MIME-Version:MIME-Version:Content-Type:Content-Type:Content-Transfer-Encoding:Content-Transfer-Encoding:Reply-To:In-Reply-To:In-Reply-To:Message-Id:Message-Id:References:References:Autocrypt:Openpgp; i=@matoro.tk; s=20221223; t=1676403500; v=1; x=1676835500; b=smzAhG7smCcxyiWOfZSfZSn5gpAelK6SFZ6rNadzmhy5mF1X/ip4MVfC1GA6AydTtVgL0u0t yRcUSCJ4P1fp2VGlErxDOox1OaWTsfMGyDf6eWvLLPYw5hpZwmIswMi61yo6ZSuqq94TO1hhf7h hEXeBwfY7ODkJNsesikkQlqdi1r6fDIwfqygcdkEKVf/ohT9qm38mS4R+P19eUxoq1KQZTEI1Kw SrqCwz71IWOmXh4rPowE/8xrZYQU15zcZ/5q/KzMiu7A3MC3OFyR0p+m1ACQDioylMD3P7K0xVg q1cUqBV5YBsxim/c/+MPvc89bon+N0iCatFIkrItIHr8Xr81j5FnScoAUXR00pumO54uAA9TXSy 3OPYrrx/YF0TXUHXH9i4MAcyV1r9JCBWVwkFi9c/RlypwWVA4wPXCuDkyGnVgSdx0SnOq4ScvsD 3VfXywRg5oWXjMehKGn6N+5dlKYXlDspjV2DYang9Hc97viOSMcwOav/ccwqx71tsN48IZHV+W3 nBLxdDTKXgYEBr9CO+PuMVyq1hCxOR/aeLcvvnSQ3o0G1bgD3jKDlsw7gAwov+mAAK/chn+lflG zTpoJdtHabXrM7X90KoMX1o3jKCaWQLYP3i7GFEQnuJmpQnNqAa3zj5B6bdMB3RFxuE6nodlYax cr15MknnoFY= Received: by matoro.tk (envelope-sender ) with ESMTPS id 34ddfd52; Tue, 14 Feb 2023 14:38:20 -0500 MIME-Version: 1.0 Date: Tue, 14 Feb 2023 14:38:20 -0500 From: matoro To: aurelien@aurel32.net, libc-alpha@sourceware.org Cc: mattst88@gmail.com, carlos@redhat.com Subject: Re: [PATCH] alpha: correct handling of negative *rlimit() args besides -1 In-Reply-To: References: <20221008024522.523048-1-mattst88@gmail.com> Message-ID: <10925e7b87d9edef1229db7beb0761b0@matoro.tk> X-Sender: matoro_mailinglist_glibc@matoro.tk Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_NUMSUBJECT,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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?