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

[Jan Kara <jack@suse.cz>]

On Mon 30-10-17 14:42:11, Miklos Szeredi wrote:
> On Mon, Oct 30, 2017 at 2:34 PM, Jan Kara <jack@suse.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@suse.com>
SUSE Labs, CR