linux-kernel - Re: [PATCH v2 2/7] fsnotify: pin both inode and vfsmount mark

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171030140514.GM23278@quack2.suse.cz>
Date:   Mon, 30 Oct 2017 15:05:14 +0100
From:   Jan Kara <jack@...e.cz>
To:     Miklos Szeredi <mszeredi@...hat.com>
Cc:     Jan Kara <jack@...e.cz>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Amir Goldstein <amir73il@...il.com>,
        Xiong Zhou <xzhou@...hat.com>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/7] fsnotify: pin both inode and vfsmount mark

On Mon 30-10-17 14:42:11, Miklos Szeredi wrote:
> On Mon, Oct 30, 2017 at 2:34 PM, Jan Kara <jack@...e.cz> wrote:
> > On Wed 25-10-17 10:41:34, Miklos Szeredi wrote:
> >> We may fail to pin one of the marks in fsnotify_prepare_user_wait() when
> >> dropping the srcu read lock, resulting in use after free at the next
> >> iteration.
> >>
> >> Solution is to store both marks in iter_info instead of just the one we'll
> >> be sending the event for.
> >
> > I'm sorry but I'm not getting it. Where exactly is use-after-free
> > happening? And how come because if fsnotify_prepare_user_wait() fails to
> > pin some mark, it does not drop SRCU and bails out, doesn't it?
> 
> No no. It's the success case, when we do drop SRCU.
> 
> Two marks non-NULL marks: inode_mark, vfsmount_mark.  We select one
> with fsnotify_compare_groups(), e.g. cmp < 0: vfsmount_mark set to
> NULL.
> 
> So we pin inode_mark but not vfsmount_mark.  Then we find the next
> mark for inode_mark, and use non-NULL vfsmount_node to find the same
> vfsmount_mark that we started out previously.  But that one was not
> pinned while SRCU was released -> use after free.

Got it. Thanks for explanation! But please add a comment that we need to
protect both inode_node and vfsmount_node against freeing so that we can
continue iteration at the place where iter_info.inode_mark and vfsmount_mark
are set.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR