[PATCH v14 3/5] overlayfs: handle XATTR_NOSECURITY flag for get xattr method

[PATCH v14 3/5] overlayfs: handle XATTR_NOSECURITY flag for get xattr method

[Mark Salyzyn]

Because of the overlayfs getxattr recursion, the incoming inode fails
to update the selinux sid resulting in avc denials being reported
against a target context of u:object_r:unlabeled:s0.

Solution is to respond to the XATTR_NOSECURITY flag in get xattr
method that calls the __vfs_getxattr handler instead so that the
context can be read in, rather than being denied with an -EACCES
when vfs_getxattr handler is called.

For the use case where access is to be blocked by the security layer.

The path then would be security(dentry) ->
__vfs_getxattr({dentry...XATTR_NOSECURITY}) ->
handler->get({dentry...XATTR_NOSECURITY}) ->
__vfs_getxattr({realdentry...XATTR_NOSECURITY}) ->
lower_handler->get({realdentry...XATTR_NOSECURITY}) which
would report back through the chain data and success as expected,
the logging security layer at the top would have the data to
determine the access permissions and report back to the logs and
the caller that the target context was blocked.

For selinux this would solve the cosmetic issue of the selinux log
and allow audit2allow to correctly report the rule needed to address
the access problem.

Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: linux-unionfs@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: kernel-team@android.com
Cc: linux-security-module@vger.kernel.org

---
v14 - rebase to use xattr_gs_args.

v13 - rebase to use __vfs_getxattr flags option.

v12 - Added back to patch series as get xattr with flag option.

v11 - Squashed out of patch series and replaced with per-thread flag
      solution.

v10 - Added to patch series as __get xattr method.

---
 fs/overlayfs/inode.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 5fb7608647a4..2eb037c325bf 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -367,12 +367,15 @@ int ovl_xattr_get(struct xattr_gs_args *args)
 {
 	ssize_t res;
 	const struct cred *old_cred;
-	struct dentry *realdentry =
+	struct xattr_gs_args my_args = *args;
+
+	my_args.dentry =
 		ovl_i_dentry_upper(args->inode) ?:
 		ovl_dentry_lower(args->dentry);
+	my_args.inode = d_inode(my_args.dentry);
 
 	old_cred = ovl_override_creds(args->dentry->d_sb);
-	res = vfs_getxattr(realdentry, args->name, args->buffer, args->size);
+	res = __vfs_getxattr(&my_args);
 	revert_creds(old_cred);
 	return res;
 }
-- 
2.23.0.866.gb869b98d4c-goog

[PATCH v14 4/5] overlayfs: internal getxattr operations without sepolicy checking

[Mark Salyzyn]

Check impure, opaque, origin & meta xattr with no sepolicy audit
(using __vfs_getxattr) since these operations are internal to
overlayfs operations and do not disclose any data.  This became
an issue for credential override off since sys_admin would have
been required by the caller; whereas would have been inherently
present for the creator since it performed the mount.

This is a change in operations since we do not check in the new
ovl_do_vfs_getxattr function if the credential override is off or
not.  Reasoning is that the sepolicy check is unnecessary overhead,
especially since the check can be expensive.

Because for override credentials off, this affects _everyone_ that
underneath performs private xattr calls without the appropriate
sepolicy permissions and sys_admin capability.  Providing blanket
support for sys_admin would be bad for all possible callers.

For the override credentials on, this will affect only the mounter,
should it lack sepolicy permissions. Not considered a security
problem since mounting by definition has sys_admin capabilities,
but sepolicy contexts would still need to be crafted.

It should be noted that there is precedence, __vfs_getxattr is used
in other filesystems for their own internal trusted xattr management.

Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: linux-unionfs@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: kernel-team@android.com
Cc: linux-security-module@vger.kernel.org

---
v14 - rebase to use xattr_gs_args.

v13 - rebase to use __vfs_getxattr flags option

v12 - rebase

v11 - switch name to ovl_do_vfs_getxattr, fortify comment

