Re: [PATCH 08/29] tools/xenlogd: add 9pfs walk request support

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Wed, Nov 1, 2023 at 6:09 AM Juergen Gross <jgross@xxxxxxxx> wrote:
>
> Add the walk request of the 9pfs protocol.
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  tools/xenlogd/io.c      | 138 ++++++++++++++++++++++++++++++++++++++++
>  tools/xenlogd/xenlogd.h |   1 +
>  2 files changed, 139 insertions(+)
>
> diff --git a/tools/xenlogd/io.c b/tools/xenlogd/io.c
> index fa825c9f39..778e1dc2c9 100644
> --- a/tools/xenlogd/io.c
> +++ b/tools/xenlogd/io.c

> @@ -600,6 +616,124 @@ static void p9_attach(device *device, struct p9_header 
> *hdr)
>      fill_buffer(device, hdr->cmd + 1, hdr->tag, "Q", &qid);
>  }
>
> +static void p9_walk(device *device, struct p9_header *hdr)
> +{
> +    uint32_t fid;
> +    uint32_t newfid;
> +    struct p9_fid *fidp;
> +    struct p9_qid *qids = NULL;
> +    unsigned int n_names = 0;
> +    unsigned int *names = NULL;
> +    unsigned int walked = 0;
> +    unsigned int i;
> +    char *path = NULL;
> +    unsigned int path_len;
> +    int ret;
> +
> +    ret = fill_data(device, "UUaS", &fid, &newfid, &n_names, &names);
> +    if ( n_names > P9_WALK_MAXELEM )
> +    {
> +        p9_error(device, hdr->tag, EINVAL);
> +        goto out;
> +    }
> +    if ( ret != 3 + n_names )
> +    {
> +        p9_error(device, hdr->tag, errno);
> +        goto out;
> +    }
> +
> +    fidp = find_fid(device, fid);
> +    if ( !fidp )
> +    {
> +        p9_error(device, hdr->tag, ENOENT);
> +        goto out;
> +    }
> +    if ( fidp->opened )
> +    {
> +        p9_error(device, hdr->tag, EINVAL);
> +        goto out;
> +    }

https://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor33
"""
The fid must represent a directory unless zero path name elements are specified.
"""
and
"""
For the first elementwise walk to succeed, the file identified by fid
must be a directory, and the implied user of the request must have
permission to search the directory (see intro(5)). Subsequent
elementwise walks have equivalent restrictions applied to the implicit
fid that results from the preceding elementwise walk.
"""

Maybe a dir check should be added?  However, it doesn't look like QEMU
does this check.  Seems like it is implicitly checked by the path
concatenation, so it may be fine as-is.

If you think it's ok as-is:
Reviewed-by: Jason Andryuk <jandryuk@xxxxxxxxx>

Regards,
Jason