From: "J. Bruce Fields" <bfields@fieldses.org>
To: Philippe Troin <phil@coldcreektech.com>
Cc: Christoph Hellwig <hch@infradead.org>,
Trond Myklebust <trond.myklebust@primarydata.com>,
Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
Linux Kernel mailing list <linux-kernel@vger.kernel.org>
Subject: Re: Phantom ACL-related xattrs on 3.14.4 NFS client
Date: Mon, 9 Jun 2014 10:46:45 -0400 [thread overview]
Message-ID: <20140609144645.GB8390@fieldses.org> (raw)
In-Reply-To: <1402195701.12743.18.camel@ceramic.home.fifi.org>
On Sat, Jun 07, 2014 at 07:48:21PM -0700, Philippe Troin wrote:
> Hi Trond & Christoph,
>
> It's still broken, but in a different way.
> The phantom attrs are gone, but the attr/acl interaction is still
> uncertain.
>
> I have tested vanilla 3.14.5 + this patch on x86_64.
> Mount options are the same as last time (NFSv3).
>
> This is what I see on the client:
>
> nfsv3client% mkdir x
> nfsv3client% cd x
> nfsv3client% getfattr -m '.*' .
> nfsv3client% getfacl .
> # file: .
> # owner: phil
> # group: phil
> user::rwx
> group::rwx
> other::r-x
>
> OK so far: no more phantom attrs.
> This is where things get dodgy:
>
> nfsv3client% setfacl -m u:root:r .
> nfsv3client% getfacl .
> # file: .
> # owner: phil
> # group: phil
> user::rwx
> user:root:r--
> group::rwx
> mask::rwx
> other::r-x
>
> nfsv3client% getfattr -m '.*' .
> [1] 2123 segmentation fault getfattr -m '.*' .
Is there a backtrace or anything in the system logs?
--b.
> % strace getfattr -m '.*' . 2>&1 | tail -n 20
> fstat(3, {st_mode=S_IFREG|0644, st_size=26254, ...}) = 0
> mmap(NULL, 26254, PROT_READ, MAP_SHARED, 3, 0) = 0x7f46a1450000
> close(3) = 0
> getrlimit(RLIMIT_NOFILE, {rlim_cur=1024, rlim_max=4*1024}) = 0
> lstat(".", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0
> listxattr(".", NULL, 0) = 23
> listxattr(".", "system.posix_acl_access", 256) = 23
> brk(0) = 0x1138000
> brk(0x1178000) = 0x1178000
> brk(0) = 0x1178000
> brk(0) = 0x1178000
> brk(0x1159000) = 0x1159000
> brk(0) = 0x1159000
> mmap(NULL, 266240, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f46a140f000
> brk(0) = 0x1159000
> brk(0) = 0x1159000
> brk(0x1139000) = 0x1139000
> brk(0) = 0x1139000
> --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x11586e8} ---
> +++ killed by SIGSEGV +++
> [1] 2311 segmentation fault strace getfattr -m '.*' . 2>&1
> |
> 2312 done tail -n 20
>
> I have no idea get getfattr crashes right after the listxattr() syscall,
> but it surely doesn't on the NFSv3 server nor with 3.13.
> A quick check on the NFS server shows the the ACLs are correctly set:
>
> nfsv3server% cd /path/to/x
> nfsv3server% getfacl .
> # file: .
> # owner: phil
> # group: phil
> user::rwx
> user:root:r--
> group::rwx
> mask::rwx
> other::r-x
>
> nfsv3server% getfattr -m '.*' .
> # file: .
> system.posix_acl_access
>
> Back on the client, clearing the ACL confuses the client further:
>
> nfsv3client% setfacl -b .
> nfsv3client% getfacl .
> # file: .
> # owner: phil
> # group: phil
> user::rwx
> group::rwx
> other::r-x
>
> nfsv3client% strace getfattr -m '.*' . 2>&1 | tail -n 20
> fstat(3, {st_mode=S_IFREG|0644, st_size=26254, ...}) = 0
> mmap(NULL, 26254, PROT_READ, MAP_SHARED, 3, 0) = 0x7fc7e3f9a000
> close(3) = 0
> getrlimit(RLIMIT_NOFILE, {rlim_cur=1024, rlim_max=4*1024}) = 0
> lstat(".", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0
> listxattr(".", NULL, 0) = 23
> listxattr(".", "system.posix_acl_access", 256) = 23
> brk(0) = 0x1655000
> brk(0x1695000) = 0x1695000
> brk(0) = 0x1695000
> brk(0) = 0x1695000
> brk(0x1676000) = 0x1676000
> brk(0) = 0x1676000
> mmap(NULL, 266240, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fc7e3f59000
> brk(0) = 0x1676000
> brk(0) = 0x1676000
> brk(0x1656000) = 0x1656000
> brk(0) = 0x1656000
> --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x16756e8} ---
> +++ killed by SIGSEGV +++
> [1] 2353 segmentation fault strace getfattr -m '.*' . 2>&1
> |
> 2354 done tail -n 20
> nfsv3client% getfattr -n system.posix_acl_access .
> # file: .
> system.posix_acl_access=0sAgAAAAEABwD/////BAAHAP////8gAAUA/////w==
>
> See how:
> * getfacl says there's no ACLs
> * getfattr says there's still a system.posix_acl_access attr.
> Interestingly, the server says otherwise:
>
> nfsv3server% getfattr -m '.*' .
> nfsv3server% getfattr -n system.posix_acl_access .
> .: system.posix_acl_access: No such attribute
> [1] 2233 exit 1 getfattr -n system.posix_acl_access .
> nfsv3server% getfacl .
> # file: .
> # owner: phil
> # group: phil
> user::rwx
> group::rwx
> other::r-x
>
> Phil.
>
> On Sat, 2014-06-07 at 07:04 -0700, Christoph Hellwig wrote:
> > On Fri, Jun 06, 2014 at 04:37:03PM -0400, Trond Myklebust wrote:
> > > Christoph, what is the intended interface for telling
> > > posix_acl_xattr_list() that there are no acls on a particular file?
> > > Should there perhaps be a call to get_acl()?
> >
> > The interface is to not call posix_acl_xattr_list unless you have ACLs.
> > Every implementation does this, except for generic_listxattr which is
> > only used by NFS.
> >
> > Philippe, can you test the patch below?
> >
> >
> > diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
> > index 871d6ed..e083827 100644
> > --- a/fs/nfs/nfs3acl.c
> > +++ b/fs/nfs/nfs3acl.c
> > @@ -247,3 +247,45 @@ const struct xattr_handler *nfs3_xattr_handlers[] = {
> > &posix_acl_default_xattr_handler,
> > NULL,
> > };
> > +
> > +static int
> > +nfs3_list_one_acl(struct inode *inode, int type, const char *name, void *data,
> > + size_t size, ssize_t *result)
> > +{
> > + struct posix_acl *acl;
> > + char *p = data + *result;
> > +
> > + acl = get_acl(inode, type);
> > + if (!acl)
> > + return 0;
> > +
> > + posix_acl_release(acl);
> > +
> > + *result += strlen(name);
> > + if (!size)
> > + return 0;
> > + if (*result > size)
> > + return -ERANGE;
> > +
> > + strcpy(p, name);
> > + return 0;
> > +}
> > +
> > +ssize_t
> > +nfs3_listxattr(struct dentry *dentry, char *data, size_t size)
> > +{
> > + struct inode *inode = dentry->d_inode;
> > + ssize_t result = 0;
> > + int error;
> > +
> > + error = nfs3_list_one_acl(inode, ACL_TYPE_ACCESS,
> > + POSIX_ACL_XATTR_ACCESS, data, size, &result);
> > + if (error)
> > + return error;
> > +
> > + error = nfs3_list_one_acl(inode, ACL_TYPE_DEFAULT,
> > + POSIX_ACL_XATTR_DEFAULT, data, size, &result);
> > + if (error)
> > + return error;
> > + return result;
> > +}
> > diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> > index db60149..0e2bb26 100644
> > --- a/fs/nfs/nfs3proc.c
> > +++ b/fs/nfs/nfs3proc.c
> > @@ -891,7 +891,7 @@ static const struct inode_operations nfs3_dir_inode_operations = {
> > .getattr = nfs_getattr,
> > .setattr = nfs_setattr,
> > #ifdef CONFIG_NFS_V3_ACL
> > - .listxattr = generic_listxattr,
> > + .listxattr = nfs3_listxattr,
> > .getxattr = generic_getxattr,
> > .setxattr = generic_setxattr,
> > .removexattr = generic_removexattr,
> > @@ -905,7 +905,7 @@ static const struct inode_operations nfs3_file_inode_operations = {
> > .getattr = nfs_getattr,
> > .setattr = nfs_setattr,
> > #ifdef CONFIG_NFS_V3_ACL
> > - .listxattr = generic_listxattr,
> > + .listxattr = nfs3_listxattr,
> > .getxattr = generic_getxattr,
> > .setxattr = generic_setxattr,
> > .removexattr = generic_removexattr,
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2014-06-09 14:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-06 19:29 Philippe Troin
2014-06-06 20:37 ` Trond Myklebust
2014-06-07 14:04 ` Christoph Hellwig
2014-06-08 2:48 ` Philippe Troin
2014-06-09 14:46 ` J. Bruce Fields [this message]
2014-06-09 15:58 ` Philippe Troin
2014-06-10 21:20 ` Philippe Troin
2014-06-11 7:24 ` Christoph Hellwig
2014-06-11 16:15 ` Philippe Troin
2014-06-11 16:22 ` Christoph Hellwig
2014-06-17 23:48 ` Philippe Troin
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=20140609144645.GB8390@fieldses.org \
--to=bfields@fieldses.org \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=phil@coldcreektech.com \
--cc=trond.myklebust@primarydata.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
all inboxes | Powered by JetHome®