v10 - added to patch series

---
 fs/overlayfs/namei.c     | 12 +++++++-----
 fs/overlayfs/overlayfs.h |  2 ++
 fs/overlayfs/util.c      | 32 +++++++++++++++++++++++---------
 3 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 9702f0d5309d..a4a452c489fa 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -106,10 +106,11 @@ int ovl_check_fh_len(struct ovl_fh *fh, int fh_len)
 
 static struct ovl_fh *ovl_get_fh(struct dentry *dentry, const char *name)
 {
-	int res, err;
+	ssize_t res;
+	int err;
 	struct ovl_fh *fh = NULL;
 
-	res = vfs_getxattr(dentry, name, NULL, 0);
+	res = ovl_do_vfs_getxattr(dentry, name, NULL, 0);
 	if (res < 0) {
 		if (res == -ENODATA || res == -EOPNOTSUPP)
 			return NULL;
@@ -123,7 +124,7 @@ static struct ovl_fh *ovl_get_fh(struct dentry *dentry, const char *name)
 	if (!fh)
 		return ERR_PTR(-ENOMEM);
 
-	res = vfs_getxattr(dentry, name, fh, res);
+	res = ovl_do_vfs_getxattr(dentry, name, fh, res);
 	if (res < 0)
 		goto fail;
 
@@ -141,10 +142,11 @@ static struct ovl_fh *ovl_get_fh(struct dentry *dentry, const char *name)
 	return NULL;
 
 fail:
-	pr_warn_ratelimited("overlayfs: failed to get origin (%i)\n", res);
+	pr_warn_ratelimited("overlayfs: failed to get origin (%zi)\n", res);
 	goto out;
 invalid:
-	pr_warn_ratelimited("overlayfs: invalid origin (%*phN)\n", res, fh);
+	pr_warn_ratelimited("overlayfs: invalid origin (%*phN)\n",
+			    (int)res, fh);
 	goto out;
 }
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index c6a8ec049099..72762642b247 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -205,6 +205,8 @@ int ovl_want_write(struct dentry *dentry);
 void ovl_drop_write(struct dentry *dentry);
 struct dentry *ovl_workdir(struct dentry *dentry);
 const struct cred *ovl_override_creds(struct super_block *sb);
+ssize_t ovl_do_vfs_getxattr(struct dentry *dentry, const char *name, void *buf,
+			    size_t size);
 struct super_block *ovl_same_sb(struct super_block *sb);
 int ovl_can_decode_fh(struct super_block *sb);
 struct dentry *ovl_indexdir(struct super_block *sb);
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index f5678a3f8350..bed12aed902c 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -40,6 +40,20 @@ const struct cred *ovl_override_creds(struct super_block *sb)
 	return override_creds(ofs->creator_cred);
 }
 
