linux-kernel - Re: [PATCH 4/4] file: Fix file descriptor leak in copy_fd

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]

Date:   Mon, 28 Mar 2022 00:54:57 +0300
From:   aissur0002@...il.com
To:     Alexey Khoroshilov <khoroshilov@...ras.ru>,
        Eric Biggers <ebiggers@...nel.org>,
        Christian Brauner <brauner@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Fedor Pchelkin <aissur0002@...il.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] file: Fix file descriptor leak in copy_fd_bitmaps()

I am sorry, that was my first attempt to contribute to the kernel and
I messed up a little bit with the patch tag: it is actually a single, 
standalone patch and there has been nothing posted previously.

Moreover, I should have provided you with the context of discovering 
the bug: a Syzkaller bug reproducer and CrashReport are attached to 
the mail.

In few words, an error occurs while executing close_range() call with
CLOSE_RANGE_UNSHARE flag: in __close_range() the value of
max_unshare_fds (later used as max_fds in dup_fd() and copy_fd_bitmaps())
can be an arbitrary number.

  if (max_fd >= last_fd(files_fdtable(cur_fds)))
    max_unshare_fds = fd;

and not be a multiple of BITS_PER_LONG or BITS_PER_BYTE. In case of
Syzkaller reproducer this value equals 69 - three standard file descriptors, 
66 pipe descriptors and 1 for /dev/ptmx; nr value used in alloc_fdtable()
is 128 (that' okay). But here is the weird place which causes the leak
described in the patch. KASAN detected that leak and possibly the
location of the leak is in copy_fd_bitmaps(): I experimented with
possible solutions and if add_byte logic is implemented, then KASAN does 
not find any leaks. Of course, the problem can be somewhere else and I 
don't notice it.

On Sat, Mar 26, 2022 at 3:15 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> Certainly NR_OPEN_DEFAULT, sane_fdtable_size() and max_fds should
> always be a multiple of BITS_PER_LONG.

I totally agree with you but in the reproducer case the max_fds
value does not follow these rules.

I think there is probably something wrong in dup_fd() when getting
open_files value and passing it to copy_fd_bitmaps().

View attachment "repro.c" of type "text/x-csrc" (7831 bytes)

View attachment "CrashLog.txt" of type "text/plain" (15025 bytes)