linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v6 24/40] fs: make helpers idmap mount aware
@ 2021-04-12 12:04 Anton Altaparmakov
  2021-04-12 16:23 ` Linus Torvalds
  0 siblings, 1 reply; 3+ messages in thread
From: Anton Altaparmakov @ 2021-04-12 12:04 UTC (permalink / raw)
  To: christian.brauner
  Cc: James.Bottomley, adilger.kernel, alban, arnd, casey, containers,
	corbet, cyphar, dhowells, dmitry.kasatkin, ebiederm, geofft, hch,
	hirofumi, john.johansen, josh, keescook, lennart, linux-api,
	linux-ext4, linux-fsdevel, linux-integrity,
	linux-security-module, linux-xfs, luto, mpatel, paul, selinux,
	seth.forshee, smbarber, stephen.smalley.work, tkjos,
	Linus Torvalds, tycho, tytso, viro, zohar

Hi,

I noticed this patch got merged into mainline and looking through the HFS+ changes, I noticed something that struck me as odd.  I am not familiar with this patch set so perhaps it is the intention but I wanted to ask you because it just seems strange thing to do.

So you are adding a new argument of "struct user_namespace *mnt_userns" to lots of functions but then inside the functions when they call another function you often make that use "&init_user_ns" instead of the passed in "mnt_userns" which kind of defeats the point of having the new "mnt_userns" argument altogether, doesn't it?

Example after this chunk:

diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 642e067d8fe8..7a937de9b2ad 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -241,7 +241,8 @@ static int hfsplus_file_release(struct inode *inode, struct file *file)
        return 0;
 }

-static int hfsplus_setattr(struct dentry *dentry, struct iattr *attr)
+static int hfsplus_setattr(struct user_namespace *mnt_userns,
+                    struct dentry *dentry, struct iattr *attr)
 {
        struct inode *inode = d_inode(dentry);
        int error;

The code now looks like this:

static int hfsplus_setattr(struct user_namespace *mnt_userns,
                           struct dentry *dentry, struct iattr *attr)
{
        struct inode *inode = d_inode(dentry);
        int error;

        error = setattr_prepare(&init_user_ns, dentry, attr);
        if (error)
                return error;
[...]
        setattr_copy(&init_user_ns, inode, attr);
        mark_inode_dirty(inode);

        return 0;
}

Shouldn't that be using mnt_userns instead of &init_user_ns both for the setattr_prepare() and setattr_copy() calls?

Please note this is just one example - it seems the kernel is now littered with such examples in current mainline and I don't mean just HFS+ - this is now all over the place...

Best regards,

	Anton
-- 
Anton Altaparmakov <anton at tuxera.com> (replace at with @)
Lead in File System Development, Tuxera Inc., http://www.tuxera.com/
Linux NTFS maintainer


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-13  8:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 12:04 [PATCH v6 24/40] fs: make helpers idmap mount aware Anton Altaparmakov
2021-04-12 16:23 ` Linus Torvalds
2021-04-13  8:26   ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).