+ssize_t ovl_do_vfs_getxattr(struct dentry *dentry, const char *name, void *buf,
+			    size_t size)
+{
+	struct xattr_gs_args args = {};
+
+	args.dentry = dentry;
+	args.inode = d_inode(dentry);
+	args.name = name;
+	args.buffer = buf;
+	args.size = size;
+	args.flags = XATTR_NOSECURITY;
+	return __vfs_getxattr(&args);
+}
+
 struct super_block *ovl_same_sb(struct super_block *sb)
 {
 	struct ovl_fs *ofs = sb->s_fs_info;
@@ -537,9 +551,9 @@ void ovl_copy_up_end(struct dentry *dentry)
 
 bool ovl_check_origin_xattr(struct dentry *dentry)
 {
-	int res;
+	ssize_t res;
 
-	res = vfs_getxattr(dentry, OVL_XATTR_ORIGIN, NULL, 0);
+	res = ovl_do_vfs_getxattr(dentry, OVL_XATTR_ORIGIN, NULL, 0);
 
 	/* Zero size value means "copied up but origin unknown" */
 	if (res >= 0)
@@ -550,13 +564,13 @@ bool ovl_check_origin_xattr(struct dentry *dentry)
 
 bool ovl_check_dir_xattr(struct dentry *dentry, const char *name)
 {
-	int res;
+	ssize_t res;
 	char val;
 
 	if (!d_is_dir(dentry))
 		return false;
 
-	res = vfs_getxattr(dentry, name, &val, 1);
+	res = ovl_do_vfs_getxattr(dentry, name, &val, 1);
 	if (res == 1 && val == 'y')
 		return true;
 
@@ -837,13 +851,13 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir)
 /* err < 0, 0 if no metacopy xattr, 1 if metacopy xattr found */
 int ovl_check_metacopy_xattr(struct dentry *dentry)
 {
-	int res;
+	ssize_t res;
 
 	/* Only regular files can have metacopy xattr */
 	if (!S_ISREG(d_inode(dentry)->i_mode))
 		return 0;
 
-	res = vfs_getxattr(dentry, OVL_XATTR_METACOPY, NULL, 0);
+	res = ovl_do_vfs_getxattr(dentry, OVL_XATTR_METACOPY, NULL, 0);
 	if (res < 0) {
 		if (res == -ENODATA || res == -EOPNOTSUPP)
 			return 0;
@@ -852,7 +866,7 @@ int ovl_check_metacopy_xattr(struct dentry *dentry)
 
 	return 1;
 out:
-	pr_warn_ratelimited("overlayfs: failed to get metacopy (%i)\n", res);
+	pr_warn_ratelimited("overlayfs: failed to get metacopy (%zi)\n", res);
 	return res;
 }
 
@@ -878,7 +892,7 @@ ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value,
 	ssize_t res;
 	char *buf = NULL;
 
-	res = vfs_getxattr(dentry, name, NULL, 0);
+	res = ovl_do_vfs_getxattr(dentry, name, NULL, 0);
 	if (res < 0) {
 		if (res == -ENODATA || res == -EOPNOTSUPP)
 			return -ENODATA;
@@ -890,7 +904,7 @@ ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value,
 		if (!buf)
 			return -ENOMEM;
 
-		res = vfs_getxattr(dentry, name, buf, res);
+		res = ovl_do_vfs_getxattr(dentry, name, buf, res);
 		if (res < 0)
 			goto fail;
 	}
-- 
2.23.0.866.gb869b98d4c-goog

Re: [PATCH v14 4/5] overlayfs: internal getxattr operations without sepolicy checking

[Amir Goldstein]

On Tue, Oct 22, 2019 at 11:46 PM Mark Salyzyn <salyzyn@android.com> wrote:
>
> Check impure, opaque, origin & meta xattr with no sepolicy audit
> (using __vfs_getxattr) since these operations are internal to
> overlayfs operations and do not disclose any data.  This became
> an issue for credential override off since sys_admin would have
> been required by the caller; whereas would have been inherently
> present for the creator since it performed the mount.
>
> This is a change in operations since we do not check in the new
> ovl_do_vfs_getxattr function if the credential override is off or
> not.  Reasoning is that the sepolicy check is unnecessary overhead,
> especially since the check can be expensive.
>
> Because for override credentials off, this affects _everyone_ that
> underneath performs private xattr calls without the appropriate
> sepolicy permissions and sys_admin capability.  Providing blanket
> support for sys_admin would be bad for all possible callers.
>
> For the override credentials on, this will affect only the mounter,
> should it lack sepolicy permissions. Not considered a security
> problem since mounting by definition has sys_admin capabilities,
> but sepolicy contexts would still need to be crafted.
>

It sounds reasonable to me, but I am not a "security person".

> It should be noted that there is precedence, __vfs_getxattr is used
> in other filesystems for their own internal trusted xattr management.
>

Urgh! "other" filesystems meaning ecryptfs_getxattr()?
That looks like a loop hole to read any trusted xattr without any
security checks. Not sure its a good example...

> Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: linux-unionfs@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: kernel-team@android.com
> Cc: linux-security-module@vger.kernel.org
>
> ---
> v14 - rebase to use xattr_gs_args.
>
> v13 - rebase to use __vfs_getxattr flags option
>
> v12 - rebase
>
> v11 - switch name to ovl_do_vfs_getxattr, fortify comment
>
> v10 - added to patch series
>
> ---
>  fs/overlayfs/namei.c     | 12 +++++++-----
>  fs/overlayfs/overlayfs.h |  2 ++
>  fs/overlayfs/util.c      | 32 +++++++++++++++++++++++---------
>  3 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index 9702f0d5309d..a4a452c489fa 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -106,10 +106,11 @@ int ovl_check_fh_len(struct ovl_fh *fh, int fh_len)
>
>  static struct ovl_fh *ovl_get_fh(struct dentry *dentry, const char *name)
>  {
> -       int res, err;
> +       ssize_t res;
> +       int err;
>         struct ovl_fh *fh = NULL;
>
> -       res = vfs_getxattr(dentry, name, NULL, 0);
> +       res = ovl_do_vfs_getxattr(dentry, name, NULL, 0);
>         if (res < 0) {
>                 if (res == -ENODATA || res == -EOPNOTSUPP)
>                         return NULL;
> @@ -123,7 +124,7 @@ static struct ovl_fh *ovl_get_fh(struct dentry *dentry, const char *name)
>         if (!fh)
>                 return ERR_PTR(-ENOMEM);
>
> -       res = vfs_getxattr(dentry, name, fh, res);
> +       res = ovl_do_vfs_getxattr(dentry, name, fh, res);
>         if (res < 0)
>                 goto fail;
>
> @@ -141,10 +142,11 @@ static struct ovl_fh *ovl_get_fh(struct dentry *dentry, const char *name)
>         return NULL;
>
>  fail:
> -       pr_warn_ratelimited("overlayfs: failed to get origin (%i)\n", res);
> +       pr_warn_ratelimited("overlayfs: failed to get origin (%zi)\n", res);
>         goto out;
>  invalid:
> -       pr_warn_ratelimited("overlayfs: invalid origin (%*phN)\n", res, fh);
> +       pr_warn_ratelimited("overlayfs: invalid origin (%*phN)\n",
> +                           (int)res, fh);
>         goto out;
>  }
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index c6a8ec049099..72762642b247 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -205,6 +205,8 @@ int ovl_want_write(struct dentry *dentry);
>  void ovl_drop_write(struct dentry *dentry);
>  struct dentry *ovl_workdir(struct dentry *dentry);
>  const struct cred *ovl_override_creds(struct super_block *sb);
> +ssize_t ovl_do_vfs_getxattr(struct dentry *dentry, const char *name, void *buf,
> +                           size_t size);
>  struct super_block *ovl_same_sb(struct super_block *sb);
>  int ovl_can_decode_fh(struct super_block *sb);
>  struct dentry *ovl_indexdir(struct super_block *sb);
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index f5678a3f8350..bed12aed902c 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -40,6 +40,20 @@ const struct cred *ovl_override_creds(struct super_block *sb)
>         return override_creds(ofs->creator_cred);
>  }
>
> +ssize_t ovl_do_vfs_getxattr(struct dentry *dentry, const char *name, void *buf,
> +                           size_t size)
> +{
> +       struct xattr_gs_args args = {};
> +
> +       args.dentry = dentry;
> +       args.inode = d_inode(dentry);
> +       args.name = name;
> +       args.buffer = buf;
> +       args.size = size;
> +       args.flags = XATTR_NOSECURITY;
> +       return __vfs_getxattr(&args);
> +}
> +

We do not understand each other.
I commented on this several times.
please put the wrapper helper ovl_do_getxattr() in overlayfs.h
next to the other ovl_do_ wrapper helpers and add pr_debug()
as all other wrappers have.

Thanks,
Amir.

Re: [PATCH v14 4/5] overlayfs: internal getxattr operations without sepolicy checking

[Mark Salyzyn]

On 10/22/19 11:39 PM, Amir Goldstein wrote:
> On Tue, Oct 22, 2019 at 11:46 PM Mark Salyzyn <salyzyn@android.com> wrote:
>> Check impure, opaque, origin & meta xattr with no sepolicy audit
>> (using __vfs_getxattr) since these operations are internal to
>> overlayfs operations and do not disclose any data.  This became
>> an issue for credential override off since sys_admin would have
>> been required by the caller; whereas would have been inherently
>> present for the creator since it performed the mount.
>>
>> This is a change in operations since we do not check in the new
>> ovl_do_vfs_getxattr function if the credential override is off or
>> not.  Reasoning is that the sepolicy check is unnecessary overhead,
>> especially since the check can be expensive.
>>
>> Because for override credentials off, this affects _everyone_ that
>> underneath performs private xattr calls without the appropriate
>> sepolicy permissions and sys_admin capability.  Providing blanket
>> support for sys_admin would be bad for all possible callers.
>>
>> For the override credentials on, this will affect only the mounter,
>> should it lack sepolicy permissions. Not considered a security
>> problem since mounting by definition has sys_admin capabilities,
>> but sepolicy contexts would still need to be crafted.
>>
> It sounds reasonable to me, but I am not a "security person".
>
>> It should be noted that there is precedence, __vfs_getxattr is used
>> in other filesystems for their own internal trusted xattr management.
>>
> Urgh! "other" filesystems meaning ecryptfs_getxattr()?
> That looks like a loop hole to read any trusted xattr without any
> security checks. Not sure its a good example...

Yes. But it also makes sense since ecryptfs_getxattr is performed inside 
a layer where the security check is done above by the filesystem that 
called it (AFAIK)? This is used by the filesystem, or the security 
layers to pull the actual sepolicy, rather than getting an EPERM and no 
data. The xattr 'data' is needed by the internal layers.

-- Mark

Re: [PATCH v14 1/5] Add flags option to get xattr method paired to __vfs_getxattr

[Amir Goldstein]

[excessive CC list reduced]

On Wed, Oct 23, 2019 at 11:07 AM Andreas Dilger via samba-technical
<samba-technical@lists.samba.org> wrote:
>
>
> On Oct 22, 2019, at 2:44 PM, Mark Salyzyn <salyzyn@android.com> wrote:
> >
> > Replace arguments for get and set xattr methods, and __vfs_getxattr
> > and __vfs_setaxtr functions with a reference to the following now
> > common argument structure:
> >
> > struct xattr_gs_args {
> >       struct dentry *dentry;
> >       struct inode *inode;
> >       const char *name;
> >       union {
> >               void *buffer;
> >               const void *value;
> >       };
> >       size_t size;
> >       int flags;
> > };
>

> > Mark,
> >
> > I do not see the first patch on fsdevel
> > and I am confused from all the suggested APIs
> > I recall Christoph's comment on v8 for not using xattr_gs_args
> > and just adding flags to existing get() method.
> > I agree to that comment.
>
> As already responded, third (?) patch version was like that,

The problem is that because of the waaay too long CC list, most revisions
of the patch and discussion were bounced from fsdevel, most emails
I did not get and cannot find in archives, so the discussion around
them is not productive.

Please resend patch to fsdevel discarding the auto added CC list
of all fs maintainers.

> gregkh@
> said it passed the limit for number of arguments, is looking a bit silly

Well, you just matched get() to set() args list, so this is not a strong
argument IMO.

> (my paraphrase), and that it should be passed as a structure. Two others
> agreed. We gained because both set and get use the same structure after
> this change (this allows a simplified read-modify-write cycle).

That sounds like a nice benefit if this was user API, but are there any
kernel users that intend to make use of that read-modify-write cycle?
I don't think so.

>
> We will need a quorum on this, 3 (structure) to 2 (flag) now (but really
> basically between Greg and Christoph?). Coding style issue: Add a flag,
> or switch to a common xattr argument  structure?
>

IIRC, Christoph was asking why the silly struct and not simply add flags
(as did I). He probably did not see Greg's comments due to the list bounce
issue. If I read your second hand description of Greg's reaction correctly,
it doesn't sound so strong opinionated as well.
Me, I can live with flags or struct - I don't care, but...

Be prepared that if you are going ahead with struct you are going to
suffer from bike shedding, which has already started and you will be
instructed (just now) to also fix all the relevant and missing Documentation.
If, on the other hand, you can get Greg and the rest to concede to adding
flags arg and match get() arg list to set() arg list, you will have a much
easier job and the patch line count, especially in fs code will be *much*
smaller - just saying.

Thanks,
Amir.

Re: [PATCH v14 1/5] Add flags option to get xattr method paired to __vfs_getxattr

[Mark Salyzyn]

On 10/23/19 9:57 PM, Amir Goldstein wrote:
> [excessive CC list reduced]
>
> On Wed, Oct 23, 2019 at 11:07 AM Andreas Dilger via samba-technical
> <samba-technical@lists.samba.org> wrote:
>>
>> On Oct 22, 2019, at 2:44 PM, Mark Salyzyn <salyzyn@android.com> wrote:
>>> Replace arguments for get and set xattr methods, and __vfs_getxattr
>>> and __vfs_setaxtr functions with a reference to the following now
>>> common argument structure:
>>>
>>> struct xattr_gs_args {
>>>        struct dentry *dentry;
>>>        struct inode *inode;
>>>        const char *name;
>>>        union {
>>>                void *buffer;
>>>                const void *value;
>>>        };
>>>        size_t size;
>>>        int flags;
>>> };
>>> Mark,
>>>
>>> I do not see the first patch on fsdevel
>>> and I am confused from all the suggested APIs
>>> I recall Christoph's comment on v8 for not using xattr_gs_args
>>> and just adding flags to existing get() method.
>>> I agree to that comment.
>> As already responded, third (?) patch version was like that,
> The problem is that because of the waaay too long CC list, most revisions
> of the patch and discussion were bounced from fsdevel, most emails
> I did not get and cannot find in archives, so the discussion around
> them is not productive.
>
> Please resend patch to fsdevel discarding the auto added CC list
> of all fs maintainers.

git send-email is not my friend :-(

>> gregkh@
>> said it passed the limit for number of arguments, is looking a bit silly
> Well, you just matched get() to set() args list, so this is not a strong
> argument IMO.
>
>> (my paraphrase), and that it should be passed as a structure. Two others
>> agreed. We gained because both set and get use the same structure after
>> this change (this allows a simplified read-modify-write cycle).
> That sounds like a nice benefit if this was user API, but are there any
> kernel users that intend to make use of that read-modify-write cycle?
> I don't think so.
(one user)
>
>> We will need a quorum on this, 3 (structure) to 2 (flag) now (but really
>> basically between Greg and Christoph?). Coding style issue: Add a flag,
>> or switch to a common xattr argument  structure?
>>
> IIRC, Christoph was asking why the silly struct and not simply add flags
> (as did I). He probably did not see Greg's comments due to the list bounce
> issue. If I read your second hand description of Greg's reaction correctly,
> it doesn't sound so strong opinionated as well.
> Me, I can live with flags or struct - I don't care, but...
>
> Be prepared that if you are going ahead with struct you are going to
> suffer from bike shedding, which has already started and you will be
> instructed (just now) to also fix all the relevant and missing Documentation.
> If, on the other hand, you can get Greg and the rest to concede to adding
> flags arg and match get() arg list to set() arg list, you will have a much
> easier job and the patch line count, especially in fs code will be *much*
> smaller - just saying.

Respining back to the v4 version of the series incorporating some of the 
fixes on the way.

Automated testing in kernel not yet handled and will be noted in the 
respin.

> Thanks,
> Amir.

Mark