[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#33644: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL extended attr
From: |
Kamil Dudka |
Subject: |
bug#33644: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL extended attributes |
Date: |
Mon, 11 Feb 2019 12:50:11 +0100 |
On Monday, February 11, 2019 6:07:18 AM CET Pádraig Brady wrote:
> On 06/12/18 05:08, Kamil Dudka wrote:
> > ... which cannot be preserved by other means
> >
> > Bug: https://bugzilla.redhat.com/1031423#c4
> > ---
> >
> > src/copy.c | 22 +++++++++++++++++-----
> > 1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/copy.c b/src/copy.c
> > index 3221b9997..754c5e1aa 100644
> > --- a/src/copy.c
> > +++ b/src/copy.c
> > @@ -640,6 +640,17 @@ copy_attr_free (struct error_context *ctx _GL_UNUSED,
> >
> > {
> > }
> >
> > +/* Include NFSv4 ACL extended attributes, which cannot be preserved by
> > + other means. Otherwise honor attributes configured for exclusion
> > + in /etc/xattr.conf. Return zero to skip. */
> > +static int
> > +check_not_nfs4_acl (const char *name, struct error_context *ctx)
> > +{
> > + return attr_copy_check_permissions(name, ctx)
> > + || !STRNCMP_LIT (name, "system.nfs4_acl")
> > + || !STRNCMP_LIT (name, "system.nfs4acl");
> > +}
> > +
> >
> > /* Exclude SELinux extended attributes that are otherwise handled,
> >
> > and are problematic to copy again. Also honor attributes
> > configured for exclusion in /etc/xattr.conf.
> >
> > @@ -649,7 +660,7 @@ static int
> >
> > check_selinux_attr (const char *name, struct error_context *ctx)
> > {
> >
> > return STRNCMP_LIT (name, "security.selinux")
> >
> > - && attr_copy_check_permissions (name, ctx);
> > + && check_not_nfs4_acl (name, ctx);
> >
> > }
> >
> > /* If positive SRC_FD and DST_FD descriptors are passed,
> >
> > @@ -663,6 +674,9 @@ copy_attr (char const *src_path, int src_fd,
> >
> > bool all_errors = (!x->data_copy_required ||
> > x->require_preserve_xattr);
> > bool some_errors = (!all_errors && !x->reduce_diagnostics);
> > bool selinux_done = (x->preserve_security_context ||
> > x->set_security_context);>
> > + int (*check) (const char *, struct error_context *) = (selinux_done)
> > + ? check_selinux_attr
> > + : check_not_nfs4_acl;
> >
> > struct error_context ctx =
> > {
> >
> > .error = all_errors ? copy_attr_allerror : copy_attr_error,
> >
> > @@ -670,12 +684,10 @@ copy_attr (char const *src_path, int src_fd,
> >
> > .quote_free = copy_attr_free
> >
> > };
> > if (0 <= src_fd && 0 <= dst_fd)
> >
> > - ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd,
> > - selinux_done ? check_selinux_attr : NULL,
> > + ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd, check,
> >
> > (all_errors || some_errors ? &ctx : NULL));
> >
> > else
> >
> > - ret = attr_copy_file (src_path, dst_path,
> > - selinux_done ? check_selinux_attr : NULL,
> > + ret = attr_copy_file (src_path, dst_path, check,
> >
> > (all_errors || some_errors ? &ctx : NULL));
> >
> > return ret == 0;
>
> This patch is confusing to read, though looks functional.
I can submit deduplication of the `selinux_done ? check_selinux_attr : NULL`
code as a separate patch if you prefer it.
> It's clearer of you rename check_not_nfs4_acl() to
> check_but_allow_nfs4_acl().
Fine by me.
> So in summary, any xattr in /etc/xattr.conf is _not_ copied.
> You want to essentially ignore the nfs4 entries in that config file.
> So why not just remove the entries from that file?
See how xattr.conf is documented:
# Actions:
# permissions - copy when trying to preserve permissions.
# skip - do not copy.
The fact that coreutils handles `persmissions` equally as `skip` is IMO a
problem of coreutils, not a problem of xattr.conf.
> Is that something that could be done in attr.git?
I think that the information in xattr.conf is correct. system.nfs4_acl is
really an attribute one wants to copy when trying to preserve permissions.
> Why would one want to treat nfs4 attrs differently to the posix_acl_access
> attrs?
It was written in the commit message. One can use `cp --preserve=mode`
to preserve POSIX ACLs whereas the only way to preserve NFSv4 ACLs was
`cp --preserve=xattr`.
> thanks,
> Pádraig.
On Monday, February 11, 2019 6:21:49 AM CET Pádraig Brady wrote:
> BTW is there anything interesting behind this paywall I can't access?
> https://access.redhat.com/solutions/115043
It just says that `cp a b` does not preserve NFSv4 ACLs whereas `cp -a a b`,
`cp --preserve=all a b`, or `cp --preserve=xattr a b` does. Unfortunately,
this is currently true only for Red Hat Enterprise Linux.
Kamil