Re: [PATCH 4/4] file: Fix file descriptor leak in copy_fd_bitmaps()

[Christian Brauner <brauner@kernel.org>]

On Sun, Mar 27, 2022 at 03:21:18PM -0700, Linus Torvalds wrote:
> On Sun, Mar 27, 2022 at 2:53 PM <aissur0002@gmail.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?

(Sorry, I was heads-deep in some other fs work and went into airplaine
mode. I'm back.)

So I investigated a similar report a little while back and I spent quite
a lot of time trying to track this down but didn't find the cause.
If you'd call:

close_range(131, -1, CLOSE_RANGE_UNSHARE);

for an fdtable that is smaller than 131 then we'd call:

unshare_fd(..., 131)
\dup_fd(..., 131)
  \sane_fdtable_size(..., 131)

So sane_fdtable_size() would return 131 which is not aligned. This
couldn't happen before CLOSE_RANGE_UNSHARE afaict. I'll try to do a
repro with this with your suggested fix applied.

Christian