| Message ID | 20210112220124.837960-16-christian.brauner@ubuntu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | idmapped mounts | expand |
On Tue, Jan 12, 2021 at 11:00:57PM +0100, Christian Brauner wrote: > Add a simple helper to retrieve the user namespace associated with the > vfsmount of a file. Christoph correctly points out that this makes > codepaths (e.g. ioctls) way easier to follow that would otherwise > dereference via mnt_user_ns(file->f_path.mnt). > > In order to make file_user_ns() static inline we'd need to include > mount.h in either file.h or fs.h which seems undesirable so let's simply > not force file_user_ns() to be inline. I'd be tempted to just make this an inline. Otherwise this looks ok: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Jan 13, 2021 at 1:52 AM Christian Brauner <christian.brauner@ubuntu.com> wrote: > Add a simple helper to retrieve the user namespace associated with the > vfsmount of a file. Christoph correctly points out that this makes > codepaths (e.g. ioctls) way easier to follow that would otherwise > dereference via mnt_user_ns(file->f_path.mnt). > > In order to make file_user_ns() static inline we'd need to include > mount.h in either file.h or fs.h which seems undesirable so let's simply > not force file_user_ns() to be inline. [...] > +struct user_namespace *file_user_ns(struct file *file) > +{ > + return mnt_user_ns(file->f_path.mnt); > +} That name is confusing to me, because when I think of "the userns of a file", it's file->f_cred->user_ns. There are a bunch of places that look at that, as you can see from grepping for "f_cred->user_ns". If you really want this to be a separate helper, can you maybe give it a clearer name? file_mnt_user_ns(), or something like that, idk.
On Tue, Jan 19, 2021 at 04:05:00PM +0100, Jann Horn wrote: > On Wed, Jan 13, 2021 at 1:52 AM Christian Brauner > <christian.brauner@ubuntu.com> wrote: > > Add a simple helper to retrieve the user namespace associated with the > > vfsmount of a file. Christoph correctly points out that this makes > > codepaths (e.g. ioctls) way easier to follow that would otherwise > > dereference via mnt_user_ns(file->f_path.mnt). > > > > In order to make file_user_ns() static inline we'd need to include > > mount.h in either file.h or fs.h which seems undesirable so let's simply > > not force file_user_ns() to be inline. > [...] > > +struct user_namespace *file_user_ns(struct file *file) > > +{ > > + return mnt_user_ns(file->f_path.mnt); > > +} > > That name is confusing to me, because when I think of "the userns of a > file", it's file->f_cred->user_ns. There are a bunch of places that > look at that, as you can see from grepping for "f_cred->user_ns". > > If you really want this to be a separate helper, can you maybe give it > a clearer name? file_mnt_user_ns(), or something like that, idk. Done.
diff --git a/fs/namei.c b/fs/namei.c index cd124e18cec5..d8dee449e92a 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -259,6 +259,12 @@ void putname(struct filename *name) __putname(name); } +struct user_namespace *file_user_ns(struct file *file) +{ + return mnt_user_ns(file->f_path.mnt); +} +EXPORT_SYMBOL(file_user_ns); + /** * check_acl - perform ACL permission checking * @mnt_userns: user namespace of the mount the inode was found from diff --git a/include/linux/fs.h b/include/linux/fs.h index ef993857682b..89e41a821bad 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2585,6 +2585,7 @@ static inline struct file *file_clone_open(struct file *file) return dentry_open(&file->f_path, file->f_flags, file->f_cred); } extern int filp_close(struct file *, fl_owner_t id); +extern struct user_namespace *file_user_ns(struct file *file); extern struct filename *getname_flags(const char __user *, int, int *); extern struct filename *getname(const char __user *);
Add a simple helper to retrieve the user namespace associated with the vfsmount of a file. Christoph correctly points out that this makes codepaths (e.g. ioctls) way easier to follow that would otherwise dereference via mnt_user_ns(file->f_path.mnt). In order to make file_user_ns() static inline we'd need to include mount.h in either file.h or fs.h which seems undesirable so let's simply not force file_user_ns() to be inline. Cc: David Howells <dhowells@redhat.com> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org Suggested-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> --- /* v2 */ patch not present /* v3 */ patch not present /* v4 */ patch not present /* v5 */ patch introduced base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837 --- fs/namei.c | 6 ++++++ include/linux/fs.h | 1 + 2 files changed, 7 insertions(+)