Adding more people.
The old stable trees seem to have rather different code.
[ Goes off and looks at the stable trees ]
Petr seems entirely correct - the stable tree backport appears broken.
Because looking at that commit 67a56e9743171 in the stable tree, it
doesn't seem to match commit 4b310319c6a8 ("NFS: Fix memory leaks and
corruption in readdir") in mainline.
That stable backport looks bogus. It added that
array = kmap(page);
line from somewhere else, probably because the stable tree didn't have
the line at all, and it was there in the context.
Because while mainline has that line to initialize array with kmap(),
in those stable trees, we have
array = nfs_readdir_get_array(page);
and as Petr says, the kmap has been done there already, and it will be
kunmap'ed by nfs_readdir_release_array().
And looking closer, this same bug seems to have happened twice: it
also exists in 0b0223f9c3a8.
But somebody else should double-check me - somebody who actually knows the code.
As to how I found the other case, do this in the stable git repo with
all the stable tags:
git log -p --no-merges --all \
--grep="NFS: Fix memory leaks and corruption in readdir"
to see all the copies of that commit backport.
Add a
-S'kmap(page)'
to that line to see the cases that added that line. Or to just get the commits:
git log --oneline --no-merges --all \
--grep="NFS: Fix memory leaks and corruption in readdir" \
-S'kmap(page)'
and the result is
67a56e974317 NFS: Fix memory leaks and corruption in readdir
0b0223f9c3a8 NFS: Fix memory leaks and corruption in readdir
Those two seem to be the 4.4 and 4.9 backports:
git name-rev 67a56e974317 0b0223f9c3a8
gives:
67a56e974317 tags/v4.9.214~38
0b0223f9c3a8 tags/v4.4.214~31
and I guess that makes sense - they are the really old ones.
Linus
On Fri, Mar 13, 2020 at 1:25 PM Petr Malat <oss@xxxxxxxxx> wrote:
>
> Array is mapped by nfs_readdir_get_array(), the further kmap is a result
> of a bad merge and should be removed.
>
> This resource leakage can be exploited for DoS by receptively reading
> a content of a directory on NFS (e.g. by running ls).
>
> Fixes: 67a56e9743171 ("NFS: Fix memory leaks and corruption in readdir")
> Signed-off-by: Petr Malat <oss@xxxxxxxxx>
> ---
> fs/nfs/dir.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index c2665d920cf8..2517fcd423b6 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -678,8 +678,6 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
> goto out_label_free;
> }
>
> - array = kmap(page);
> -
> status = nfs_readdir_alloc_pages(pages, array_size);
> if (status < 0)
> goto out_release_array;
> --
> 2.20.1
>
