From patchwork Mon Feb 3 20:22:15 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Trond Myklebust X-Patchwork-Id: 316305 Return-Path: X-Original-To: incoming-imx@patchwork.ozlabs.org Delivered-To: patchwork-incoming-imx@bilbo.ozlabs.org Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:770:15f::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 98C122C0089 for ; Tue, 4 Feb 2014 07:23:05 +1100 (EST) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WAQ2Z-0003g7-7L; Mon, 03 Feb 2014 20:22:47 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WAQ2W-0008Mv-H8; Mon, 03 Feb 2014 20:22:44 +0000 Received: from mail-ig0-f170.google.com ([209.85.213.170]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WAQ2T-0008Jp-17 for linux-arm-kernel@lists.infradead.org; Mon, 03 Feb 2014 20:22:41 +0000 Received: by mail-ig0-f170.google.com with SMTP id m12so6505206iga.1 for ; Mon, 03 Feb 2014 12:22:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:organization:content-type:mime-version :content-transfer-encoding; bh=/8PDRCvJdCs3NZNNKwmgV8ookyrJbsdJD0Q0Mm+Nzx0=; b=elfLg7LfSljZQUv9ymihN/dcFim+6lLMA4zcXB6v6tCst+D8f53n5LnvhRy0eseN4b M3N4Xvr6lKeUSNmv8JYoyNqLj1u7YiNGRijffhrLrUKrpd42oIkWeRZAGpKJOb3KyHyB M+5FFq6aPIaSAPkJZvoqfw3Jrl+dwhbZR5aVxqNYG2MIcmdMqyw9MCbBeafZVfhKEprK K2OZHwiR/rZrS/zf5y2GHyQ/aXKL4f0wX/ZkoNPVoQaNNy5xVnKbQxkSjwPcKIzYd7Fi N6V5I4nKsrCr6S7W4w0TLEwghPzFVRS1QQNAN82QRHmg7pIHm+uVWWQGQIpvbDyGeAnL qQJA== X-Gm-Message-State: ALoCoQkXKh4OFdBTdEqXBngsTvgqG1+Nu7j/qGzzxUKCanEyGbGLNY9cb4MAq30B5ex+7jVhfzm4 X-Received: by 10.50.29.49 with SMTP id g17mr13598634igh.35.1391458937731; Mon, 03 Feb 2014 12:22:17 -0800 (PST) Received: from [172.16.74.153] (c-98-209-19-95.hsd1.mi.comcast.net. [98.209.19.95]) by mx.google.com with ESMTPSA id kb10sm33969894igb.6.2014.02.03.12.22.16 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Mon, 03 Feb 2014 12:22:17 -0800 (PST) Message-ID: <1391458935.17089.1.camel@leira.trondhjem.org> Subject: Re: NFS client broken in Linus' tip From: Trond Myklebust To: Christoph Hellwig Date: Mon, 03 Feb 2014 15:22:15 -0500 In-Reply-To: <3003D7E5-93F8-4B32-ACDB-07ED3F6CE70D@primarydata.com> References: <20140130140834.GW15937@n2100.arm.linux.org.uk> <20140130141405.GA23985@infradead.org> <20140130142752.GX15937@n2100.arm.linux.org.uk> <20140130143208.GB9573@infradead.org> <20140130153812.GA15937@n2100.arm.linux.org.uk> <1391201970.6978.1.camel@leira.trondhjem.org> <20140203080325.GB806@infradead.org> <85AAFCF5-60EE-42E5-B103-37A4613C5947@primarydata.com> <20140203145759.GA30263@infradead.org> <3003D7E5-93F8-4B32-ACDB-07ED3F6CE70D@primarydata.com> Organization: Primary Data X-Mailer: Evolution 3.10.3 (3.10.3-1.fc20) Mime-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140203_152241_144522_FA46C833 X-CRM114-Status: GOOD ( 27.76 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.213.170 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: linuxnfs , Russell King - ARM Linux , Linux Kernel Mailing List , Viro Alexander , Christoph Hellwig , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org List-Id: linux-imx-kernel.lists.patchwork.ozlabs.org On Mon, 2014-02-03 at 10:45 -0500, Trond Myklebust wrote: > On Feb 3, 2014, at 9:57, Christoph Hellwig wrote: > > > On Mon, Feb 03, 2014 at 09:17:30AM -0500, Trond Myklebust wrote: > >> As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP). > > > > Is it really the wrong answer? How does userspace care wether this > > server doesn't support ACLs at all or none is set? The resulting > > behavior is the same. > > It will certainly cause acl_get_file() to behave differently than previously. I’ve no idea how that will affect applications, though. > > > If there's a good reason to care we might have to go with your patch, > > but if we can avoid it I'd prefer to keep things simple. > > One alternative is to simply wrap posix_acl_xattr_get() in fs/nfs/nfs3acl.c, and have it check the value of nfs_server_capable(inode, NFS_CAP_ACLS) before returning ENODATA. That’s rather ugly too... FWIW, here is the alternative patch. I've tested it, and it seems to work. 8<--------------------------------------------------------------------- From 2e527b12169a67e9cfcf43898ae4d15bcfa1ede9 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Mon, 3 Feb 2014 13:07:05 -0500 Subject: [PATCH] NFSv3: Fix return values for get_acl() Ensure that nfs3_get_acl() returns NULL when the server doesn't support POSIX acls. This ensures that posix_acl_create() does the right thing, and applies the current_umask() to the mode before returning. Add a wrapper around posix_acl_xattr_get() so that we continue to return EOPNOTSUPP when the server doesn't support POSIX acls. Otherwise, the NULL return from nfs3_get_acl() will cause ENODATA to be returned. Also add the appropriate exports to posix_acl_xattr_get, posix_acl_xattr_set and posix_acl_xattr_list to enable their use in the wrapper. Reported-by: Russell King Link: http://lkml.kernel.org/r/20140130140834.GW15937@n2100.arm.linux.org.uk Cc: Christoph Hellwig Cc: Al Viro viro@zeniv.linux.org.uk> Signed-off-by: Trond Myklebust --- fs/nfs/nfs3acl.c | 42 ++++++++++++++++++++++++++++++++++++----- fs/posix_acl.c | 9 ++++++--- include/linux/posix_acl_xattr.h | 8 ++++++++ 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c index 871d6eda8dba..d1bc84f22f64 100644 --- a/fs/nfs/nfs3acl.c +++ b/fs/nfs/nfs3acl.c @@ -28,8 +28,10 @@ struct posix_acl *nfs3_get_acl(struct inode *inode, int type) }; int status, count; - if (!nfs_server_capable(inode, NFS_CAP_ACLS)) - return ERR_PTR(-EOPNOTSUPP); + if (!nfs_server_capable(inode, NFS_CAP_ACLS)) { + cache_no_acl(inode); + return NULL; + } status = nfs_revalidate_inode(server, inode); if (status < 0) @@ -70,7 +72,7 @@ struct posix_acl *nfs3_get_acl(struct inode *inode, int type) dprintk("NFS_V3_ACL extension not supported; disabling\n"); server->caps &= ~NFS_CAP_ACLS; case -ENOTSUPP: - status = -EOPNOTSUPP; + status = 0; default: goto getout; } @@ -242,8 +244,38 @@ fail: return PTR_ERR(alloc); } +static int +nfs_posix_acl_xattr_get(struct dentry *dentry, const char *name, + void *value, size_t size, int type) +{ + int ret; + + ret = posix_acl_xattr_get(dentry, name, value, size, type); + /* + * This check is needed to override the ENODATA error that + * posix_acl_xattr_get will return if the acl probe fails. + */ + if (!nfs_server_capable(dentry->d_inode, NFS_CAP_ACLS)) + ret = -EOPNOTSUPP; + return ret; +} + +static const struct xattr_handler nfs_posix_acl_access_xattr_handler = { + .prefix = POSIX_ACL_XATTR_ACCESS, + .flags = ACL_TYPE_ACCESS, + .get = nfs_posix_acl_xattr_get, + .set = posix_acl_xattr_set, +}; + +static const struct xattr_handler nfs_posix_acl_default_xattr_handler = { + .prefix = POSIX_ACL_XATTR_DEFAULT, + .flags = ACL_TYPE_DEFAULT, + .get = nfs_posix_acl_xattr_get, + .set = posix_acl_xattr_set, +}; + const struct xattr_handler *nfs3_xattr_handlers[] = { - &posix_acl_access_xattr_handler, - &posix_acl_default_xattr_handler, + &nfs_posix_acl_access_xattr_handler, + &nfs_posix_acl_default_xattr_handler, NULL, }; diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 38bae5a0ea25..835167f92952 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -750,7 +750,7 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl, } EXPORT_SYMBOL (posix_acl_to_xattr); -static int +int posix_acl_xattr_get(struct dentry *dentry, const char *name, void *value, size_t size, int type) { @@ -773,8 +773,9 @@ posix_acl_xattr_get(struct dentry *dentry, const char *name, return error; } +EXPORT_SYMBOL_GPL(posix_acl_xattr_get); -static int +int posix_acl_xattr_set(struct dentry *dentry, const char *name, const void *value, size_t size, int flags, int type) { @@ -809,8 +810,9 @@ out: posix_acl_release(acl); return ret; } +EXPORT_SYMBOL_GPL(posix_acl_xattr_set); -static size_t +size_t posix_acl_xattr_list(struct dentry *dentry, char *list, size_t list_size, const char *name, size_t name_len, int type) { @@ -832,6 +834,7 @@ posix_acl_xattr_list(struct dentry *dentry, char *list, size_t list_size, memcpy(list, xname, size); return size; } +EXPORT_SYMBOL_GPL(posix_acl_xattr_list); const struct xattr_handler posix_acl_access_xattr_handler = { .prefix = POSIX_ACL_XATTR_ACCESS, diff --git a/include/linux/posix_acl_xattr.h b/include/linux/posix_acl_xattr.h index 6f14ee295822..fc062003a456 100644 --- a/include/linux/posix_acl_xattr.h +++ b/include/linux/posix_acl_xattr.h @@ -69,6 +69,14 @@ struct posix_acl *posix_acl_from_xattr(struct user_namespace *user_ns, int posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl, void *buffer, size_t size); +int posix_acl_xattr_get(struct dentry *dentry, const char *name, + void *value, size_t size, int type); +int posix_acl_xattr_set(struct dentry *dentry, const char *name, + const void *value, size_t size, int flags, int type); +size_t posix_acl_xattr_list(struct dentry *dentry, char *list, + size_t list_size, const char *name, + size_t name_len, int type); + extern const struct xattr_handler posix_acl_access_xattr_handler; extern const struct xattr_handler posix_acl_default_xattr_handler;