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]

Message-ID: <CAHk-=wh2Ao+OgnWSxHsJodXiLwtaUndXSkuhh9yKnA3iXyBLEA@mail.gmail.com>
Date:   Sun, 27 Mar 2022 15:21:18 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Fedor Pchelkin <aissur0002@...il.com>
Cc:     Alexey Khoroshilov <khoroshilov@...ras.ru>,
        Eric Biggers <ebiggers@...nel.org>,
        Christian Brauner <brauner@...nel.org>,
        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()

On Sun, Mar 27, 2022 at 2:53 PM <aissur0002@...il.com> wrote:
>
> 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.

No problem, thanks for clarifying.

But the patch itself in that case is missing some detail, since it
clearly doesn't apply to upstream.

Anyway:

> 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.

Very good, that's the piece I was missing. I only looked in fs/file.c,
and missed how that max_unshare_fds gets passed from __close_range()
into fork.c for unshare_fd() and then back to file.c and dup_fd(). And
then affects sane_fdtable_size().

I _think_ it should be sufficient to just do something like

       max_fds = ALIGN(max_fds, BITS_PER_LONG)

in sane_fdtable_size(), but honestly, I haven't actually thought about
it all that much. That's just a gut feel without really analyzing
things - sane_fdtable_size() really should never return a value that
isn't BITS_PER_LONG aligned.

And that whole close_range() is why I added Christian Brauner to the
participant list, though, so let's see if Christian has any comments.

Christian?

Btw, do you have a pointer to the syzbot report? I see the repro and
the crashlog you attached, but it would be good to have that pointer
to the syzbot original too.

Or did you just do this by running syzkaller yourself and there is no
external report?

                 Linus