linux-kernel - Re: [PATCH v5 1/3] open: add close

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87d06hdozy.fsf@mid.deneb.enyo.de>
Date:   Wed, 03 Jun 2020 01:30:57 +0200
From:   Florian Weimer <fw@...eb.enyo.de>
To:     Christian Brauner <christian.brauner@...ntu.com>
Cc:     torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org,
        Kyle Evans <self@...e-evans.net>,
        Victor Stinner <victor.stinner@...il.com>,
        viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
        linux-api@...r.kernel.org, fweimer@...hat.com, jannh@...gle.com,
        oleg@...hat.com, arnd@...db.de, shuah@...nel.org,
        dhowells@...hat.com, ldv@...linux.org
Subject: Re: [PATCH v5 1/3] open: add close_range()

* Christian Brauner:

> The performance is striking. For good measure, comparing the following
> simple close_all_fds() userspace implementation that is essentially just
> glibc's version in [6]:
>
> static int close_all_fds(void)
> {
>         int dir_fd;
>         DIR *dir;
>         struct dirent *direntp;
>
>         dir = opendir("/proc/self/fd");
>         if (!dir)
>                 return -1;
>         dir_fd = dirfd(dir);
>         while ((direntp = readdir(dir))) {
>                 int fd;
>                 if (strcmp(direntp->d_name, ".") == 0)
>                         continue;
>                 if (strcmp(direntp->d_name, "..") == 0)
>                         continue;
>                 fd = atoi(direntp->d_name);
>                 if (fd == dir_fd || fd == 0 || fd == 1 || fd == 2)
>                         continue;
>                 close(fd);
>         }
>         closedir(dir);
>         return 0;
> }
>

> [6]: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/grantpt.c;h=2030e07fa6e652aac32c775b8c6e005844c3c4eb;hb=HEAD#l17
>      Note that this is an internal implementation that is not exported.
>      Currently, libc seems to not provide an exported version of this
>      because of missing kernel support to do this.

Just to be clear, this code is not compiled into glibc anymore in
typical configurations.  I have posted a patch to turn grantpt into a
no-op: <https://sourceware.org/pipermail/libc-alpha/2020-May/114379.html>

I'm not entirely convinced that it's safe to keep iterating over
/proc/self/fd while also closing descriptors.  Ideally, I think an
application should call getdents64, process the file names for
descriptors in the buffer, and if any have been closed, seek to zero
before the next getdents64 call.  Maybe procfs is different, but with
other file systems, unlinking files can trigger directory reordering,
and then you get strange effects.  The d_ino behavior for
/proc/self/fd is a bit strange as well (it's not consistently
descriptor plus 3).