diff mbox series

[12/29] tools/xenlogd: add 9pfs stat request support

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

Commit Message

Juergen Gross Nov. 1, 2023, 9:33 a.m. UTC
Add the stat request of the 9pfs protocol.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenlogd/io.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

Comments

Jason Andryuk Nov. 7, 2023, 2:04 p.m. UTC | #1
On Wed, Nov 1, 2023 at 5:34 AM Juergen Gross <jgross@suse.com> wrote:
>
> Add the stat request of the 9pfs protocol.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  tools/xenlogd/io.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
>
> diff --git a/tools/xenlogd/io.c b/tools/xenlogd/io.c
> index 34f137be1b..6e92667fab 100644
> --- a/tools/xenlogd/io.c
> +++ b/tools/xenlogd/io.c
> @@ -33,6 +33,7 @@

> +static void fill_p9_stat(struct p9_stat *p9s, struct stat *st, const char *name)
> +{
> +    memset(p9s, 0, sizeof(*p9s));
> +    fill_qid(NULL, &p9s->qid, st);
> +    p9s->mode = st->st_mode & 0777;
> +    if ( S_ISDIR(st->st_mode) )
> +        p9s->mode |= P9_CREATE_PERM_DIR;
> +    p9s->atime = st->st_atime;
> +    p9s->mtime = st->st_mtime;
> +    p9s->length = st->st_size;
> +    p9s->name = name;
> +    p9s->uid = "";
> +    p9s->gid = "";
> +    p9s->muid = "";
> +    p9s->extension = "";
> +    p9s->n_uid = 0;
> +    p9s->n_gid = 0;

If the daemon is running as root and managing the directories, these
probably match.  Still, do we want uid & gid to be populated from the
stat struct?

> +    p9s->n_muid = 0;
> +
> +    /*
> +     * Size of individual fields without the size field, including 5 2-byte
> +     * string length fields.
> +     */
> +    p9s->size = 71 + strlen(p9s->name);
> +}
> +
> +static void p9_stat(device *device, struct p9_header *hdr)
> +{
> +    uint32_t fid;
> +    struct p9_fid *fidp;
> +    struct p9_stat p9s;
> +    struct stat st;
> +    uint16_t total_length;

total_length = 0;

Otherwise it is used uninitialized.

Regards,
Jason
Juergen Gross Nov. 7, 2023, 2:42 p.m. UTC | #2
On 07.11.23 15:04, Jason Andryuk wrote:
> On Wed, Nov 1, 2023 at 5:34 AM Juergen Gross <jgross@suse.com> wrote:
>>
>> Add the stat request of the 9pfs protocol.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   tools/xenlogd/io.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 89 insertions(+)
>>
>> diff --git a/tools/xenlogd/io.c b/tools/xenlogd/io.c
>> index 34f137be1b..6e92667fab 100644
>> --- a/tools/xenlogd/io.c
>> +++ b/tools/xenlogd/io.c
>> @@ -33,6 +33,7 @@
> 
>> +static void fill_p9_stat(struct p9_stat *p9s, struct stat *st, const char *name)
>> +{
>> +    memset(p9s, 0, sizeof(*p9s));
>> +    fill_qid(NULL, &p9s->qid, st);
>> +    p9s->mode = st->st_mode & 0777;
>> +    if ( S_ISDIR(st->st_mode) )
>> +        p9s->mode |= P9_CREATE_PERM_DIR;
>> +    p9s->atime = st->st_atime;
>> +    p9s->mtime = st->st_mtime;
>> +    p9s->length = st->st_size;
>> +    p9s->name = name;
>> +    p9s->uid = "";
>> +    p9s->gid = "";
>> +    p9s->muid = "";
>> +    p9s->extension = "";
>> +    p9s->n_uid = 0;
>> +    p9s->n_gid = 0;
> 
> If the daemon is running as root and managing the directories, these
> probably match.  Still, do we want uid & gid to be populated from the
> stat struct?

I wouldn't want to do that. In the end the permissions of the daemon are
relevant for being able to access the files. There is no need to leak any
uids and gids from the host to the guests.

> 
>> +    p9s->n_muid = 0;
>> +
>> +    /*
>> +     * Size of individual fields without the size field, including 5 2-byte
>> +     * string length fields.
>> +     */
>> +    p9s->size = 71 + strlen(p9s->name);
>> +}
>> +
>> +static void p9_stat(device *device, struct p9_header *hdr)
>> +{
>> +    uint32_t fid;
>> +    struct p9_fid *fidp;
>> +    struct p9_stat p9s;
>> +    struct stat st;
>> +    uint16_t total_length;
> 
> total_length = 0;
> 
> Otherwise it is used uninitialized.

I don't think so. There is a single user just after setting the variable.


Juergen
Jason Andryuk Nov. 7, 2023, 2:48 p.m. UTC | #3
On Tue, Nov 7, 2023 at 9:42 AM Juergen Gross <jgross@suse.com> wrote:
>
> On 07.11.23 15:04, Jason Andryuk wrote:
> > On Wed, Nov 1, 2023 at 5:34 AM Juergen Gross <jgross@suse.com> wrote:
> >>
> >> Add the stat request of the 9pfs protocol.
> >>
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> >> ---
> >>   tools/xenlogd/io.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 89 insertions(+)
> >>
> >> diff --git a/tools/xenlogd/io.c b/tools/xenlogd/io.c
> >> index 34f137be1b..6e92667fab 100644
> >> --- a/tools/xenlogd/io.c
> >> +++ b/tools/xenlogd/io.c
> >> @@ -33,6 +33,7 @@
> >
> >> +static void fill_p9_stat(struct p9_stat *p9s, struct stat *st, const char *name)
> >> +{
> >> +    memset(p9s, 0, sizeof(*p9s));
> >> +    fill_qid(NULL, &p9s->qid, st);
> >> +    p9s->mode = st->st_mode & 0777;
> >> +    if ( S_ISDIR(st->st_mode) )
> >> +        p9s->mode |= P9_CREATE_PERM_DIR;
> >> +    p9s->atime = st->st_atime;
> >> +    p9s->mtime = st->st_mtime;
> >> +    p9s->length = st->st_size;
> >> +    p9s->name = name;
> >> +    p9s->uid = "";
> >> +    p9s->gid = "";
> >> +    p9s->muid = "";
> >> +    p9s->extension = "";
> >> +    p9s->n_uid = 0;
> >> +    p9s->n_gid = 0;
> >
> > If the daemon is running as root and managing the directories, these
> > probably match.  Still, do we want uid & gid to be populated from the
> > stat struct?
>
> I wouldn't want to do that. In the end the permissions of the daemon are
> relevant for being able to access the files. There is no need to leak any
> uids and gids from the host to the guests.

Ok.

> >
> >> +    p9s->n_muid = 0;
> >> +
> >> +    /*
> >> +     * Size of individual fields without the size field, including 5 2-byte
> >> +     * string length fields.
> >> +     */
> >> +    p9s->size = 71 + strlen(p9s->name);
> >> +}
> >> +
> >> +static void p9_stat(device *device, struct p9_header *hdr)
> >> +{
> >> +    uint32_t fid;
> >> +    struct p9_fid *fidp;
> >> +    struct p9_stat p9s;
> >> +    struct stat st;
> >> +    uint16_t total_length;
> >
> > total_length = 0;
> >
> > Otherwise it is used uninitialized.
>
> I don't think so. There is a single user just after setting the variable.

Whoops - you are right.  Sorry for the noise.

Regards,
Jason
Jason Andryuk Nov. 7, 2023, 3:20 p.m. UTC | #4
On Tue, Nov 7, 2023 at 9:48 AM Jason Andryuk <jandryuk@gmail.com> wrote:
>
> On Tue, Nov 7, 2023 at 9:42 AM Juergen Gross <jgross@suse.com> wrote:
> >
> > On 07.11.23 15:04, Jason Andryuk wrote:
> > > On Wed, Nov 1, 2023 at 5:34 AM Juergen Gross <jgross@suse.com> wrote:
> > >>
> > >> Add the stat request of the 9pfs protocol.
> > >>
> > >> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
diff mbox series

Patch

diff --git a/tools/xenlogd/io.c b/tools/xenlogd/io.c
index 34f137be1b..6e92667fab 100644
--- a/tools/xenlogd/io.c
+++ b/tools/xenlogd/io.c
@@ -33,6 +33,7 @@ 
 #define P9_CMD_OPEN       112
 #define P9_CMD_CREATE     114
 #define P9_CMD_CLUNK      120
+#define P9_CMD_STAT       124
 
 /* P9 protocol open flags. */
 #define P9_OREAD            0   /* read */
@@ -59,6 +60,25 @@  struct p9_qid {
     uint64_t path;
 };
 
+struct p9_stat {
+    uint16_t size;
+    uint16_t type;
+    uint32_t dev;
+    struct p9_qid qid;
+    uint32_t mode;
+    uint32_t atime;
+    uint32_t mtime;
+    uint64_t length;
+    const char *name;
+    const char *uid;
+    const char *gid;
+    const char *muid;
+    const char *extension;
+    uint32_t n_uid;
+    uint32_t n_gid;
+    uint32_t n_muid;
+};
+
 /*
  * Note that the ring names "in" and "out" are from the frontend's
  * perspective, so the "in" ring will be used for responses to the frontend,
@@ -1022,6 +1042,71 @@  static void p9_clunk(device *device, struct p9_header *hdr)
     fill_buffer(device, hdr->cmd + 1, hdr->tag, "");
 }
 
+static void fill_p9_stat(struct p9_stat *p9s, struct stat *st, const char *name)
+{
+    memset(p9s, 0, sizeof(*p9s));
+    fill_qid(NULL, &p9s->qid, st);
+    p9s->mode = st->st_mode & 0777;
+    if ( S_ISDIR(st->st_mode) )
+        p9s->mode |= P9_CREATE_PERM_DIR;
+    p9s->atime = st->st_atime;
+    p9s->mtime = st->st_mtime;
+    p9s->length = st->st_size;
+    p9s->name = name;
+    p9s->uid = "";
+    p9s->gid = "";
+    p9s->muid = "";
+    p9s->extension = "";
+    p9s->n_uid = 0;
+    p9s->n_gid = 0;
+    p9s->n_muid = 0;
+
+    /*
+     * Size of individual fields without the size field, including 5 2-byte
+     * string length fields.
+     */
+    p9s->size = 71 + strlen(p9s->name);
+}
+
+static void p9_stat(device *device, struct p9_header *hdr)
+{
+    uint32_t fid;
+    struct p9_fid *fidp;
+    struct p9_stat p9s;
+    struct stat st;
+    uint16_t total_length;
+    int ret;
+
+    ret = fill_data(device, "U", &fid);
+    if ( ret != 1 )
+    {
+        p9_error(device, hdr->tag, EINVAL);
+        return;
+    }
+
+    fidp = find_fid(device, fid);
+    if ( !fidp )
+    {
+        p9_error(device, hdr->tag, ENOENT);
+        return;
+    }
+
+    if ( stat(fidp->path, &st) < 0 )
+    {
+        p9_error(device, hdr->tag, errno);
+        return;
+    }
+    fill_p9_stat(&p9s, &st, strrchr(fidp->path, '/') + 1);
+
+    total_length = p9s.size + sizeof(p9s.size);
+    fill_buffer(device, hdr->cmd + 1, hdr->tag, "uuuUQUUULSSSSSUUU",
+                &total_length, &p9s.size, &p9s.type, &p9s.dev, &p9s.qid,
+                &p9s.mode, &p9s.atime, &p9s.mtime, &p9s.length, p9s.name,
+                p9s.uid, p9s.gid, p9s.muid, p9s.extension, &p9s.n_uid,
+                &p9s.n_gid, &p9s.n_muid);
+
+}
+
 void *io_thread(void *arg)
 {
     device *device = arg;
@@ -1101,6 +1186,10 @@  void *io_thread(void *arg)
                 p9_clunk(device, &hdr);
                 break;
 
+            case P9_CMD_STAT:
+                p9_stat(device, &hdr);
+                break;
+
             default:
                 syslog(LOG_DEBUG, "%u.%u sent unhandled command %u\n",
                        device->domid, device->devid, hdr.cmd);