* [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr [not found] <20200623223927.31795-1-fllinden@amazon.com> @ 2020-06-23 22:39 ` Frank van der Linden 2020-06-25 20:39 ` Frank van der Linden ` (2 more replies) 2020-06-23 22:39 ` [PATCH v3 02/10] xattr: add a function to check if a namespace is supported Frank van der Linden 2020-07-04 14:37 ` [PATCH v3 00/10] server side user xattr support (RFC 8276) Chuck Lever 2 siblings, 3 replies; 14+ messages in thread From: Frank van der Linden @ 2020-06-23 22:39 UTC (permalink / raw) To: bfields, chuck.lever, linux-nfs Cc: Frank van der Linden, linux-fsdevel, Al Viro set/removexattr on an exported filesystem should break NFS delegations. This is true in general, but also for the upcoming support for RFC 8726 (NFSv4 extended attribute support). Make sure that they do. Additonally, they need to grow a _locked variant, since callers might call this with i_rwsem held (like the NFS server code). Cc: stable@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Frank van der Linden <fllinden@amazon.com> --- fs/xattr.c | 84 +++++++++++++++++++++++++++++++++++++++---- include/linux/xattr.h | 2 ++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/fs/xattr.c b/fs/xattr.c index 91608d9bfc6a..95f38f57347f 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -204,10 +204,22 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name, return error; } - +/** + * __vfs_setxattr_locked: set an extended attribute while holding the inode + * lock + * + * @dentry - object to perform setxattr on + * @name - xattr name to set + * @value - value to set @name to + * @size - size of @value + * @flags - flags to pass into filesystem operations + * @delegated_inode - on return, will contain an inode pointer that + * a delegation was broken on, NULL if none. + */ int -vfs_setxattr(struct dentry *dentry, const char *name, const void *value, - size_t size, int flags) +__vfs_setxattr_locked(struct dentry *dentry, const char *name, + const void *value, size_t size, int flags, + struct inode **delegated_inode) { struct inode *inode = dentry->d_inode; int error; @@ -216,15 +228,40 @@ vfs_setxattr(struct dentry *dentry, const char *name, const void *value, if (error) return error; - inode_lock(inode); error = security_inode_setxattr(dentry, name, value, size, flags); if (error) goto out; + error = try_break_deleg(inode, delegated_inode); + if (error) + goto out; + error = __vfs_setxattr_noperm(dentry, name, value, size, flags); out: + return error; +} +EXPORT_SYMBOL_GPL(__vfs_setxattr_locked); + +int +vfs_setxattr(struct dentry *dentry, const char *name, const void *value, + size_t size, int flags) +{ + struct inode *inode = dentry->d_inode; + struct inode *delegated_inode = NULL; + int error; + +retry_deleg: + inode_lock(inode); + error = __vfs_setxattr_locked(dentry, name, value, size, flags, + &delegated_inode); inode_unlock(inode); + + if (delegated_inode) { + error = break_deleg_wait(&delegated_inode); + if (!error) + goto retry_deleg; + } return error; } EXPORT_SYMBOL_GPL(vfs_setxattr); @@ -378,8 +415,18 @@ __vfs_removexattr(struct dentry *dentry, const char *name) } EXPORT_SYMBOL(__vfs_removexattr); +/** + * __vfs_removexattr_locked: set an extended attribute while holding the inode + * lock + * + * @dentry - object to perform setxattr on + * @name - name of xattr to remove + * @delegated_inode - on return, will contain an inode pointer that + * a delegation was broken on, NULL if none. + */ int -vfs_removexattr(struct dentry *dentry, const char *name) +__vfs_removexattr_locked(struct dentry *dentry, const char *name, + struct inode **delegated_inode) { struct inode *inode = dentry->d_inode; int error; @@ -388,11 +435,14 @@ vfs_removexattr(struct dentry *dentry, const char *name) if (error) return error; - inode_lock(inode); error = security_inode_removexattr(dentry, name); if (error) goto out; + error = try_break_deleg(inode, delegated_inode); + if (error) + goto out; + error = __vfs_removexattr(dentry, name); if (!error) { @@ -401,12 +451,32 @@ vfs_removexattr(struct dentry *dentry, const char *name) } out: + return error; +} +EXPORT_SYMBOL_GPL(__vfs_removexattr_locked); + +int +vfs_removexattr(struct dentry *dentry, const char *name) +{ + struct inode *inode = dentry->d_inode; + struct inode *delegated_inode = NULL; + int error; + +retry_deleg: + inode_lock(inode); + error = __vfs_removexattr_locked(dentry, name, &delegated_inode); inode_unlock(inode); + + if (delegated_inode) { + error = break_deleg_wait(&delegated_inode); + if (!error) + goto retry_deleg; + } + return error; } EXPORT_SYMBOL_GPL(vfs_removexattr); - /* * Extended attribute SET operations */ diff --git a/include/linux/xattr.h b/include/linux/xattr.h index 47eaa34f8761..a2f3cd02653c 100644 --- a/include/linux/xattr.h +++ b/include/linux/xattr.h @@ -51,8 +51,10 @@ ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t); ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size); int __vfs_setxattr(struct dentry *, struct inode *, const char *, const void *, size_t, int); int __vfs_setxattr_noperm(struct dentry *, const char *, const void *, size_t, int); +int __vfs_setxattr_locked(struct dentry *, const char *, const void *, size_t, int, struct inode **); int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int); int __vfs_removexattr(struct dentry *, const char *); +int __vfs_removexattr_locked(struct dentry *, const char *, struct inode **); int vfs_removexattr(struct dentry *, const char *); ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size); -- 2.17.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr 2020-06-23 22:39 ` [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr Frank van der Linden @ 2020-06-25 20:39 ` Frank van der Linden 2020-07-14 17:11 ` Frank van der Linden 2020-07-01 19:33 ` Sasha Levin 2020-07-10 14:03 ` Sasha Levin 2 siblings, 1 reply; 14+ messages in thread From: Frank van der Linden @ 2020-06-25 20:39 UTC (permalink / raw) To: bfields, chuck.lever, Al Viro; +Cc: linux-fsdevel Hi Al, Do you have any comments / concerns about this patch? It's part of nfs server side user xattr support, full series here: https://lore.kernel.org/linux-nfs/20200623223927.31795-1-fllinden@amazon.com/ I copied this one to linux-fsdevel and you, just giving you an extra ping. Bruce/Chuck are OK with the rest of the series, so I just need your ACK on this one, and the next one (will send the ping separately). Thanks, - Frank On Tue, Jun 23, 2020 at 10:39:18PM +0000, Frank van der Linden wrote: > set/removexattr on an exported filesystem should break NFS delegations. > This is true in general, but also for the upcoming support for > RFC 8726 (NFSv4 extended attribute support). Make sure that they do. > > Additonally, they need to grow a _locked variant, since callers might > call this with i_rwsem held (like the NFS server code). > > Cc: stable@vger.kernel.org > Cc: linux-fsdevel@vger.kernel.org > Cc: Al Viro <viro@zeniv.linux.org.uk> > Signed-off-by: Frank van der Linden <fllinden@amazon.com> > --- > fs/xattr.c | 84 +++++++++++++++++++++++++++++++++++++++---- > include/linux/xattr.h | 2 ++ > 2 files changed, 79 insertions(+), 7 deletions(-) > > diff --git a/fs/xattr.c b/fs/xattr.c > index 91608d9bfc6a..95f38f57347f 100644 > --- a/fs/xattr.c > +++ b/fs/xattr.c > @@ -204,10 +204,22 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name, > return error; > } > > - > +/** > + * __vfs_setxattr_locked: set an extended attribute while holding the inode > + * lock > + * > + * @dentry - object to perform setxattr on > + * @name - xattr name to set > + * @value - value to set @name to > + * @size - size of @value > + * @flags - flags to pass into filesystem operations > + * @delegated_inode - on return, will contain an inode pointer that > + * a delegation was broken on, NULL if none. > + */ > int > -vfs_setxattr(struct dentry *dentry, const char *name, const void *value, > - size_t size, int flags) > +__vfs_setxattr_locked(struct dentry *dentry, const char *name, > + const void *value, size_t size, int flags, > + struct inode **delegated_inode) > { > struct inode *inode = dentry->d_inode; > int error; > @@ -216,15 +228,40 @@ vfs_setxattr(struct dentry *dentry, const char *name, const void *value, > if (error) > return error; > > - inode_lock(inode); > error = security_inode_setxattr(dentry, name, value, size, flags); > if (error) > goto out; > > + error = try_break_deleg(inode, delegated_inode); > + if (error) > + goto out; > + > error = __vfs_setxattr_noperm(dentry, name, value, size, flags); > > out: > + return error; > +} > +EXPORT_SYMBOL_GPL(__vfs_setxattr_locked); > + > +int > +vfs_setxattr(struct dentry *dentry, const char *name, const void *value, > + size_t size, int flags) > +{ > + struct inode *inode = dentry->d_inode; > + struct inode *delegated_inode = NULL; > + int error; > + > +retry_deleg: > + inode_lock(inode); > + error = __vfs_setxattr_locked(dentry, name, value, size, flags, > + &delegated_inode); > inode_unlock(inode); > + > + if (delegated_inode) { > + error = break_deleg_wait(&delegated_inode); > + if (!error) > + goto retry_deleg; > + } > return error; > } > EXPORT_SYMBOL_GPL(vfs_setxattr); > @@ -378,8 +415,18 @@ __vfs_removexattr(struct dentry *dentry, const char *name) > } > EXPORT_SYMBOL(__vfs_removexattr); > > +/** > + * __vfs_removexattr_locked: set an extended attribute while holding the inode > + * lock > + * > + * @dentry - object to perform setxattr on > + * @name - name of xattr to remove > + * @delegated_inode - on return, will contain an inode pointer that > + * a delegation was broken on, NULL if none. > + */ > int > -vfs_removexattr(struct dentry *dentry, const char *name) > +__vfs_removexattr_locked(struct dentry *dentry, const char *name, > + struct inode **delegated_inode) > { > struct inode *inode = dentry->d_inode; > int error; > @@ -388,11 +435,14 @@ vfs_removexattr(struct dentry *dentry, const char *name) > if (error) > return error; > > - inode_lock(inode); > error = security_inode_removexattr(dentry, name); > if (error) > goto out; > > + error = try_break_deleg(inode, delegated_inode); > + if (error) > + goto out; > + > error = __vfs_removexattr(dentry, name); > > if (!error) { > @@ -401,12 +451,32 @@ vfs_removexattr(struct dentry *dentry, const char *name) > } > > out: > + return error; > +} > +EXPORT_SYMBOL_GPL(__vfs_removexattr_locked); > + > +int > +vfs_removexattr(struct dentry *dentry, const char *name) > +{ > + struct inode *inode = dentry->d_inode; > + struct inode *delegated_inode = NULL; > + int error; > + > +retry_deleg: > + inode_lock(inode); > + error = __vfs_removexattr_locked(dentry, name, &delegated_inode); > inode_unlock(inode); > + > + if (delegated_inode) { > + error = break_deleg_wait(&delegated_inode); > + if (!error) > + goto retry_deleg; > + } > + > return error; > } > EXPORT_SYMBOL_GPL(vfs_removexattr); > > - > /* > * Extended attribute SET operations > */ > diff --git a/include/linux/xattr.h b/include/linux/xattr.h > index 47eaa34f8761..a2f3cd02653c 100644 > --- a/include/linux/xattr.h > +++ b/include/linux/xattr.h > @@ -51,8 +51,10 @@ ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t); > ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size); > int __vfs_setxattr(struct dentry *, struct inode *, const char *, const void *, size_t, int); > int __vfs_setxattr_noperm(struct dentry *, const char *, const void *, size_t, int); > +int __vfs_setxattr_locked(struct dentry *, const char *, const void *, size_t, int, struct inode **); > int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int); > int __vfs_removexattr(struct dentry *, const char *); > +int __vfs_removexattr_locked(struct dentry *, const char *, struct inode **); > int vfs_removexattr(struct dentry *, const char *); > > ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size); > -- > 2.17.2 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr 2020-06-25 20:39 ` Frank van der Linden @ 2020-07-14 17:11 ` Frank van der Linden 0 siblings, 0 replies; 14+ messages in thread From: Frank van der Linden @ 2020-07-14 17:11 UTC (permalink / raw) To: bfields, chuck.lever, Al Viro; +Cc: linux-fsdevel, torvalds On Thu, Jun 25, 2020 at 08:39:54PM +0000, Frank van der Linden wrote: > Hi Al, > > Do you have any comments / concerns about this patch? It's part of nfs > server side user xattr support, full series here: > > https://lore.kernel.org/linux-nfs/20200623223927.31795-1-fllinden@amazon.com/ > > I copied this one to linux-fsdevel and you, just giving you an extra > ping. Bruce/Chuck are OK with the rest of the series, so I just need > your ACK on this one, and the next one (will send the ping separately). > > Thanks, > > - Frank Hi Al, Any comments on this one? It's a simple change to break (NFSv4) delegations on set/removexattr, so it's pretty nfsd specific. Linus - since this is nfsd specific, could this go in through the nfsd maintainer tree directly? Chuck has included it in a tree that is being readied for 5.9. Thanks, - Frank > > > On Tue, Jun 23, 2020 at 10:39:18PM +0000, Frank van der Linden wrote: > > set/removexattr on an exported filesystem should break NFS delegations. > > This is true in general, but also for the upcoming support for > > RFC 8726 (NFSv4 extended attribute support). Make sure that they do. > > > > Additonally, they need to grow a _locked variant, since callers might > > call this with i_rwsem held (like the NFS server code). > > > > Cc: stable@vger.kernel.org > > Cc: linux-fsdevel@vger.kernel.org > > Cc: Al Viro <viro@zeniv.linux.org.uk> > > Signed-off-by: Frank van der Linden <fllinden@amazon.com> > > --- > > fs/xattr.c | 84 +++++++++++++++++++++++++++++++++++++++---- > > include/linux/xattr.h | 2 ++ > > 2 files changed, 79 insertions(+), 7 deletions(-) > > > > diff --git a/fs/xattr.c b/fs/xattr.c > > index 91608d9bfc6a..95f38f57347f 100644 > > --- a/fs/xattr.c > > +++ b/fs/xattr.c > > @@ -204,10 +204,22 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name, > > return error; > > } > > > > - > > +/** > > + * __vfs_setxattr_locked: set an extended attribute while holding the inode > > + * lock > > + * > > + * @dentry - object to perform setxattr on > > + * @name - xattr name to set > > + * @value - value to set @name to > > + * @size - size of @value > > + * @flags - flags to pass into filesystem operations > > + * @delegated_inode - on return, will contain an inode pointer that > > + * a delegation was broken on, NULL if none. > > + */ > > int > > -vfs_setxattr(struct dentry *dentry, const char *name, const void *value, > > - size_t size, int flags) > > +__vfs_setxattr_locked(struct dentry *dentry, const char *name, > > + const void *value, size_t size, int flags, > > + struct inode **delegated_inode) > > { > > struct inode *inode = dentry->d_inode; > > int error; > > @@ -216,15 +228,40 @@ vfs_setxattr(struct dentry *dentry, const char *name, const void *value, > > if (error) > > return error; > > > > - inode_lock(inode); > > error = security_inode_setxattr(dentry, name, value, size, flags); > > if (error) > > goto out; > > > > + error = try_break_deleg(inode, delegated_inode); > > + if (error) > > + goto out; > > + > > error = __vfs_setxattr_noperm(dentry, name, value, size, flags); > > > > out: > > + return error; > > +} > > +EXPORT_SYMBOL_GPL(__vfs_setxattr_locked); > > + > > +int > > +vfs_setxattr(struct dentry *dentry, const char *name, const void *value, > > + size_t size, int flags) > > +{ > > + struct inode *inode = dentry->d_inode; > > + struct inode *delegated_inode = NULL; > > + int error; > > + > > +retry_deleg: > > + inode_lock(inode); > > + error = __vfs_setxattr_locked(dentry, name, value, size, flags, > > + &delegated_inode); > > inode_unlock(inode); > > + > > + if (delegated_inode) { > > + error = break_deleg_wait(&delegated_inode); > > + if (!error) > > + goto retry_deleg; > > + } > > return error; > > } > > EXPORT_SYMBOL_GPL(vfs_setxattr); > > @@ -378,8 +415,18 @@ __vfs_removexattr(struct dentry *dentry, const char *name) > > } > > EXPORT_SYMBOL(__vfs_removexattr); > > > > +/** > > + * __vfs_removexattr_locked: set an extended attribute while holding the inode > > + * lock > > + * > > + * @dentry - object to perform setxattr on > > + * @name - name of xattr to remove > > + * @delegated_inode - on return, will contain an inode pointer that > > + * a delegation was broken on, NULL if none. > > + */ > > int > > -vfs_removexattr(struct dentry *dentry, const char *name) > > +__vfs_removexattr_locked(struct dentry *dentry, const char *name, > > + struct inode **delegated_inode) > > { > > struct inode *inode = dentry->d_inode; > > int error; > > @@ -388,11 +435,14 @@ vfs_removexattr(struct dentry *dentry, const char *name) > > if (error) > > return error; > > > > - inode_lock(inode); > > error = security_inode_removexattr(dentry, name); > > if (error) > > goto out; > > > > + error = try_break_deleg(inode, delegated_inode); > > + if (error) > > + goto out; > > + > > error = __vfs_removexattr(dentry, name); > > > > if (!error) { > > @@ -401,12 +451,32 @@ vfs_removexattr(struct dentry *dentry, const char *name) > > } > > > > out: > > + return error; > > +} > > +EXPORT_SYMBOL_GPL(__vfs_removexattr_locked); > > + > > +int > > +vfs_removexattr(struct dentry *dentry, const char *name) > > +{ > > + struct inode *inode = dentry->d_inode; > > + struct inode *delegated_inode = NULL; > > + int error; > > + > > +retry_deleg: > > + inode_lock(inode); > > + error = __vfs_removexattr_locked(dentry, name, &delegated_inode); > > inode_unlock(inode); > > + > > + if (delegated_inode) { > > + error = break_deleg_wait(&delegated_inode); > > + if (!error) > > + goto retry_deleg; > > + } > > + > > return error; > > } > > EXPORT_SYMBOL_GPL(vfs_removexattr); > > > > - > > /* > > * Extended attribute SET operations > > */ > > diff --git a/include/linux/xattr.h b/include/linux/xattr.h > > index 47eaa34f8761..a2f3cd02653c 100644 > > --- a/include/linux/xattr.h > > +++ b/include/linux/xattr.h > > @@ -51,8 +51,10 @@ ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t); > > ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size); > > int __vfs_setxattr(struct dentry *, struct inode *, const char *, const void *, size_t, int); > > int __vfs_setxattr_noperm(struct dentry *, const char *, const void *, size_t, int); > > +int __vfs_setxattr_locked(struct dentry *, const char *, const void *, size_t, int, struct inode **); > > int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int); > > int __vfs_removexattr(struct dentry *, const char *); > > +int __vfs_removexattr_locked(struct dentry *, const char *, struct inode **); > > int vfs_removexattr(struct dentry *, const char *); > > > > ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size); > > -- > > 2.17.2 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr 2020-06-23 22:39 ` [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr Frank van der Linden 2020-06-25 20:39 ` Frank van der Linden @ 2020-07-01 19:33 ` Sasha Levin 2020-07-10 14:03 ` Sasha Levin 2 siblings, 0 replies; 14+ messages in thread From: Sasha Levin @ 2020-07-01 19:33 UTC (permalink / raw) To: Sasha Levin, Frank van der Linden, bfields, chuck.lever Cc: Frank van der Linden, stable, linux-fsdevel, Al Viro, stable Hi [This is an automated email] This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all The bot has tested the following trees: v5.7.6, v5.4.49, v4.19.130, v4.14.186, v4.9.228, v4.4.228. v5.7.6: Build OK! v5.4.49: Build OK! v4.19.130: Build OK! v4.14.186: Build OK! v4.9.228: Build OK! v4.4.228: Failed to apply! Possible dependencies: 5d6c31910bc07 ("xattr: Add __vfs_{get,set,remove}xattr helpers") 6b2553918d8b4 ("replace ->follow_link() with new method that could stay in RCU mode") aa80deab33a8f ("namei: page_getlink() and page_follow_link_light() are the same thing") cd3417c8fc950 ("kill free_page_put_link()") ce23e64013348 ("->getxattr(): pass dentry and inode as separate arguments") fceef393a5381 ("switch ->get_link() to delayed_call, kill ->put_link()") NOTE: The patch will not be queued to stable trees until it is upstream. How should we proceed with this patch? -- Thanks Sasha ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr 2020-06-23 22:39 ` [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr Frank van der Linden 2020-06-25 20:39 ` Frank van der Linden 2020-07-01 19:33 ` Sasha Levin @ 2020-07-10 14:03 ` Sasha Levin 2020-07-10 14:08 ` Chuck Lever 2 siblings, 1 reply; 14+ messages in thread From: Sasha Levin @ 2020-07-10 14:03 UTC (permalink / raw) To: Sasha Levin, Frank van der Linden, bfields, chuck.lever Cc: Frank van der Linden, stable, linux-fsdevel, Al Viro, stable Hi [This is an automated email] This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all The bot has tested the following trees: v5.7.6, v5.4.49, v4.19.130, v4.14.186, v4.9.228, v4.4.228. v5.7.6: Build OK! v5.4.49: Build OK! v4.19.130: Build OK! v4.14.186: Build OK! v4.9.228: Build OK! v4.4.228: Failed to apply! Possible dependencies: 5d6c31910bc07 ("xattr: Add __vfs_{get,set,remove}xattr helpers") 6b2553918d8b4 ("replace ->follow_link() with new method that could stay in RCU mode") aa80deab33a8f ("namei: page_getlink() and page_follow_link_light() are the same thing") cd3417c8fc950 ("kill free_page_put_link()") ce23e64013348 ("->getxattr(): pass dentry and inode as separate arguments") fceef393a5381 ("switch ->get_link() to delayed_call, kill ->put_link()") NOTE: The patch will not be queued to stable trees until it is upstream. How should we proceed with this patch? -- Thanks Sasha ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr 2020-07-10 14:03 ` Sasha Levin @ 2020-07-10 14:08 ` Chuck Lever 0 siblings, 0 replies; 14+ messages in thread From: Chuck Lever @ 2020-07-10 14:08 UTC (permalink / raw) To: Sasha Levin Cc: Frank van der Linden, Bruce Fields, stable, linux-fsdevel, Al Viro Hi Sasha- > On Jul 10, 2020, at 10:03 AM, Sasha Levin <sashal@kernel.org> wrote: > > Hi > > [This is an automated email] > > This commit has been processed because it contains a -stable tag. > The stable tag indicates that it's relevant for the following trees: all > > The bot has tested the following trees: v5.7.6, v5.4.49, v4.19.130, v4.14.186, v4.9.228, v4.4.228. > > v5.7.6: Build OK! > v5.4.49: Build OK! > v4.19.130: Build OK! > v4.14.186: Build OK! > v4.9.228: Build OK! > v4.4.228: Failed to apply! Possible dependencies: > 5d6c31910bc07 ("xattr: Add __vfs_{get,set,remove}xattr helpers") > 6b2553918d8b4 ("replace ->follow_link() with new method that could stay in RCU mode") > aa80deab33a8f ("namei: page_getlink() and page_follow_link_light() are the same thing") > cd3417c8fc950 ("kill free_page_put_link()") > ce23e64013348 ("->getxattr(): pass dentry and inode as separate arguments") > fceef393a5381 ("switch ->get_link() to delayed_call, kill ->put_link()") > > > NOTE: The patch will not be queued to stable trees until it is upstream. > > How should we proceed with this patch? I've updated the "cc: stable" tag in my testing branch to include "# v4.9+". -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 02/10] xattr: add a function to check if a namespace is supported [not found] <20200623223927.31795-1-fllinden@amazon.com> 2020-06-23 22:39 ` [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr Frank van der Linden @ 2020-06-23 22:39 ` Frank van der Linden 2020-06-25 20:41 ` Frank van der Linden 2020-07-04 14:37 ` [PATCH v3 00/10] server side user xattr support (RFC 8276) Chuck Lever 2 siblings, 1 reply; 14+ messages in thread From: Frank van der Linden @ 2020-06-23 22:39 UTC (permalink / raw) To: bfields, chuck.lever, linux-nfs Cc: Frank van der Linden, linux-fsdevel, Al Viro Add a function that checks is an extended attribute namespace is supported for an inode, meaning that a handler must be present for either the whole namespace, or at least one synthetic xattr in the namespace. To be used by the nfs server code when being queried for extended attributes support. Cc: linux-fsdevel@vger.kernel.org Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Frank van der Linden <fllinden@amazon.com> --- fs/xattr.c | 27 +++++++++++++++++++++++++++ include/linux/xattr.h | 2 ++ 2 files changed, 29 insertions(+) diff --git a/fs/xattr.c b/fs/xattr.c index 95f38f57347f..386b45676d7e 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -134,6 +134,33 @@ xattr_permission(struct inode *inode, const char *name, int mask) return inode_permission(inode, mask); } +/* + * Look for any handler that deals with the specified namespace. + */ +int +xattr_supported_namespace(struct inode *inode, const char *prefix) +{ + const struct xattr_handler **handlers = inode->i_sb->s_xattr; + const struct xattr_handler *handler; + size_t preflen; + + if (!(inode->i_opflags & IOP_XATTR)) { + if (unlikely(is_bad_inode(inode))) + return -EIO; + return -EOPNOTSUPP; + } + + preflen = strlen(prefix); + + for_each_xattr_handler(handlers, handler) { + if (!strncmp(xattr_prefix(handler), prefix, preflen)) + return 0; + } + + return -EOPNOTSUPP; +} +EXPORT_SYMBOL(xattr_supported_namespace); + int __vfs_setxattr(struct dentry *dentry, struct inode *inode, const char *name, const void *value, size_t size, int flags) diff --git a/include/linux/xattr.h b/include/linux/xattr.h index a2f3cd02653c..fac75810d9d3 100644 --- a/include/linux/xattr.h +++ b/include/linux/xattr.h @@ -61,6 +61,8 @@ ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_siz ssize_t vfs_getxattr_alloc(struct dentry *dentry, const char *name, char **xattr_value, size_t size, gfp_t flags); +int xattr_supported_namespace(struct inode *inode, const char *prefix); + static inline const char *xattr_prefix(const struct xattr_handler *handler) { return handler->prefix ?: handler->name; -- 2.17.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 02/10] xattr: add a function to check if a namespace is supported 2020-06-23 22:39 ` [PATCH v3 02/10] xattr: add a function to check if a namespace is supported Frank van der Linden @ 2020-06-25 20:41 ` Frank van der Linden 2020-07-14 17:13 ` Frank van der Linden 0 siblings, 1 reply; 14+ messages in thread From: Frank van der Linden @ 2020-06-25 20:41 UTC (permalink / raw) To: bfields, chuck.lever, Al Viro; +Cc: linux-fsdevel Hi Al, ..and here is the other xattr change that was part of the nfsd user xattr support series that I mentioned in my previous email. Full series here: https://lore.kernel.org/linux-nfs/20200623223927.31795-1-fllinden@amazon.com/ Any comments / concerned about this one? Thanks, - Frank On Tue, Jun 23, 2020 at 10:39:19PM +0000, Frank van der Linden wrote: > Add a function that checks is an extended attribute namespace is > supported for an inode, meaning that a handler must be present > for either the whole namespace, or at least one synthetic > xattr in the namespace. > > To be used by the nfs server code when being queried for extended > attributes support. > > Cc: linux-fsdevel@vger.kernel.org > Cc: Al Viro <viro@zeniv.linux.org.uk> > Signed-off-by: Frank van der Linden <fllinden@amazon.com> > --- > fs/xattr.c | 27 +++++++++++++++++++++++++++ > include/linux/xattr.h | 2 ++ > 2 files changed, 29 insertions(+) > > diff --git a/fs/xattr.c b/fs/xattr.c > index 95f38f57347f..386b45676d7e 100644 > --- a/fs/xattr.c > +++ b/fs/xattr.c > @@ -134,6 +134,33 @@ xattr_permission(struct inode *inode, const char *name, int mask) > return inode_permission(inode, mask); > } > > +/* > + * Look for any handler that deals with the specified namespace. > + */ > +int > +xattr_supported_namespace(struct inode *inode, const char *prefix) > +{ > + const struct xattr_handler **handlers = inode->i_sb->s_xattr; > + const struct xattr_handler *handler; > + size_t preflen; > + > + if (!(inode->i_opflags & IOP_XATTR)) { > + if (unlikely(is_bad_inode(inode))) > + return -EIO; > + return -EOPNOTSUPP; > + } > + > + preflen = strlen(prefix); > + > + for_each_xattr_handler(handlers, handler) { > + if (!strncmp(xattr_prefix(handler), prefix, preflen)) > + return 0; > + } > + > + return -EOPNOTSUPP; > +} > +EXPORT_SYMBOL(xattr_supported_namespace); > + > int > __vfs_setxattr(struct dentry *dentry, struct inode *inode, const char *name, > const void *value, size_t size, int flags) > diff --git a/include/linux/xattr.h b/include/linux/xattr.h > index a2f3cd02653c..fac75810d9d3 100644 > --- a/include/linux/xattr.h > +++ b/include/linux/xattr.h > @@ -61,6 +61,8 @@ ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_siz > ssize_t vfs_getxattr_alloc(struct dentry *dentry, const char *name, > char **xattr_value, size_t size, gfp_t flags); > > +int xattr_supported_namespace(struct inode *inode, const char *prefix); > + > static inline const char *xattr_prefix(const struct xattr_handler *handler) > { > return handler->prefix ?: handler->name; > -- > 2.17.2 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 02/10] xattr: add a function to check if a namespace is supported 2020-06-25 20:41 ` Frank van der Linden @ 2020-07-14 17:13 ` Frank van der Linden 2020-07-14 18:46 ` Linus Torvalds 0 siblings, 1 reply; 14+ messages in thread From: Frank van der Linden @ 2020-07-14 17:13 UTC (permalink / raw) To: bfields, chuck.lever, Al Viro; +Cc: linux-fsdevel, torvalds On Thu, Jun 25, 2020 at 08:41:57PM +0000, Frank van der Linden wrote: > Hi Al, > > ..and here is the other xattr change that was part of the nfsd user xattr > support series that I mentioned in my previous email. > > Full series here: > > https://lore.kernel.org/linux-nfs/20200623223927.31795-1-fllinden@amazon.com/ > > Any comments / concerned about this one? > > Thanks, > > - Frank Hi Al, Here's the other one I'm just sending a quick ping on. It's a simple change - just add a little new function that enables nfsd to check if the "user." namespace is at all supported by a filesystem. Any comments? Again, Linus - this is a pretty small change, doesn't affect any existing codepaths, and it's already in the tree Chuck is setting up for 5.9. Could this go in through that directly? Thanks, - Frank > > On Tue, Jun 23, 2020 at 10:39:19PM +0000, Frank van der Linden wrote: > > Add a function that checks is an extended attribute namespace is > > supported for an inode, meaning that a handler must be present > > for either the whole namespace, or at least one synthetic > > xattr in the namespace. > > > > To be used by the nfs server code when being queried for extended > > attributes support. > > > > Cc: linux-fsdevel@vger.kernel.org > > Cc: Al Viro <viro@zeniv.linux.org.uk> > > Signed-off-by: Frank van der Linden <fllinden@amazon.com> > > --- > > fs/xattr.c | 27 +++++++++++++++++++++++++++ > > include/linux/xattr.h | 2 ++ > > 2 files changed, 29 insertions(+) > > > > diff --git a/fs/xattr.c b/fs/xattr.c > > index 95f38f57347f..386b45676d7e 100644 > > --- a/fs/xattr.c > > +++ b/fs/xattr.c > > @@ -134,6 +134,33 @@ xattr_permission(struct inode *inode, const char *name, int mask) > > return inode_permission(inode, mask); > > } > > > > +/* > > + * Look for any handler that deals with the specified namespace. > > + */ > > +int > > +xattr_supported_namespace(struct inode *inode, const char *prefix) > > +{ > > + const struct xattr_handler **handlers = inode->i_sb->s_xattr; > > + const struct xattr_handler *handler; > > + size_t preflen; > > + > > + if (!(inode->i_opflags & IOP_XATTR)) { > > + if (unlikely(is_bad_inode(inode))) > > + return -EIO; > > + return -EOPNOTSUPP; > > + } > > + > > + preflen = strlen(prefix); > > + > > + for_each_xattr_handler(handlers, handler) { > > + if (!strncmp(xattr_prefix(handler), prefix, preflen)) > > + return 0; > > + } > > + > > + return -EOPNOTSUPP; > > +} > > +EXPORT_SYMBOL(xattr_supported_namespace); > > + > > int > > __vfs_setxattr(struct dentry *dentry, struct inode *inode, const char *name, > > const void *value, size_t size, int flags) > > diff --git a/include/linux/xattr.h b/include/linux/xattr.h > > index a2f3cd02653c..fac75810d9d3 100644 > > --- a/include/linux/xattr.h > > +++ b/include/linux/xattr.h > > @@ -61,6 +61,8 @@ ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_siz > > ssize_t vfs_getxattr_alloc(struct dentry *dentry, const char *name, > > char **xattr_value, size_t size, gfp_t flags); > > > > +int xattr_supported_namespace(struct inode *inode, const char *prefix); > > + > > static inline const char *xattr_prefix(const struct xattr_handler *handler) > > { > > return handler->prefix ?: handler->name; > > -- > > 2.17.2 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 02/10] xattr: add a function to check if a namespace is supported 2020-07-14 17:13 ` Frank van der Linden @ 2020-07-14 18:46 ` Linus Torvalds 2020-07-28 14:17 ` Chuck Lever 0 siblings, 1 reply; 14+ messages in thread From: Linus Torvalds @ 2020-07-14 18:46 UTC (permalink / raw) To: Frank van der Linden; +Cc: J. Bruce Fields, Chuck Lever, Al Viro, linux-fsdevel On Tue, Jul 14, 2020 at 10:13 AM Frank van der Linden <fllinden@amazon.com> wrote: > > Again, Linus - this is a pretty small change, doesn't affect any existing > codepaths, and it's already in the tree Chuck is setting up for 5.9. Could > this go in through that directly? Both ok by me, but I'd like to have Al ack them. Al? Linus ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 02/10] xattr: add a function to check if a namespace is supported 2020-07-14 18:46 ` Linus Torvalds @ 2020-07-28 14:17 ` Chuck Lever 2020-07-28 14:33 ` Al Viro 0 siblings, 1 reply; 14+ messages in thread From: Chuck Lever @ 2020-07-28 14:17 UTC (permalink / raw) To: Linus Torvalds, Al Viro; +Cc: Frank van der Linden, Bruce Fields, linux-fsdevel Hi Linus- > On Jul 14, 2020, at 2:46 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Tue, Jul 14, 2020 at 10:13 AM Frank van der Linden > <fllinden@amazon.com> wrote: >> >> Again, Linus - this is a pretty small change, doesn't affect any existing >> codepaths, and it's already in the tree Chuck is setting up for 5.9. Could >> this go in through that directly? > > Both ok by me, but I'd like to have Al ack them. Al? I have the NFSD user xattr patches in the current series waiting to be merged into v5.9. I'd like to create the nfsd-5.9 merge tag soon, but I haven't heard any review comments from Al. How would you like me to proceed? -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 02/10] xattr: add a function to check if a namespace is supported 2020-07-28 14:17 ` Chuck Lever @ 2020-07-28 14:33 ` Al Viro 2020-07-29 12:23 ` Chuck Lever 0 siblings, 1 reply; 14+ messages in thread From: Al Viro @ 2020-07-28 14:33 UTC (permalink / raw) To: Chuck Lever Cc: Linus Torvalds, Frank van der Linden, Bruce Fields, linux-fsdevel On Tue, Jul 28, 2020 at 10:17:07AM -0400, Chuck Lever wrote: > Hi Linus- > > > On Jul 14, 2020, at 2:46 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > > On Tue, Jul 14, 2020 at 10:13 AM Frank van der Linden > > <fllinden@amazon.com> wrote: > >> > >> Again, Linus - this is a pretty small change, doesn't affect any existing > >> codepaths, and it's already in the tree Chuck is setting up for 5.9. Could > >> this go in through that directly? > > > > Both ok by me, but I'd like to have Al ack them. Al? > > I have the NFSD user xattr patches in the current series waiting to be > merged into v5.9. I'd like to create the nfsd-5.9 merge tag soon, but I > haven't heard any review comments from Al. How would you like me to > proceed? Looks sane, AFAICS. Sorry, still digging myself from under the mounds of mail... ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 02/10] xattr: add a function to check if a namespace is supported 2020-07-28 14:33 ` Al Viro @ 2020-07-29 12:23 ` Chuck Lever 0 siblings, 0 replies; 14+ messages in thread From: Chuck Lever @ 2020-07-29 12:23 UTC (permalink / raw) To: Al Viro; +Cc: Linus Torvalds, Frank van der Linden, Bruce Fields, linux-fsdevel > On Jul 28, 2020, at 10:33 AM, Al Viro <viro@zeniv.linux.org.uk> wrote: > > On Tue, Jul 28, 2020 at 10:17:07AM -0400, Chuck Lever wrote: >> Hi Linus- >> >>> On Jul 14, 2020, at 2:46 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: >>> >>> On Tue, Jul 14, 2020 at 10:13 AM Frank van der Linden >>> <fllinden@amazon.com> wrote: >>>> >>>> Again, Linus - this is a pretty small change, doesn't affect any existing >>>> codepaths, and it's already in the tree Chuck is setting up for 5.9. Could >>>> this go in through that directly? >>> >>> Both ok by me, but I'd like to have Al ack them. Al? >> >> I have the NFSD user xattr patches in the current series waiting to be >> merged into v5.9. I'd like to create the nfsd-5.9 merge tag soon, but I >> haven't heard any review comments from Al. How would you like me to >> proceed? > > Looks sane, AFAICS. Sorry, still digging myself from under the mounds of > mail... No problem at all. Thanks! -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 00/10] server side user xattr support (RFC 8276) [not found] <20200623223927.31795-1-fllinden@amazon.com> 2020-06-23 22:39 ` [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr Frank van der Linden 2020-06-23 22:39 ` [PATCH v3 02/10] xattr: add a function to check if a namespace is supported Frank van der Linden @ 2020-07-04 14:37 ` Chuck Lever 2 siblings, 0 replies; 14+ messages in thread From: Chuck Lever @ 2020-07-04 14:37 UTC (permalink / raw) To: Frank van der Linden, Al Viro Cc: Bruce Fields, Linux NFS Mailing List, linux-fsdevel > On Jun 23, 2020, at 6:39 PM, Frank van der Linden <fllinden@amazon.com> wrote: > > v3: > * Rebase to v5.8-rc2 > * Use length probe + allocate + query for the listxattr and setxattr > operations to avoid allocating unneeded space. > * Because of the above, drop the 'use kvmalloc for svcxdr_tmpalloc' patch, > as it's no longer needed. v3 of this series has been applied to nfsd-5.9. Thanks! See: git://git.linux-nfs.org/projects/cel/cel-2.6.git nfsd-5.9 Still waiting for Acks on 01/13 and 02/13. > v2: > * As per the discussion, user extended attributes are enabled if > the client and server support them (e.g. they support 4.2 and > advertise the user extended attribute FATTR). There are no longer > options to switch them off. > * The code is no longer conditioned on a config option. > * The number of patches has been reduced somewhat by merging > smaller, related ones. > * Renamed some functions and added parameter comments as requested. > > v1: > > * Split in to client and server (changed from the original RFC patch). > > Original RFC combined set is here: > > https://www.spinics.net/lists/linux-nfs/msg74843.html > > In general, these patches were, both server and client, tested as > follows: > * stress-ng-xattr with 1000 workers > * Test all corner cases (XATTR_SIZE_*) > * Test all failure cases (no xattr, setxattr with different or > invalid flags, etc). > * Verify the content of xattrs across several operations. > * Use KASAN and KMEMLEAK for a longer mix of testruns to verify > that there were no leaks (after unmounting the filesystem). > * Interop run against FreeBSD server/client implementation. > * Ran xfstests-dev, with no unexpected/new failures as compared > to an unpatched kernel. To fully use xfstests-dev, it needed > some modifications, as it expects to either use all xattr > namespaces, or none. Whereas NFS only suppors the "user." > namespace (+ optional ACLs). I will send the changes in > seperately. > > > Frank van der Linden (10): > xattr: break delegations in {set,remove}xattr > xattr: add a function to check if a namespace is supported > nfs,nfsd: NFSv4.2 extended attribute protocol definitions > nfsd: split off the write decode code in to a separate function > nfsd: add defines for NFSv4.2 extended attribute support > nfsd: define xattr functions to call in to their vfs counterparts > nfsd: take xattr bits in to account for permission checks > nfsd: add structure definitions for xattr requests / responses > nfsd: implement the xattr functions and en/decode logic > nfsd: add fattr support for user extended attributes > > fs/nfsd/nfs4proc.c | 128 ++++++++- > fs/nfsd/nfs4xdr.c | 531 +++++++++++++++++++++++++++++++++++--- > fs/nfsd/nfsd.h | 5 +- > fs/nfsd/vfs.c | 239 +++++++++++++++++ > fs/nfsd/vfs.h | 10 + > fs/nfsd/xdr4.h | 31 +++ > fs/xattr.c | 111 +++++++- > include/linux/nfs4.h | 22 +- > include/linux/xattr.h | 4 + > include/uapi/linux/nfs4.h | 3 + > 10 files changed, 1044 insertions(+), 40 deletions(-) > > -- > 2.17.2 > -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-07-29 12:24 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200623223927.31795-1-fllinden@amazon.com> 2020-06-23 22:39 ` [PATCH v3 01/10] xattr: break delegations in {set,remove}xattr Frank van der Linden 2020-06-25 20:39 ` Frank van der Linden 2020-07-14 17:11 ` Frank van der Linden 2020-07-01 19:33 ` Sasha Levin 2020-07-10 14:03 ` Sasha Levin 2020-07-10 14:08 ` Chuck Lever 2020-06-23 22:39 ` [PATCH v3 02/10] xattr: add a function to check if a namespace is supported Frank van der Linden 2020-06-25 20:41 ` Frank van der Linden 2020-07-14 17:13 ` Frank van der Linden 2020-07-14 18:46 ` Linus Torvalds 2020-07-28 14:17 ` Chuck Lever 2020-07-28 14:33 ` Al Viro 2020-07-29 12:23 ` Chuck Lever 2020-07-04 14:37 ` [PATCH v3 00/10] server side user xattr support (RFC 8276) Chuck Lever
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).