Re: fcntl(F_DUPFD) causing apparent file descriptor table corruption

[Al Viro <viro@zeniv.linux.org.uk>]

On Tue, May 19, 2020 at 02:03:03PM -0700, Thiago Macieira wrote:

> On my machine, /proc/sys/fs/nr_open is 1073741816 and I have 32 GB of RAM (if 
> the problem is related to memory consumption).
> 
> The problem only occurs when growing the table.
> 
> strace shows something like:
> fcntl(2, F_DUPFD, 1024)                 = 1024
> close(1024)                             = 0
> fcntl(2, F_DUPFD, 2048)                 = 2048
> close(2048)                             = 0
> fcntl(2, F_DUPFD, 4096)                 = 4096
> close(4096)                             = 0
> fcntl(2, F_DUPFD, 8192)                 = 8192
> close(8192)                             = 0
> fcntl(2, F_DUPFD, 16384)                = 16384
> close(16384)                            = 0
> fcntl(2, F_DUPFD, 32768)                = 32768
> close(32768)                            = 0
> fcntl(2, F_DUPFD, 65536)                = 65536
> close(65536)                            = 0
> fcntl(2, F_DUPFD, 131072)               = 131072
> close(131072)                           = 0
> fcntl(2, F_DUPFD, 262144)               = 262144
> close(262144)                           = 0
> fcntl(2, F_DUPFD, 524288)               = 524288
> close(524288)                           = 0
> fcntl(2, F_DUPFD, 1048576)              = 1048576
> close(1048576)                          = 0
> fcntl(2, F_DUPFD, 2097152)              = 2097152
> close(2097152)                          = 0
> fcntl(2, F_DUPFD, 4194304)              = 4194304
> close(4194304)                          = 0
> fcntl(2, F_DUPFD, 8388608)              = 8388608
> close(8388608)                          = 0
> fcntl(2, F_DUPFD, 16777216)             = 16777216
> close(16777216)                         = 0
> fcntl(2, F_DUPFD, 33554432)             = 33554432
> close(33554432)                         = 0
> fcntl(2, F_DUPFD, 67108864)             = 67108864
> close(67108864)                         = 0
> fcntl(2, F_DUPFD, 134217728)            = 134217728
> close(134217728)                        = 0
> fcntl(2, F_DUPFD, 536870912)            = 536870912
> close(536870912)                        = 0
> write(1, "success\n", 8)                = EBADF

Er...  It's going by powers of two, isn't it?  So where's
268435456 between 134217728 and 536870912?  And, assuming
it's a 64bit box, 536870912 pointers is 4Gb, which suggests
that we are running into 32bit overflow on calculating
some size...  Let's see -
static int expand_fdtable(struct files_struct *files, unsigned int nr)
...
        cur_fdt = files_fdtable(files);
        BUG_ON(nr < cur_fdt->max_fds);
        copy_fdtable(new_fdt, cur_fdt);
so we probably want copy_fdtable().
static void copy_fdtable(struct fdtable *nfdt, struct fdtable *ofdt)
{
        unsigned int cpy, set;

        BUG_ON(nfdt->max_fds < ofdt->max_fds);

        cpy = ofdt->max_fds * sizeof(struct file *);

Right, here's your multiplication overflow.

        set = (nfdt->max_fds - ofdt->max_fds) * sizeof(struct file *);
        memcpy(nfdt->fd, ofdt->fd, cpy);
... and here's not getting the things copied.  Which means that pointer
is left uninitialized and the damn thing might very well be a security
problem - you'd lucked out and ran into NULL, but had there been a pointer
to something, you would've gotten a memory corruptor.

The obvious fix would be to turn cpy and set into size_t - as in
ed fs/file.c <<'EOF'
/copy_fdtable/+2s/unsigned int/size_t/
w
q
EOF

On size_t overflow you would've failed allocation before getting to that
point - see sysctl_nr_open_max initializer.  Overflow in alloc_fdtable()
(nr is unsigned int there) also can't happen, AFAICS - the worst you
can get is 1U<<31, which will fail sysctl_nr_open comparison.

I really wonder about the missing couple of syscalls in your strace, though;
could you verify that they _are_ missing and see what the fix above does to
your testcase?