All of lore.kernel.org
 help / color / mirror / Atom feed
From: "bfields@fieldses.org" <bfields@fieldses.org>
To: Trond Myklebust <trondmy@hammerspace.com>
Cc: "guy@vastdata.com" <guy@vastdata.com>,
	"schumakeranna@gmail.com" <schumakeranna@gmail.com>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: [PATCH] nfs: we don't support removing system.nfs4_acl
Date: Thu, 11 Feb 2021 13:54:44 -0500	[thread overview]
Message-ID: <20210211185444.GA6048@fieldses.org> (raw)
In-Reply-To: <20210208220855.GA15116@fieldses.org>

From: "J. Bruce Fields" <bfields@redhat.com>

The contents of the system.nfs4_acl xattr are literally just the
xdr-encoded ACL attribute.  That attribute starts with a 4-byte integer
representing the number of ACEs in the ACL.  So even a zero-ACE ACL will
be at least 4 bytes.

We've never actually bothered to sanity-check the ACL encoding that
userspace gives us.  The only problem that causes is that we return an
error that's probably wrong.  (The server will return BADXDR, which
we'll translate to EIO, when EINVAL would make more sense.)

It's not much a problem in practice since the standard utilities give us
well-formed XDR.  The one case we're likely to see from userspace in
practice is a set of a zero-length xattr since that's how

	removexattr(path, "system.nfs4_acl")

is implemented.  It's worth trying to give a better error for that case.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfs/nfs4proc.c | 6 ++++++
 1 file changed, 6 insertions(+)

On Mon, Feb 08, 2021 at 05:08:55PM -0500, bfields@fieldses.org wrote:
> On Mon, Feb 08, 2021 at 07:31:38PM +0000, Trond Myklebust wrote:
> > OK. So you're not really saying that the SETATTR has a zero length
> > body, but that the ACL attribute in this case has a zero length body,
> > whereas in the 'empty acl' case, it is supposed to have a body
> > containing a zero-length nfsace4<> array. Fair enough.
> 
> Yep!  I'll see if I can think of a helpful concise comment, and resend.

Oops, forgot about this, here you go.--b.

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 2f4679a62712..86e87f7d7686 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5895,6 +5895,12 @@ static int __nfs4_proc_set_acl(struct inode *inode, const void *buf, size_t bufl
 	unsigned int npages = DIV_ROUND_UP(buflen, PAGE_SIZE);
 	int ret, i;
 
+	/*
+	 * We don't support removing system.nfs4_acl, and even a
+	 * 0-length ACL needs at least 4 bytes for the number of ACEs:
+	 */
+	if (buflen < 4)
+		return -EINVAL;
 	if (!nfs4_server_supports_acls(server))
 		return -EOPNOTSUPP;
 	if (npages > ARRAY_SIZE(pages))
-- 
2.29.2


  reply	other threads:[~2021-02-11 19:02 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-28 22:36 [PATCH] nfs: we don't support removing system.nfs4_acl J. Bruce Fields
     [not found] ` <95e5f9e4-76d4-08c4-ece3-35a10c06073b@vastdata.com>
2021-01-29  1:37   ` Trond Myklebust
2021-01-29  2:35     ` bfields
2021-01-29  2:50       ` bfields
2021-01-31 20:41         ` Trond Myklebust
2021-01-31 21:58           ` bfields
2021-02-03 20:07             ` bfields
2021-02-08 19:31               ` Trond Myklebust
2021-02-08 22:08                 ` bfields
2021-02-11 18:54                   ` bfields [this message]
2021-03-04  2:30                     ` Murphy Zhou
2021-03-12 15:43                       ` Anna Schumaker

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=20210211185444.GA6048@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=guy@vastdata.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=schumakeranna@gmail.com \
    --cc=trondmy@hammerspace.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.