> On Jan 11, 2021, at 4:01 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
>
> From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
>
> If you export a subdirectory of a filesystem, a READDIRPLUS on the root
> of that export will return the filehandle of the parent with the ".."
> entry.
>
> The filehandle is optional, so let's just not return the filehandle for
> ".." if we're at the root of an export.
>
> Note that once the client learns one filehandle outside of the export,
> they can trivially access the rest of the export using further lookups.
>
> However, it is also not very difficult to guess filehandles outside of
> the export. So exporting a subdirectory of a filesystem should
> considered equivalent to providing access to the entire filesystem. To
> avoid confusion, we recommend only exporting entire filesystems.
>
> Reported-by: 吴异 <wangzhibei1999@xxxxxxxxx>
> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
> ---
> fs/nfsd/nfs3xdr.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index 821db21ba072..34b880211e5e 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -865,9 +865,14 @@ compose_entry_fh(struct nfsd3_readdirres *cd, struct svc_fh *fhp,
> if (isdotent(name, namlen)) {
> if (namlen == 2) {
> dchild = dget_parent(dparent);
> - /* filesystem root - cannot return filehandle for ".." */
> + /*
> + * Don't return filehandle for ".." if we're at
> OA+ * the filesystem or export root:
> + */
> if (dchild == dparent)
> goto out;
> + if (dparent == exp->ex_path.dentry)
> + goto out;
> } else
> dchild = dget(dparent);
> } else
> --
> 2.29.2
Thanks for the fix!
I've replaced the Reported-by: tag and pushed this to my
cel-next topic branch, and intend to submit it with the
next 5.11 -rc pull request. See:
https://git.linux-nfs.org/?p=cel/cel-2.6.git;a=shortlog;h=refs/heads/cel-next
Is there additional context that should be added? A Link:
tag that points to the discussion on security@ perhaps?
Note there was some damage in the patch body: there's a
spurious "OA" in the hunk that had to be removed before
the patch would apply.
--
Chuck Lever
