linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Anton Altaparmakov <anton@tuxera.com>
To: "christian.brauner@ubuntu.com" <christian.brauner@ubuntu.com>
Cc: "James.Bottomley@hansenpartnership.com" 
	<James.Bottomley@hansenpartnership.com>,
	"adilger.kernel@dilger.ca" <adilger.kernel@dilger.ca>,
	"alban@kinvolk.io" <alban@kinvolk.io>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"casey@schaufler-ca.com" <casey@schaufler-ca.com>,
	"containers@lists.linux-foundation.org" 
	<containers@lists.linux-foundation.org>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"cyphar@cyphar.com" <cyphar@cyphar.com>,
	"dhowells@redhat.com" <dhowells@redhat.com>,
	"dmitry.kasatkin@gmail.com" <dmitry.kasatkin@gmail.com>,
	"ebiederm@xmission.com" <ebiederm@xmission.com>,
	"geofft@ldpreload.com" <geofft@ldpreload.com>,
	"hch@lst.de" <hch@lst.de>,
	"hirofumi@mail.parknet.co.jp" <hirofumi@mail.parknet.co.jp>,
	"john.johansen@canonical.com" <john.johansen@canonical.com>,
	"josh@joshtriplett.org" <josh@joshtriplett.org>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"lennart@poettering.net" <lennart@poettering.net>,
	"linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
	"linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>,
	"linux-security-module@vger.kernel.org" 
	<linux-security-module@vger.kernel.org>,
	"linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
	"luto@kernel.org" <luto@kernel.org>,
	"mpatel@redhat.com" <mpatel@redhat.com>,
	"paul@paul-moore.com" <paul@paul-moore.com>,
	"selinux@vger.kernel.org" <selinux@vger.kernel.org>,
	"seth.forshee@canonical.com" <seth.forshee@canonical.com>,
	"smbarber@chromium.org" <smbarber@chromium.org>,
	"stephen.smalley.work@gmail.com" <stephen.smalley.work@gmail.com>,
	"tkjos@google.com" <tkjos@google.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"tycho@tycho.ws" <tycho@tycho.ws>,
	"tytso@mit.edu" <tytso@mit.edu>,
	"viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
	"zohar@linux.ibm.com" <zohar@linux.ibm.com>
Subject: Re: [PATCH v6 24/40] fs: make helpers idmap mount aware
Date: Mon, 12 Apr 2021 12:04:59 +0000	[thread overview]
Message-ID: <E901E25F-41FA-444D-B3C7-A7A786DDD5D5@tuxera.com> (raw)

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


             reply	other threads:[~2021-04-12 12:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-12 12:04 Anton Altaparmakov [this message]
2021-04-12 16:23 ` [PATCH v6 24/40] fs: make helpers idmap mount aware Linus Torvalds
2021-04-13  8:26   ` Christian Brauner
  -- strict thread matches above, loose matches on Subject: below --
2021-01-21 13:19 [PATCH v6 00/40] idmapped mounts Christian Brauner
2021-01-21 13:19 ` [PATCH v6 24/40] fs: make helpers idmap mount aware Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E901E25F-41FA-444D-B3C7-A7A786DDD5D5@tuxera.com \
    --to=anton@tuxera.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=alban@kinvolk.io \
    --cc=arnd@arndb.de \
    --cc=casey@schaufler-ca.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=cyphar@cyphar.com \
    --cc=dhowells@redhat.com \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=geofft@ldpreload.com \
    --cc=hch@lst.de \
    --cc=hirofumi@mail.parknet.co.jp \
    --cc=john.johansen@canonical.com \
    --cc=josh@joshtriplett.org \
    --cc=keescook@chromium.org \
    --cc=lennart@poettering.net \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mpatel@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=selinux@vger.kernel.org \
    --cc=seth.forshee@canonical.com \
    --cc=smbarber@chromium.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=tkjos@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=tycho@tycho.ws \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).