On Fri, Mar 26, 2021 at 2:50 PM Christian Brauner <christian.brauner@xxxxxxxxxx> wrote: > > 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: The crash does not happen with this patch. Tested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> Thanks for the quick fix! > 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 >
