Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
From: Mark Salyzyn <salyzyn@android.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Latchesar Ionkov" <lucho@ionkov.net>,
	"Dave Kleikamp" <shaggy@kernel.org>,
	jfs-discussion@lists.sourceforge.net,
	linux-integrity@vger.kernel.org,
	"Martin Brandenburg" <martin@omnibond.com>,
	samba-technical@lists.samba.org,
	"Dominique Martinet" <asmadeus@codewreck.org>,
	"Chao Yu" <yuchao0@huawei.com>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	linux-unionfs@vger.kernel.org,
	"David Howells" <dhowells@redhat.com>, "Chris Mason" <clm@fb.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Andreas Dilger" <adilger.kernel@dilger.ca>,
	"Eric Paris" <eparis@parisplace.org>,
	netdev@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net,
	linux-afs@lists.infradead.org,
	"Mike Marshall" <hubcap@omnibond.com>,
	linux-xfs@vger.kernel.org,
	"Andreas Gruenbacher" <agruenba@redhat.com>,
	"Sage Weil" <sage@redhat.com>,
	"Miklos Szeredi" <miklos@szeredi.hu>,
	"Richard Weinberger" <richard@nod.at>,
	"Mark Fasheh" <mark@fasheh.com>,
	"Hugh Dickins" <hughd@google.com>,
	"James Morris" <jmorris@namei.org>,
	cluster-devel@redhat.com, selinux@vger.kernel.org,
	"Vyacheslav Dubeyko" <slava@dubeyko.com>,
	"Casey Schaufler" <casey@schaufler-ca.com>,
	v9fs-developer@lists.sourceforge.net,
	"Ilya Dryomov" <idryomov@gmail.com>,
	linux-ext4@vger.kernel.org, kernel-team@android.com,
	linux-mm@kvack.org, devel@lists.orangefs.org,
	"Serge Hallyn" <serge@hallyn.com>,
	"Ernesto A. Fernández" <ernesto.mnd.fernandez@gmail.com>,
	linux-cifs@vger.kernel.org,
	"Eric Van Hensbergen" <ericvh@gmail.com>,
	ecryptfs@vger.kernel.org, "Josef Bacik" <josef@toxicpanda.com>,
	reiserfs-devel@vger.kernel.org, "Tejun Heo" <tj@kernel.org>,
	"Joel Becker" <jlbec@evilplan.org>,
	linux-mtd@lists.infradead.org, "David Sterba" <dsterba@suse.com>,
	"Jaegeuk Kim" <jaegeuk@kernel.org>,
	ceph-devel@vger.kernel.org,
	"Trond Myklebust" <trond.myklebust@hammerspace.com>,
	"Paul Moore" <paul@paul-moore.com>,
	linux-nfs@vger.kernel.org, "Theodore Ts\'o" <tytso@mit.edu>,
	linux-fsdevel@vger.kernel.org,
	"Joseph Qi" <joseph.qi@linux.alibaba.com>,
	"Mathieu Malaterre" <malat@debian.org>,
	"Stephen Smalley" <sds@tycho.nsa.gov>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	"Jeff Layton" <jlayton@kernel.org>,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	"Tyler Hicks" <tyhicks@canonical.com>,
	"Steve French" <sfrench@samba.org>,
	linux-security-module@vger.kernel.org,
	ocfs2-devel@oss.oracle.com, "Jan Kara" <jack@suse.com>,
	"Bob Peterson" <rpeterso@redhat.com>,
	"Phillip Lougher" <phillip@squashfs.org.uk>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"David Woodhouse" <dwmw2@infradead.org>,
	"Anna Schumaker" <anna.schumaker@netapp.com>,
	linux-btrfs@vger.kernel.org,
	"Alexander Viro" <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH] Add flags option to get xattr method paired to __vfs_getxattr
Date: Tue, 13 Aug 2019 07:37:29 -0700
Message-ID: <e211bef2-f346-c9c7-f4b8-c774159b14e1@android.com> (raw)
In-Reply-To: <20190813084801.GA972@kroah.com>

