diff mbox series

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

Message ID 20231101093325.30302-9-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series tools: enable xenstore-stubdom to use 9pfs | expand

Commit Message

Jürgen Groß Nov. 1, 2023, 9:33 a.m. UTC
Add the walk request of the 9pfs protocol.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenlogd/io.c      | 138 ++++++++++++++++++++++++++++++++++++++++
 tools/xenlogd/xenlogd.h |   1 +
 2 files changed, 139 insertions(+)

Comments

Jason Andryuk Nov. 3, 2023, 7:48 p.m. UTC | #1
On Wed, Nov 1, 2023 at 6:09 AM Juergen Gross <jgross@suse.com> wrote:
>
> Add the walk request of the 9pfs protocol.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  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@gmail.com>

Regards,
Jason
Jürgen Groß Nov. 6, 2023, 7:53 a.m. UTC | #2
On 03.11.23 20:48, Jason Andryuk wrote:
> On Wed, Nov 1, 2023 at 6:09 AM Juergen Gross <jgross@suse.com> wrote:
>>
>> Add the walk request of the 9pfs protocol.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   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@gmail.com>

Thanks.

I think I'll leave it as is.


Juergen
diff mbox series

Patch

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
@@ -27,9 +27,11 @@ 
 #define P9_CMD_VERSION    100
 #define P9_CMD_ATTACH     104
 #define P9_CMD_ERROR      107
+#define P9_CMD_WALK       110
 
 #define P9_MIN_MSIZE      2048
 #define P9_VERSION        "9P2000.u"
+#define P9_WALK_MAXELEM   16
 
 struct p9_qid {
     uint8_t type;
@@ -523,6 +525,20 @@  static int fill_qid(const char *path, struct p9_qid *qid, struct stat *stbuf)
     return 0;
 }
 
+static bool name_ok(const char *str)
+{
+    if ( !*str )
+        return false;
+
+    if ( strchr(str, '/' ) )
+        return false;
+
+    if ( !strcmp(str, "..") || !strcmp(str, ".") )
+        return false;
+
+    return true;
+}
+
 static void p9_error(device *device, uint16_t tag, uint32_t err)
 {
     unsigned int erroff;
@@ -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;
+    }
+
+    path_len = strlen(fidp->path) + 1;
+    for ( i = 0; i < n_names; i++ )
+    {
+        if ( !name_ok(device->str + names[i]) )
+        {
+            p9_error(device, hdr->tag, ENOENT);
+            goto out;
+        }
+        path_len += strlen(device->str + names[i]) + 1;
+    }
+    path = calloc(path_len + 1, 1);
+    if ( !path )
+    {
+        p9_error(device, hdr->tag, ENOMEM);
+        goto out;
+    }
+    strcpy(path, fidp->path);
+
+    if ( n_names )
+    {
+        qids = calloc(n_names, sizeof(*qids));
+        if ( !qids )
+        {
+            p9_error(device, hdr->tag, ENOMEM);
+            goto out;
+        }
+        for ( i = 0; i < n_names; i++ )
+        {
+            strcat(path, "/");
+            strcat(path, device->str + names[i]);
+            ret = fill_qid(path, qids + i, NULL);
+            if ( ret )
+            {
+                if ( !walked )
+                {
+                    p9_error(device, hdr->tag, errno);
+                    goto out;
+                }
+                break;
+            }
+            walked++;
+        }
+    }
+
+    if ( walked == n_names )
+    {
+        const char *rel_path = path + strlen(device->host_path);
+        bool ok = false;
+
+        if ( fid == newfid )
+        {
+            struct p9_fid *new_fidp;
+
+            new_fidp = alloc_fid_mem(device, fid, rel_path);
+            if ( new_fidp )
+            {
+                XEN_TAILQ_REMOVE(&device->fids, fidp, list);
+                XEN_TAILQ_INSERT_HEAD(&device->fids, new_fidp, list);
+                free(fidp);
+                ok = true;
+            }
+        }
+        else
+            ok = alloc_fid(device, newfid, rel_path);
+
+        if ( !ok )
+        {
+            p9_error(device, hdr->tag, errno);
+            goto out;
+        }
+    }
+
+    fill_buffer(device, hdr->cmd + 1, hdr->tag, "aQ", &walked, qids);
+
+ out:
+    free(qids);
+    free(path);
+    free(names);
+}
+
 void *io_thread(void *arg)
 {
     device *device = arg;
@@ -663,6 +797,10 @@  void *io_thread(void *arg)
                 p9_attach(device, &hdr);
                 break;
 
+            case P9_CMD_WALK:
+                p9_walk(device, &hdr);
+                break;
+
             default:
                 syslog(LOG_DEBUG, "%u.%u sent unhandled command %u\n",
                        device->domid, device->devid, hdr.cmd);
diff --git a/tools/xenlogd/xenlogd.h b/tools/xenlogd/xenlogd.h
index bd2a283ccb..23f013af9e 100644
--- a/tools/xenlogd/xenlogd.h
+++ b/tools/xenlogd/xenlogd.h
@@ -22,6 +22,7 @@  struct p9_header {
 struct p9_fid {
     XEN_TAILQ_ENTRY(struct p9_fid) list;
     unsigned int fid;
+    bool opened;
     char path[];
 };