[Qemu-devel] [PATCH] 9pfs: fix readdir() for 9p2000.u

Greg Kurz posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/150574930075.27635.3945832472060840946.stgit@bahia
Test checkpatch passed
Test docker passed
Test s390x passed
hw/9pfs/9p.c |    3 +++
1 file changed, 3 insertions(+)
[Qemu-devel] [PATCH] 9pfs: fix readdir() for 9p2000.u
Posted by Greg Kurz 6 years, 6 months ago
If the client is using 9p2000.u, the following occurs:

$ cd ${virtfs_shared_dir}
$ mkdir -p a/b/c
$ ls a/b
ls: cannot access 'a/b/a': No such file or directory
ls: cannot access 'a/b/b': No such file or directory
a  b  c

instead of the expected:

$ ls a/b
c

This is a regression introduced by commit f57f5878578a;
local_name_to_path() now resolves ".." and "." in paths,
and v9fs_do_readdir_with_stat()->stat_to_v9stat() then
copies the basename of the resulting path to the response.
With the example above, this means that "." and ".." are
turned into "b" and "a" respectively...

Actually, the name we need to pass is the d_name field of
the dirent.

Signed-off-by: Greg Kurz <groug@kaod.org>
---

Hi Jan,

I found this will testing your patches. I'd appreciate if you could
review this. Then I'll push all these patches to 9p-next and send
a pull request.

Thanks,

--
Greg

 hw/9pfs/9p.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 0a37c8bd1361..0474e9e787c0 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1748,6 +1748,9 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
         if (err < 0) {
             break;
         }
+        v9fs_path_free(&path);
+
+        v9fs_path_sprintf(&path, "%s", dent->d_name);
         err = stat_to_v9stat(pdu, &path, &stbuf, &v9stat);
         if (err < 0) {
             break;


Re: [Qemu-devel] [PATCH] 9pfs: fix readdir() for 9p2000.u
Posted by Jan Dakinevich 6 years, 6 months ago
Hi Greg,

I am also thinking about this... :-(

As I see, stat_to_v9stat() is also used for symbolic links resolving and
requires full path, not only d_name.

With your patch I have the following:

$ ln -s /usr usr
$ ls -l
ls: reading directory .: Invalid argument
total 0

On 09/18/2017 06:46 PM, Greg Kurz wrote:
> If the client is using 9p2000.u, the following occurs:
> 
> $ cd ${virtfs_shared_dir}
> $ mkdir -p a/b/c
> $ ls a/b
> ls: cannot access 'a/b/a': No such file or directory
> ls: cannot access 'a/b/b': No such file or directory
> a  b  c
> 
> instead of the expected:
> 
> $ ls a/b
> c
> 
> This is a regression introduced by commit f57f5878578a;
> local_name_to_path() now resolves ".." and "." in paths,
> and v9fs_do_readdir_with_stat()->stat_to_v9stat() then
> copies the basename of the resulting path to the response.
> With the example above, this means that "." and ".." are
> turned into "b" and "a" respectively...
> 
> Actually, the name we need to pass is the d_name field of
> the dirent.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> 
> Hi Jan,
> 
> I found this will testing your patches. I'd appreciate if you could
> review this. Then I'll push all these patches to 9p-next and send
> a pull request.
> 
> Thanks,
> 
> --
> Greg
> 
>  hw/9pfs/9p.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 0a37c8bd1361..0474e9e787c0 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1748,6 +1748,9 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
>          if (err < 0) {
>              break;
>          }
> +        v9fs_path_free(&path);
> +
> +        v9fs_path_sprintf(&path, "%s", dent->d_name);
>          err = stat_to_v9stat(pdu, &path, &stbuf, &v9stat);
>          if (err < 0) {
>              break;
> 

-- 
Best regards
Jan Dakinevich