linux-ext4 - Re: [PATCH v6 24/40] fs: make helpers idmap mount aware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 12 Apr 2021 12:04:59 +0000
From:   Anton Altaparmakov <anton@...era.com>
To:     "christian.brauner@...ntu.com" <christian.brauner@...ntu.com>
CC:     "James.Bottomley@...senpartnership.com" 
        <James.Bottomley@...senpartnership.com>,
        "adilger.kernel@...ger.ca" <adilger.kernel@...ger.ca>,
        "alban@...volk.io" <alban@...volk.io>,
        "arnd@...db.de" <arnd@...db.de>,
        "casey@...aufler-ca.com" <casey@...aufler-ca.com>,
        "containers@...ts.linux-foundation.org" 
        <containers@...ts.linux-foundation.org>,
        "corbet@....net" <corbet@....net>,
        "cyphar@...har.com" <cyphar@...har.com>,
        "dhowells@...hat.com" <dhowells@...hat.com>,
        "dmitry.kasatkin@...il.com" <dmitry.kasatkin@...il.com>,
        "ebiederm@...ssion.com" <ebiederm@...ssion.com>,
        "geofft@...reload.com" <geofft@...reload.com>,
        "hch@....de" <hch@....de>,
        "hirofumi@...l.parknet.co.jp" <hirofumi@...l.parknet.co.jp>,
        "john.johansen@...onical.com" <john.johansen@...onical.com>,
        "josh@...htriplett.org" <josh@...htriplett.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "lennart@...ttering.net" <lennart@...ttering.net>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "linux-xfs@...r.kernel.org" <linux-xfs@...r.kernel.org>,
        "luto@...nel.org" <luto@...nel.org>,
        "mpatel@...hat.com" <mpatel@...hat.com>,
        "paul@...l-moore.com" <paul@...l-moore.com>,
        "selinux@...r.kernel.org" <selinux@...r.kernel.org>,
        "seth.forshee@...onical.com" <seth.forshee@...onical.com>,
        "smbarber@...omium.org" <smbarber@...omium.org>,
        "stephen.smalley.work@...il.com" <stephen.smalley.work@...il.com>,
        "tkjos@...gle.com" <tkjos@...gle.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        "tycho@...ho.ws" <tycho@...ho.ws>, "tytso@....edu" <tytso@....edu>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "zohar@...ux.ibm.com" <zohar@...ux.ibm.com>
Subject: Re: [PATCH v6 24/40] fs: make helpers idmap mount aware

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