| Message ID | 20200623223904.31643-13-fllinden@amazon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | client side user xattr (RFC8276) support | expand |
Hi, On Wed, Jun 24, 2020 at 6:51 AM Frank van der Linden <fllinden@amazon.com> wrote: > > Now that all the lower level code is there to make the RPC calls, hook > it in to the xattr handlers and the listxattr entry point, to make them > available. > > Signed-off-by: Frank van der Linden <fllinden@amazon.com> > --- > fs/nfs/nfs4proc.c | 123 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 121 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 0fbd2925a828..92a07956f07b 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -66,6 +66,7 @@ > #include "nfs4idmap.h" > #include "nfs4session.h" > #include "fscache.h" > +#include "nfs42.h" > > #include "nfs4trace.h" > > @@ -7440,6 +7441,103 @@ nfs4_listxattr_nfs4_label(struct inode *inode, char *list, size_t list_len) > > #endif > > +#ifdef CONFIG_NFS_V4_2 > +static int nfs4_xattr_set_nfs4_user(const struct xattr_handler *handler, > + struct dentry *unused, struct inode *inode, > + const char *key, const void *buf, > + size_t buflen, int flags) > +{ > + struct nfs_access_entry cache; > + > + if (!nfs_server_capable(inode, NFS_CAP_XATTR)) > + return -EOPNOTSUPP; > + > + /* > + * There is no mapping from the MAY_* flags to the NFS_ACCESS_XA* > + * flags right now. Handling of xattr operations use the normal > + * file read/write permissions. > + * > + * Just in case the server has other ideas (which RFC 8276 allows), > + * do a cached access check for the XA* flags to possibly avoid > + * doing an RPC and getting EACCES back. > + */ > + if (!nfs_access_get_cached(inode, current_cred(), &cache, true)) { > + if (!(cache.mask & NFS_ACCESS_XAWRITE)) > + return -EACCES; > + } > + > + if (buf == NULL) > + return nfs42_proc_removexattr(inode, key); > + else > + return nfs42_proc_setxattr(inode, key, buf, buflen, flags); > +} > + > +static int nfs4_xattr_get_nfs4_user(const struct xattr_handler *handler, > + struct dentry *unused, struct inode *inode, > + const char *key, void *buf, size_t buflen) > +{ > + struct nfs_access_entry cache; > + > + if (!nfs_server_capable(inode, NFS_CAP_XATTR)) > + return -EOPNOTSUPP; > + > + if (!nfs_access_get_cached(inode, current_cred(), &cache, true)) { > + if (!(cache.mask & NFS_ACCESS_XAREAD)) > + return -EACCES; > + } > + > + return nfs42_proc_getxattr(inode, key, buf, buflen); > +} > + > +static ssize_t > +nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len) > +{ > + u64 cookie; > + bool eof; > + int ret, size; > + char *buf; > + size_t buflen; > + struct nfs_access_entry cache; > + > + if (!nfs_server_capable(inode, NFS_CAP_XATTR)) > + return 0; > + > + if (!nfs_access_get_cached(inode, current_cred(), &cache, true)) { > + if (!(cache.mask & NFS_ACCESS_XALIST)) > + return 0; > + } > + > + cookie = 0; > + eof = false; > + buflen = list_len ? list_len : XATTR_LIST_MAX; > + buf = list_len ? list : NULL; > + size = 0; > + > + while (!eof) { > + ret = nfs42_proc_listxattrs(inode, buf, buflen, > + &cookie, &eof); > + if (ret < 0) > + return ret; > + > + if (list_len) { > + buf += ret; > + buflen -= ret; > + } > + size += ret; > + } > + > + return size; > +} > + > +#else > + > +static ssize_t > +nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len) > +{ > + return 0; > +} > +#endif /* CONFIG_NFS_V4_2 */ > + > /* > * nfs_fhget will use either the mounted_on_fileid or the fileid > */ > @@ -10045,7 +10143,7 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = { > > static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size) > { > - ssize_t error, error2; > + ssize_t error, error2, error3; > > error = generic_listxattr(dentry, list, size); > if (error < 0) > @@ -10058,7 +10156,17 @@ static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size) > error2 = nfs4_listxattr_nfs4_label(d_inode(dentry), list, size); > if (error2 < 0) > return error2; > - return error + error2; > + > + if (list) { > + list += error2; > + size -= error2; > + } > + > + error3 = nfs4_listxattr_nfs4_user(d_inode(dentry), list, size); > + if (error3 < 0) > + return error3; > + > + return error + error2 + error3; > } > > static const struct inode_operations nfs4_dir_inode_operations = { > @@ -10146,10 +10254,21 @@ static const struct xattr_handler nfs4_xattr_nfs4_acl_handler = { > .set = nfs4_xattr_set_nfs4_acl, > }; > > +#ifdef CONFIG_NFS_V4_2 > +static const struct xattr_handler nfs4_xattr_nfs4_user_handler = { > + .prefix = XATTR_USER_PREFIX, > + .get = nfs4_xattr_get_nfs4_user, > + .set = nfs4_xattr_set_nfs4_user, > +}; > +#endif > + Any plan to support XATTR_TRUSTED_PREFIX ? Thanks. > const struct xattr_handler *nfs4_xattr_handlers[] = { > &nfs4_xattr_nfs4_acl_handler, > #ifdef CONFIG_NFS_V4_SECURITY_LABEL > &nfs4_xattr_nfs4_label_handler, > +#endif > +#ifdef CONFIG_NFS_V4_2 > + &nfs4_xattr_nfs4_user_handler, > #endif > NULL > }; > -- > 2.17.2 >
On Fri, Aug 21, 2020 at 02:50:59PM +0800, Murphy Zhou wrote: > Hi, > > On Wed, Jun 24, 2020 at 6:51 AM Frank van der Linden > <fllinden@amazon.com> wrote: [...] > > static const struct inode_operations nfs4_dir_inode_operations = { > > @@ -10146,10 +10254,21 @@ static const struct xattr_handler nfs4_xattr_nfs4_acl_handler = { > > .set = nfs4_xattr_set_nfs4_acl, > > }; > > > > +#ifdef CONFIG_NFS_V4_2 > > +static const struct xattr_handler nfs4_xattr_nfs4_user_handler = { > > + .prefix = XATTR_USER_PREFIX, > > + .get = nfs4_xattr_get_nfs4_user, > > + .set = nfs4_xattr_set_nfs4_user, > > +}; > > +#endif > > + > > Any plan to support XATTR_TRUSTED_PREFIX ? > > Thanks. This is an implementation of RFC 8276, which explicitly restricts itself to the "user" namespace. There is currently no portable way to implement the "trusted" namespace within the boundaries of the NFS specification(s), so it's not supported. - Frank
On Fri, 2020-08-21 at 16:03 +0000, Frank van der Linden wrote: > On Fri, Aug 21, 2020 at 02:50:59PM +0800, Murphy Zhou wrote: > > Hi, > > > > On Wed, Jun 24, 2020 at 6:51 AM Frank van der Linden > > <fllinden@amazon.com> wrote: > [...] > > > static const struct inode_operations nfs4_dir_inode_operations = > > > { > > > @@ -10146,10 +10254,21 @@ static const struct xattr_handler > > > nfs4_xattr_nfs4_acl_handler = { > > > .set = nfs4_xattr_set_nfs4_acl, > > > }; > > > > > > +#ifdef CONFIG_NFS_V4_2 > > > +static const struct xattr_handler nfs4_xattr_nfs4_user_handler = > > > { > > > + .prefix = XATTR_USER_PREFIX, > > > + .get = nfs4_xattr_get_nfs4_user, > > > + .set = nfs4_xattr_set_nfs4_user, > > > +}; > > > +#endif > > > + > > > > Any plan to support XATTR_TRUSTED_PREFIX ? > > > > Thanks. > > This is an implementation of RFC 8276, which explicitly restricts > itself > to the "user" namespace. > > There is currently no portable way to implement the "trusted" > namespace > within the boundaries of the NFS specification(s), so it's not > supported. > Correct. 'trusted' is just another way to implement private protocols. Those are unacceptable in a shared filesystem environment.
On Fri, Aug 21, 2020 at 04:41:04PM +0000, Trond Myklebust wrote: > On Fri, 2020-08-21 at 16:03 +0000, Frank van der Linden wrote: > > On Fri, Aug 21, 2020 at 02:50:59PM +0800, Murphy Zhou wrote: > > > Hi, > > > > > > On Wed, Jun 24, 2020 at 6:51 AM Frank van der Linden > > > <fllinden@amazon.com> wrote: > > [...] > > > > static const struct inode_operations nfs4_dir_inode_operations = > > > > { > > > > @@ -10146,10 +10254,21 @@ static const struct xattr_handler > > > > nfs4_xattr_nfs4_acl_handler = { > > > > .set = nfs4_xattr_set_nfs4_acl, > > > > }; > > > > > > > > +#ifdef CONFIG_NFS_V4_2 > > > > +static const struct xattr_handler nfs4_xattr_nfs4_user_handler = > > > > { > > > > + .prefix = XATTR_USER_PREFIX, > > > > + .get = nfs4_xattr_get_nfs4_user, > > > > + .set = nfs4_xattr_set_nfs4_user, > > > > +}; > > > > +#endif > > > > + > > > > > > Any plan to support XATTR_TRUSTED_PREFIX ? > > > > > > Thanks. > > > > This is an implementation of RFC 8276, which explicitly restricts > > itself > > to the "user" namespace. > > > > There is currently no portable way to implement the "trusted" > > namespace > > within the boundaries of the NFS specification(s), so it's not > > supported. > > > > Correct. 'trusted' is just another way to implement private protocols. > Those are unacceptable in a shared filesystem environment. Thank you guys explanation! I'm asking because after NFSv4.2 xattr update, there are some xfstests new failures about 'trusted' xattr. Now they can be surely marked as expected. > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > >
On Mon, Aug 24, 2020 at 08:13:45AM +0800, Murphy Zhou wrote: > Thank you guys explanation! > > I'm asking because after NFSv4.2 xattr update, there are some xfstests > new failures about 'trusted' xattr. Now they can be surely marked as > expected. I have some patches to xfstests that, amongst other things, split the xfstests xattr requirement checks in to individual namespace checks, so that tests that use "user" xattrs will be run on NFS, but others, e.g. "trusted" do not. I should clean them off and send them in. - Frank
On Mon, Aug 24, 2020 at 04:16:57PM +0000, Frank van der Linden wrote: > On Mon, Aug 24, 2020 at 08:13:45AM +0800, Murphy Zhou wrote: > > Thank you guys explanation! > > > > I'm asking because after NFSv4.2 xattr update, there are some xfstests > > new failures about 'trusted' xattr. Now they can be surely marked as > > expected. > > I have some patches to xfstests that, amongst other things, split > the xfstests xattr requirement checks in to individual namespace > checks, so that tests that use "user" xattrs will be run on NFS, but > others, e.g. "trusted" do not. > > I should clean them off and send them in. That's perfect. Thanks! > > - Frank
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 0fbd2925a828..92a07956f07b 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -66,6 +66,7 @@ #include "nfs4idmap.h" #include "nfs4session.h" #include "fscache.h" +#include "nfs42.h" #include "nfs4trace.h" @@ -7440,6 +7441,103 @@ nfs4_listxattr_nfs4_label(struct inode *inode, char *list, size_t list_len) #endif +#ifdef CONFIG_NFS_V4_2 +static int nfs4_xattr_set_nfs4_user(const struct xattr_handler *handler, + struct dentry *unused, struct inode *inode, + const char *key, const void *buf, + size_t buflen, int flags) +{ + struct nfs_access_entry cache; + + if (!nfs_server_capable(inode, NFS_CAP_XATTR)) + return -EOPNOTSUPP; + + /* + * There is no mapping from the MAY_* flags to the NFS_ACCESS_XA* + * flags right now. Handling of xattr operations use the normal + * file read/write permissions. + * + * Just in case the server has other ideas (which RFC 8276 allows), + * do a cached access check for the XA* flags to possibly avoid + * doing an RPC and getting EACCES back. + */ + if (!nfs_access_get_cached(inode, current_cred(), &cache, true)) { + if (!(cache.mask & NFS_ACCESS_XAWRITE)) + return -EACCES; + } + + if (buf == NULL) + return nfs42_proc_removexattr(inode, key); + else + return nfs42_proc_setxattr(inode, key, buf, buflen, flags); +} + +static int nfs4_xattr_get_nfs4_user(const struct xattr_handler *handler, + struct dentry *unused, struct inode *inode, + const char *key, void *buf, size_t buflen) +{ + struct nfs_access_entry cache; + + if (!nfs_server_capable(inode, NFS_CAP_XATTR)) + return -EOPNOTSUPP; + + if (!nfs_access_get_cached(inode, current_cred(), &cache, true)) { + if (!(cache.mask & NFS_ACCESS_XAREAD)) + return -EACCES; + } + + return nfs42_proc_getxattr(inode, key, buf, buflen); +} + +static ssize_t +nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len) +{ + u64 cookie; + bool eof; + int ret, size; + char *buf; + size_t buflen; + struct nfs_access_entry cache; + + if (!nfs_server_capable(inode, NFS_CAP_XATTR)) + return 0; + + if (!nfs_access_get_cached(inode, current_cred(), &cache, true)) { + if (!(cache.mask & NFS_ACCESS_XALIST)) + return 0; + } + + cookie = 0; + eof = false; + buflen = list_len ? list_len : XATTR_LIST_MAX; + buf = list_len ? list : NULL; + size = 0; + + while (!eof) { + ret = nfs42_proc_listxattrs(inode, buf, buflen, + &cookie, &eof); + if (ret < 0) + return ret; + + if (list_len) { + buf += ret; + buflen -= ret; + } + size += ret; + } + + return size; +} + +#else + +static ssize_t +nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len) +{ + return 0; +} +#endif /* CONFIG_NFS_V4_2 */ + /* * nfs_fhget will use either the mounted_on_fileid or the fileid */ @@ -10045,7 +10143,7 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = { static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size) { - ssize_t error, error2; + ssize_t error, error2, error3; error = generic_listxattr(dentry, list, size); if (error < 0) @@ -10058,7 +10156,17 @@ static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size) error2 = nfs4_listxattr_nfs4_label(d_inode(dentry), list, size); if (error2 < 0) return error2; - return error + error2; + + if (list) { + list += error2; + size -= error2; + } + + error3 = nfs4_listxattr_nfs4_user(d_inode(dentry), list, size); + if (error3 < 0) + return error3; + + return error + error2 + error3; } static const struct inode_operations nfs4_dir_inode_operations = { @@ -10146,10 +10254,21 @@ static const struct xattr_handler nfs4_xattr_nfs4_acl_handler = { .set = nfs4_xattr_set_nfs4_acl, }; +#ifdef CONFIG_NFS_V4_2 +static const struct xattr_handler nfs4_xattr_nfs4_user_handler = { + .prefix = XATTR_USER_PREFIX, + .get = nfs4_xattr_get_nfs4_user, + .set = nfs4_xattr_set_nfs4_user, +}; +#endif + const struct xattr_handler *nfs4_xattr_handlers[] = { &nfs4_xattr_nfs4_acl_handler, #ifdef CONFIG_NFS_V4_SECURITY_LABEL &nfs4_xattr_nfs4_label_handler, +#endif +#ifdef CONFIG_NFS_V4_2 + &nfs4_xattr_nfs4_user_handler, #endif NULL };
Now that all the lower level code is there to make the RPC calls, hook it in to the xattr handlers and the listxattr entry point, to make them available. Signed-off-by: Frank van der Linden <fllinden@amazon.com> --- fs/nfs/nfs4proc.c | 123 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 121 insertions(+), 2 deletions(-)