From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752539AbdJ3OFS (ORCPT ); Mon, 30 Oct 2017 10:05:18 -0400 Received: from mx2.suse.de ([195.135.220.15]:53861 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751572AbdJ3OFQ (ORCPT ); Mon, 30 Oct 2017 10:05:16 -0400 Date: Mon, 30 Oct 2017 15:05:14 +0100 From: Jan Kara To: Miklos Szeredi Cc: Jan Kara , linux-fsdevel , Amir Goldstein , Xiong Zhou , lkml Subject: Re: [PATCH v2 2/7] fsnotify: pin both inode and vfsmount mark Message-ID: <20171030140514.GM23278@quack2.suse.cz> References: <1508920899-8115-1-git-send-email-mszeredi@redhat.com> <1508920899-8115-3-git-send-email-mszeredi@redhat.com> <20171030133438.GK23278@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 30-10-17 14:42:11, Miklos Szeredi wrote: > On Mon, Oct 30, 2017 at 2:34 PM, Jan Kara 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 SUSE Labs, CR