From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1BA79C433B4 for ; Tue, 20 Apr 2021 11:42:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C9D0D613BF for ; Tue, 20 Apr 2021 11:42:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231422AbhDTLmb (ORCPT ); Tue, 20 Apr 2021 07:42:31 -0400 Received: from mail.kernel.org ([198.145.29.99]:57582 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230491AbhDTLma (ORCPT ); Tue, 20 Apr 2021 07:42:30 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 4B5AF613AB; Tue, 20 Apr 2021 11:41:57 +0000 (UTC) Date: Tue, 20 Apr 2021 13:41:54 +0200 From: Christian Brauner To: Amir Goldstein Cc: Jan Kara , linux-fsdevel , Miklos Szeredi , "J. Bruce Fields" , Al Viro Subject: Re: fsnotify path hooks Message-ID: <20210420114154.mwjj7reyntzjkvnw@wittgenstein> References: <20210401102947.GA29690@quack2.suse.cz> <20210408125258.GB3271@quack2.suse.cz> <20210409100811.GA20833@quack2.suse.cz> <20210409104546.37i6h2i4ga2xakvp@wittgenstein> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Tue, Apr 20, 2021 at 09:01:09AM +0300, Amir Goldstein wrote: > > One thing, whatever you end up passing to vfs_create() please make sure > > to retrieve mnt_userns once so permission checking and object creation > > line-up: > > > > int vfs_create(struct vfsmount *mnt, struct inode *dir, > > struct dentry *dentry, umode_t mode, bool want_excl) > > { > > struct user_namespace *mnt_userns; > > > > mnt_userns = mnt_user_ns(mnt); > > > > int error = may_create(mnt_userns, dir, dentry); > > if (error) > > return error; > > > > if (!dir->i_op->create) > > return -EACCES; /* shouldn't it be ENOSYS? */ > > mode &= S_IALLUGO; > > mode |= S_IFREG; > > error = security_inode_create(dir, dentry, mode); > > if (error) > > return error; > > error = dir->i_op->create(mnt_userns, dir, dentry, mode, want_excl); > > if (!error) > > fsnotify_create(mnt, dir, dentry); > > return error; > > } > > > > Christian, > > What is the concern here? > Can mnt_user_ns() change under us? > I am asking because Al doesn't like both mnt_userns AND path to > be passed to do_tuncate() => notify_change() > So I will need to retrieve mnt_userns again inside notify_change() > after it had been used for security checks in do_open(). > Would that be acceptable to you? The mnt_userns can't change once a mnt has been idmapped and it can never change if the mount is visible in the filesystem already. The only case we've been worried about and why we did it this way is when you have a caller do fd = open_tree(OPEN_TREE_CLONE) and then share that unattached fd with multiple processes T1: mkdirat(fd, "dir1", 0755); T2: mount_setattr(fd, "",); /* changes idmapping */ That case isn't a problem if the mnt_userns is only retrieved once for permission checking and operating on the inode. I think with your changes that still shouldn't be an issue though since the vfs_*() helpers encompass the permission checking anyway and for notify_change, we could simply add a mnt_userns field to struct iattr and pass it down.