lkml.org 
[lkml]   [2018]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: possible deadlock in mnt_want_write
On Wed, Nov 21, 2018 at 10:04 PM Goldwyn Rodrigues <rgoldwyn@suse.de> wrote:
>
> On 20:57 21/11, Amir Goldstein wrote:
> > On Wed, Nov 21, 2018 at 8:33 PM syzbot
> > <syzbot+ae82084b07d0297e566b@syzkaller.appspotmail.com> wrote:
> > >
> > > syzbot has found a reproducer for the following crash on:
> > >
> > > HEAD commit: 442b8cea2477 Add linux-next specific files for 20181109
> > > git tree: linux-next
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=11a1426d400000
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=2f72bdb11df9fbe8
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=ae82084b07d0297e566b
> > > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1632326d400000
> > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17a16ed5400000
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+ae82084b07d0297e566b@syzkaller.appspotmail.com
> > >
...
> > > percpu_down_read include/linux/percpu-rwsem.h:59 [inline]
> > > __sb_start_write+0x214/0x370 fs/super.c:1564
> > > sb_start_write include/linux/fs.h:1607 [inline]
> > > mnt_want_write+0x3f/0xc0 fs/namespace.c:359
> > > ovl_want_write+0x76/0xa0 fs/overlayfs/util.c:24
> > > ovl_open_maybe_copy_up+0x12c/0x190 fs/overlayfs/copy_up.c:888
> > > ovl_open+0xb3/0x260 fs/overlayfs/file.c:123
> > > do_dentry_open+0x499/0x1250 fs/open.c:771
> > > vfs_open fs/open.c:880 [inline]
> > > dentry_open+0x143/0x1d0 fs/open.c:896
> > > ima_calc_file_hash+0x324/0x570
> >
> > I suppose ima_calc_file_hash opens the file with write flags
> > and cause overlay to try to copy up which takes mnt_want_write().
> > Why does IMA need to open the file with write flags?
> >
> > Isn't this commit supposed to prevent that:
> > a408e4a86b36 ima: open a new file instance if no read permissions
> >
>
> Not write, read flags. This patch re-opens the files in O_RDONLY for
> files opened with O_WRONLY and cannot be read, so that the hash can be
> calculated. IOW, the user opened the file in overlayfs with write flags.
>

My point is:
ovl_open_need_copy_up() -> ovl_open_flags_need_copy_up()
returns false for O_RDONLY flags and never gets to ovl_want_write(),
so how is the stack trace below possible when ima_calc_file_hash()
removes all "write" flags before opening the file?

ovl_want_write+0x76/0xa0 fs/overlayfs/util.c:24
ovl_open_maybe_copy_up+0x12c/0x190 fs/overlayfs/copy_up.c:888
ovl_open+0xb3/0x260 fs/overlayfs/file.c:123
do_dentry_open+0x499/0x1250 fs/open.c:771
vfs_open fs/open.c:880 [inline]
dentry_open+0x143/0x1d0 fs/open.c:896
ima_calc_file_hash+0x324/0x570 security/integrity/ima/ima_crypto.c:427

The answer is found in the zysbot repro: it opens the file with
open(O_WRONLY|O_RDWR) (0x3)
Not nice, but apparently possible.

How about adding O_RDWR to the masked flags in ima_calc_file_hash()?
Since syzbot has a reproducer, you can send it a patch to verify the fix.

Thanks,
Amir.

\
 
 \ /
  Last update: 2018-11-21 21:23    [W:0.053 / U:1.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site