This is an archive of the discontinued LLVM Phabricator instance.

[libc] Fix setrlimit/getrlimit on 32-bit systems
ClosedPublic

Authored by mikhail.ramalho on Aug 29 2023, 8:16 AM.

Details

Summary

libc uses SYS_prlimit64 (which takes a struct rlimit64) to implement
setrlimt and getrlimit (which take a struct rlimit). In 64-bit bits
systems this is not an issue since the members of struct rlimit64 and
struct rlimit are 64 bits long, however, in 32-bit systems the members
of struct rlimit are only 32 bits long, causing wrong values being
passed to SYS_prlimit64.

This patch adds new struct rlimit64 (and rlim64_t) to fix this issue.
We now create a local struct rlimit64 variable in setrlimt/getrlimit,
pass it to SYS_prlimit64 (and rewrite the original rlimit struct in
getrlimit if the syscall succeeds).

Diff Detail

Event Timeline

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 29 2023, 8:16 AM
mikhail.ramalho requested review of this revision.Aug 29 2023, 8:16 AM
sivachandra added inline comments.Sep 5 2023, 10:24 AM
libc/include/llvm-libc-types/rlim64_t.h
12 ↗(On Diff #554357)

Can just define rlim_t this way? Would there be any problems in the overlay build?

My _guess_ is that it would work normally.

I tried to follow glibc here: it defines rlim_t as unsigned long int
for all systems, while also defining rlim64_t, which is always 64 bits
long.

OTOH, musl defines rlim_t as unsigned long long in both x64 and arm32.
It also defines rlim64_t as unsigned long long.

Em ter., 5 de set. de 2023 às 14:24, Siva Chandra via Phabricator <
reviews@reviews.llvm.org> escreveu:

sivachandra added inline comments.

Comment at: libc/include/llvm-libc-types/rlim64_t.h:12
+
+typedef UINT64_TYPE rlim64_t;

+

Can just define rlim_t this way? Would there be any problems in the
overlay build?

Repository:

rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION

https://reviews.llvm.org/D159104/new/

https://reviews.llvm.org/D159104

I think we can define rlim_t to be 64-bit type always and add an internal type for use with the syscall:

struct RLimits64 {
  uint64_t rlim_cur;
  uint64_t rlim_max;
};

.. getrlimit(... *limits) {
  RLimits64 limits64;
  syscall_impl(SYS_prlimit64, ..., &limits64);
  limits->rlim_cur = static_cast<decltype(limits->rlim_cur)>(limits64.rlim_cur);
  ...
}

This way, we ensure that the implementation is overlay mode safe as well.

I forgot to mention in the previous message that glibc and musl also
provide rlimit64, so changing rlim_t to always be 64 bit long makes
rlimit64 the same as limit, which simplifies the code.

btw, musl simply does:

#define rlimit64 rlimit
#define rlim64_t rlim_t

while glibc actually defines the types.

Which way do you prefer, to use defines in the headers or to add new
headers (like this PR does)? I'm assuming here that we want to define both
rlim64_t and rlimit64 types.

Em ter., 5 de set. de 2023 às 16:45, Siva Chandra via Phabricator <
reviews@reviews.llvm.org> escreveu:

sivachandra added a comment.

I think we can define rlim_t to be 64-bit type always and add an
internal type for use with the syscall:

struct RLimits64 {
  uint64_t rlim_cur;
  uint64_t rlim_max;
};

.. getrlimit(... *limits) {
  RLimits64 limits64;
  syscall_impl(SYS_prlimit64, ..., &limits64);
  limits->rlim_cur =

static_cast<decltype(limits->rlim_cur)>(limits64.rlim_cur);

  ...
}

This way, we ensure that the implementation is overlay mode safe as well.

Repository:

rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION

https://reviews.llvm.org/D159104/new/

https://reviews.llvm.org/D159104

As far as I understand, the 64-bit flavors are really GNU extensions. We can add them if and when required. Until then, lets stick to just the POSIX types and functions.

  • removed rlim64_t and rlimit64
  • define rlim_t as UINT64_TYPE
sivachandra accepted this revision.Sep 7 2023, 8:48 AM

OK after updating the commit message.

This revision is now accepted and ready to land.Sep 7 2023, 8:48 AM