Re: [PATCH 01/24] lustre: rip the private symlink nesting limit out

[Andreas Dilger]

On Apr 20, 2015, at 12:12 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> 
> From: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> 
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>

Al, the patch itself looks good, thanks.

However, if this is applied at the start of the series it could
allow tests to easily cause a stack overflow during a bisection (I
don't think users would see a kernel in the middle of the series).

Could this be converted over to checking nd->link_count along with
the [02/24] patch until closer to the end of the series when the
recursion has been removed?

It isn't fatal if that doesn't happen, since this whole series should
land at one time and the chance of testing Lustre symlinks right
in the middle of the series is low, just something I thought when
reviewing the patch.

Cheers, Andreas

> ---
> drivers/staging/lustre/lustre/llite/symlink.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/symlink.c b/drivers/staging/lustre/lustre/llite/symlink.c
> index 3711e67..0615f86 100644
> --- a/drivers/staging/lustre/lustre/llite/symlink.c
> +++ b/drivers/staging/lustre/lustre/llite/symlink.c
> @@ -126,18 +126,9 @@ static void *ll_follow_link(struct dentry *dentry, struct nameidata *nd)
> 	char *symname = NULL;
> 
> 	CDEBUG(D_VFSTRACE, "VFS Op\n");
> -	/* Limit the recursive symlink depth to 5 instead of default
> -	 * 8 links when kernel has 4k stack to prevent stack overflow.
> -	 * For 8k stacks we need to limit it to 7 for local servers. */
> -	if (THREAD_SIZE < 8192 && current->link_count >= 6) {
> -		rc = -ELOOP;
> -	} else if (THREAD_SIZE == 8192 && current->link_count >= 8) {
> -		rc = -ELOOP;
> -	} else {
> -		ll_inode_size_lock(inode);
> -		rc = ll_readlink_internal(inode, &request, &symname);
> -		ll_inode_size_unlock(inode);
> -	}
> +	ll_inode_size_lock(inode);
> +	rc = ll_readlink_internal(inode, &request, &symname);
> +	ll_inode_size_unlock(inode);
> 	if (rc) {
> 		ptlrpc_req_finished(request);
> 		request = NULL;
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/