Message ID | 20220324110907.169470-1-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | [v2] linux: Fix __closefrom_fallback iterates until max int (BZ#28993) | expand |
On Mär 24 2022, Adhemerval Zanella via Libc-alpha wrote: > if (dirfd == -1) > { > /* The closefrom should work even when process can't open new files. */ > - if (errno == ENOENT || !dirfd_fallback) > - goto err; > + if (errno != EMFILE || !dirfd_fallback) There is also ENFILE.
* Adhemerval Zanella via Libc-alpha: > The __closefrom_fallback tries to get a available file descriptor > if the initial open ("/proc/self/fd/", ...) fails. It assumes the > failure would be only if procfs is not mount (ENOENT), however if > the the proc file is not accessible (due some other kernel filtering > such apparmor) it will iterate over a potentially large file set > issuing close calls. > > It should only try the close fallback if open returns EMFILE. I disagree. If there are performance issues due to the current code, people should fix their container hosts.
On 24/03/2022 08:25, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> The __closefrom_fallback tries to get a available file descriptor >> if the initial open ("/proc/self/fd/", ...) fails. It assumes the >> failure would be only if procfs is not mount (ENOENT), however if >> the the proc file is not accessible (due some other kernel filtering >> such apparmor) it will iterate over a potentially large file set >> issuing close calls. >> >> It should only try the close fallback if open returns EMFILE. > > I disagree. If there are performance issues due to the current code, > people should fix their container hosts. For closefrom it will trigger a __foritfy_fail and for posix_spawn_file_actions_addclosefrom_np it will make posix_spawn fail. I think on both cases it is better to fail early.
diff --git a/sysdeps/unix/sysv/linux/closefrom_fallback.c b/sysdeps/unix/sysv/linux/closefrom_fallback.c index 60101aa3ba..da1f29752d 100644 --- a/sysdeps/unix/sysv/linux/closefrom_fallback.c +++ b/sysdeps/unix/sysv/linux/closefrom_fallback.c @@ -30,15 +30,13 @@ _Bool __closefrom_fallback (int from, _Bool dirfd_fallback) { - bool ret = false; - int dirfd = __open_nocancel (FD_TO_FILENAME_PREFIX, O_RDONLY | O_DIRECTORY, 0); if (dirfd == -1) { /* The closefrom should work even when process can't open new files. */ - if (errno == ENOENT || !dirfd_fallback) - goto err; + if (errno != EMFILE || !dirfd_fallback) + return false; for (int i = from; i < INT_MAX; i++) { @@ -54,6 +52,7 @@ __closefrom_fallback (int from, _Bool dirfd_fallback) } char buffer[1024]; + bool ret = false; while (true) { ssize_t ret = __getdents64 (dirfd, buffer, sizeof (buffer));