On Fri, Mar 26, 2021 at 10:34:28AM +0100, Christian Brauner wrote: > On Fri, Mar 26, 2021, 10:21 Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote: > > > On Fri, Mar 26, 2021 at 10:12 AM Christian Brauner > > <christian.brauner@xxxxxxxxxx> wrote: > > > > > > On Fri, Mar 26, 2021 at 09:02:08AM +0100, Dmitry Vyukov wrote: > > > > On Fri, Mar 26, 2021 at 8:55 AM syzbot > > > > <syzbot+283ce5a46486d6acdbaf@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > > > > Hello, > > > > > > > > > > syzbot found the following issue on: > > > > > > > > > > HEAD commit: 5ee96fa9 Merge tag 'irq-urgent-2021-03-21' of git:// > > git.ke.. > > > > > git tree: upstream > > > > > console output: > > https://syzkaller.appspot.com/x/log.txt?x=17fb84bed00000 > > > > > kernel config: > > https://syzkaller.appspot.com/x/.config?x=6abda3336c698a07 > > > > > dashboard link: > > https://syzkaller.appspot.com/bug?extid=283ce5a46486d6acdbaf > > > > > > > > > > Unfortunately, I don't have any reproducer for this issue yet. > > > > > > > > > > IMPORTANT: if you fix the issue, please add the following tag to the > > commit: > > > > > Reported-by: syzbot+283ce5a46486d6acdbaf@xxxxxxxxxxxxxxxxxxxxxxxxx > > > > > > > > I was able to reproduce this with the following C program: > > > > > > https://gist.githubusercontent.com/dvyukov/00fb7aae489f22c60b4e64b45ef14d60/raw/cb368ca523d01986c2917f4414add0893b8f4243/gistfile1.txt > > > > > > > > +Christian > > > > The repro also contains close_range as the previous similar crash: > > > > > > https://syzkaller.appspot.com/bug?id=1bef50bdd9622a1969608d1090b2b4a588d0c6ac > > > > I don't know if it's related or not in this case, but looks suspicious. > > > > > > Hm, I fail to reproduce this with your repro. Do you need to have it run > > > for a long time? > > > One thing that strucky my eye is that binfmt_misc gets setup which made > > > me go huh and I see commit > > > > > > commit e7850f4d844e0acfac7e570af611d89deade3146 > > > Author: Lior Ribak <liorribak@xxxxxxxxx> > > > Date: Fri Mar 12 21:07:41 2021 -0800 > > > > > > binfmt_misc: fix possible deadlock in bm_register_write > > > > > > which uses filp_close() after having called open_exec() on the > > > interpreter which makes me wonder why this doesn't have to use fput() > > > like in all other codepaths for binfmnt_*. > > > > > > Can you revert this commit and see if you can reproduce this issue. > > > Maybe this is a complete red herring but worth a try. > > > > > > This program reproduces the crash for me almost immediately. Are you > > sure you used the right commit/config? > > > > I was trying to reproduce on v5.12-rc3 with all KASAN, KCSAN, KFENCE etc. > turned on. > I have an appointment I need to go to but will try to reproduce with commit > and config you provided when I get home. > I really hope it's not reproducible with v5.12-rc3 and only later commits > since that would allow easier bisection. Ok, I think I know what's going on. This fixes it for me. Can you test too, please? I tried the #syz test way but syzbot doesn't have the reproducer you gave me: Thank you! Christian >From eeb120d02f40b15a925f54ebcf2b4c747c741ad0 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brauner@xxxxxxxxxx> Date: Fri, 26 Mar 2021 13:33:03 +0100 Subject: [PATCH] file: fix close_range() for unshare+cloexec syzbot reported a bug when putting the last reference to a tasks file descriptor table. Debugging this showed we didn't recalculate the current maximum fd number for CLOSE_RANGE_UNSHARE | CLOSE_RANGE_CLOEXEC after we unshared the file descriptors table. So max_fd could exceed the current fdtable maximum causing us to set excessive bits. As a concrete example, let's say the user requested everything from fd 4 to ~0UL to be closed and their current fdtable size is 256 with their highest open fd being 4. With CLOSE_RANGE_UNSHARE the caller will end up with a new fdtable which has room for 64 file descriptors since that is the lowest fdtable size we accept. But now max_fd will still point to 255 and needs to be adjusted. Fix this by retrieving the correct maximum fd value in __range_cloexec(). Reported-by: syzbot+283ce5a46486d6acdbaf@xxxxxxxxxxxxxxxxxxxxxxxxx Cc: Christoph Hellwig <hch@xxxxxx> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> Cc: linux-fsdevel@xxxxxxxxxxxxxxx Cc: stable@xxxxxxxxxxxxxxx Signed-off-by: Christian Brauner <christian.brauner@xxxxxxxxxx> --- fs/file.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/fs/file.c b/fs/file.c index f3a4bac2cbe9..5ef62377d924 100644 --- a/fs/file.c +++ b/fs/file.c @@ -632,6 +632,7 @@ EXPORT_SYMBOL(close_fd); /* for ksys_close() */ static inline void __range_cloexec(struct files_struct *cur_fds, unsigned int fd, unsigned int max_fd) { + unsigned int cur_max; struct fdtable *fdt; if (fd > max_fd) @@ -639,7 +640,12 @@ static inline void __range_cloexec(struct files_struct *cur_fds, spin_lock(&cur_fds->file_lock); fdt = files_fdtable(cur_fds); - bitmap_set(fdt->close_on_exec, fd, max_fd - fd + 1); + /* make very sure we're using the correct maximum value */ + cur_max = fdt->max_fds; + cur_max--; + cur_max = min(max_fd, cur_max); + if (fd <= cur_max) + bitmap_set(fdt->close_on_exec, fd, cur_max - fd + 1); spin_unlock(&cur_fds->file_lock); } -- 2.27.0