On 8/13/19 1:48 AM, Greg Kroah-Hartman wrote:
> On Mon, Aug 12, 2019 at 12:32:49PM -0700, Mark Salyzyn wrote:
>> --- a/include/linux/xattr.h
>> +++ b/include/linux/xattr.h
>> @@ -30,10 +30,10 @@ struct xattr_handler {
>>   	const char *prefix;
>>   	int flags;      /* fs private flags */
>>   	bool (*list)(struct dentry *dentry);
>> -	int (*get)(const struct xattr_handler *, struct dentry *dentry,
>> +	int (*get)(const struct xattr_handler *handler, struct dentry *dentry,
>>   		   struct inode *inode, const char *name, void *buffer,
>> -		   size_t size);
>> -	int (*set)(const struct xattr_handler *, struct dentry *dentry,
>> +		   size_t size, int flags);
>> +	int (*set)(const struct xattr_handler *handler, struct dentry *dentry,
>>   		   struct inode *inode, const char *name, const void *buffer,
>>   		   size_t size, int flags);
> Wow, 7 arguments.  Isn't there some nice rule of thumb that says once
> you get more then 5, a function becomes impossible to understand?

This is a method with a pot-pourri of somewhat intuitive useful, but not 
always necessary, arguments, the additional argument does not complicate 
the function(s) AFAIK, but maybe its usage. Most functions do not even 
reference handler, the inode is typically a derivative of dentry, The 
arguments most used are the name of the attribute and the buffer/size 
the results are to be placed into.

The addition of flags is actually a pattern borrowed from the [.]set 
method, which provides at least 32 bits of 'control' (of which we added 
only one). Before, it was an anti-pattern.

> Surely this could be a structure passed in here somehow, that way when
> you add the 8th argument in the future, you don't have to change
> everything yet again?  :)
Just be happy I provided int flags, instead of bool no_security ;-> 
there are a few bits there that can be used in the future.
> I don't have anything concrete to offer as a replacement fix for this,
> but to me this just feels really wrong...

I went through 6 different alternatives (in the overlayfs security fix 
patch set) until I found this one that resonated with the security and 
filesystem stakeholders. The one was a direct result of trying to reduce 
the security attack surface. This code was created by threading a 
needle, and evolution. I am game for a 7th alternative to solve the 
unionfs set of recursive calls into acquiring the extended attributes.

-- Mark

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-12 19:32 Mark Salyzyn
2019-08-13  8:28 ` kbuild test robot
2019-08-13  8:48 ` Greg Kroah-Hartman
2019-08-13 14:37   ` Mark Salyzyn [this message]
2019-08-15 19:20   ` James Morris
2019-08-15 20:43     ` Greg Kroah-Hartman
2019-08-15 21:26     ` Mark Salyzyn
2019-08-15 22:27       ` James Morris
2019-08-16 15:30         ` Mark Salyzyn

Reply instructions:

You may reply publically 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=e211bef2-f346-c9c7-f4b8-c774159b14e1@android.com \
    --to=salyzyn@android.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=agruenba@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=anna.schumaker@netapp.com \
    --cc=asmadeus@codewreck.org \
    --cc=casey@schaufler-ca.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=clm@fb.com \
    --cc=cluster-devel@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=davem@davemloft.net \
    --cc=devel@lists.orangefs.org \
    --cc=dhowells@redhat.com \
    --cc=dsterba@suse.com \
    --cc=dwmw2@infradead.org \
    --cc=ecryptfs@vger.kernel.org \
    --cc=eparis@parisplace.org \
    --cc=ericvh@gmail.com \
    --cc=ernesto.mnd.fernandez@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hubcap@omnibond.com \
    --cc=hughd@google.com \
    --cc=idryomov@gmail.com \
    --cc=jack@suse.com \
    --cc=jaegeuk@kernel.org \
    --cc=jfs-discussion@lists.sourceforge.net \
    --cc=jlayton@kernel.org \
    --cc=jlbec@evilplan.org \
    --cc=jmorris@namei.org \
    --cc=josef@toxicpanda.com \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=kernel-team@android.com \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=lucho@ionkov.net \
    --cc=malat@debian.org \
    --cc=mark@fasheh.com \
    --cc=martin@omnibond.com \
    --cc=miklos@szeredi.hu \
    --cc=netdev@vger.kernel.org \
    --cc=ocfs2-devel@oss.oracle.com \
    --cc=paul@paul-moore.com \
    --cc=phillip@squashfs.org.uk \
    --cc=reiserfs-devel@vger.kernel.org \
    --cc=richard@nod.at \
    --cc=rpeterso@redhat.com \
    --cc=sage@redhat.com \
    --cc=samba-technical@lists.samba.org \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=sfrench@samba.org \
    --cc=shaggy@kernel.org \
    --cc=slava@dubeyko.com \
    --cc=stable@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=trond.myklebust@hammerspace.com \
    --cc=tyhicks@canonical.com \
    --cc=tytso@mit.edu \
    --cc=v9fs-developer@lists.sourceforge.net \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yuchao0@huawei.com \
    --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

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git