Received-SPF: pass (google.com: domain of linux-nfs+bounces-915-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <170429478711.50646.12675561629884992953.stgit@bazille.1015granger.net>
 <170429517779.50646.9656897459585544068.stgit@bazille.1015granger.net>
In-Reply-To: <170429517779.50646.9656897459585544068.stgit@bazille.1015granger.net>
From: Amir Goldstein <amir73il@gmail.com>
Date: Thu, 4 Jan 2024 09:39:04 +0200
Message-ID: <CAOQ4uxgMLWGqqoSNvSgB=Qfmw6Brk2eO6yB7FZqX6p-DcTiUtw@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] exportfs: fix the fallback implementation of the
 get_name export operation
To: Chuck Lever <cel@kernel.org>
Cc: jlayton@redhat.com, Trond Myklebust <trond.myklebust@hammerspace.com>, 
	Jeff Layton <jlayton@kernel.org>, Chuck Lever <chuck.lever@oracle.com>, 
	linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org, 
	trondmy@hammerspace.com, viro@zeniv.linux.org.uk, brauner@kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 3, 2024 at 5:19=E2=80=AFPM Chuck Lever <cel@kernel.org> wrote:
>
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> The fallback implementation for the get_name export operation uses
> readdir() to try to match the inode number to a filename. That filename
> is then used together with lookup_one() to produce a dentry.
> A problem arises when we match the '.' or '..' entries, since that
> causes lookup_one() to fail. This has sometimes been seen to occur for
> filesystems that violate POSIX requirements around uniqueness of inode
> numbers, something that is common for snapshot directories.
>
> This patch just ensures that we skip '.' and '..' rather than allowing a
> match.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Acked-by: Amir Goldstein <amir73il@gmail.com>
> Link: https://lore.kernel.org/linux-nfs/CAOQ4uxiOZobN76OKB-VBNXWeFKVwLW_e=
K5QtthGyYzWU9mjb7Q@mail.gmail.com/
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/exportfs/expfs.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 3ae0154c5680..84af58eaf2ca 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -255,7 +255,9 @@ static bool filldir_one(struct dir_context *ctx, cons=
t char *name, int len,
>                 container_of(ctx, struct getdents_callback, ctx);
>
>         buf->sequence++;
> -       if (buf->ino =3D=3D ino && len <=3D NAME_MAX) {
> +       /* Ignore the '.' and '..' entries */
> +       if ((len > 2 || name[0] !=3D '.' || (len =3D=3D 2 && name[1] !=3D=
 '.')) &&
> +           buf->ino =3D=3D ino && len <=3D NAME_MAX) {


Thank you for creating the helper, but if you already went to this trouble,
I think it is better to introduce is_dot_dotdot() as a local helper already
in this backportable patch, so that stable kernel code is same as upstream
code (good for future fixes) and then dedupe the local helper with the rest
of the local helpers in patch 2?

Thanks,
Amir.