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

2023-11-07 Thread Jason Andryuk
On Tue, Nov 7, 2023 at 9:48 AM Jason Andryuk  wrote:
>
> On Tue, Nov 7, 2023 at 9:42 AM Juergen Gross  wrote:
> >
> > On 07.11.23 15:04, Jason Andryuk wrote:
> > > On Wed, Nov 1, 2023 at 5:34 AM Juergen Gross  wrote:
> > >>
> > >> Add the stat request of the 9pfs protocol.
> > >>
> > >> Signed-off-by: Juergen Gross 

Reviewed-by: Jason Andryuk 



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

2023-11-07 Thread Jason Andryuk
On Tue, Nov 7, 2023 at 9:42 AM Juergen Gross  wrote:
>
> On 07.11.23 15:04, Jason Andryuk wrote:
> > On Wed, Nov 1, 2023 at 5:34 AM Juergen Gross  wrote:
> >>
> >> Add the stat request of the 9pfs protocol.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >>   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, >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



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

2023-11-07 Thread Juergen Gross

On 07.11.23 15:04, Jason Andryuk wrote:

On Wed, Nov 1, 2023 at 5:34 AM Juergen Gross  wrote:


Add the stat request of the 9pfs protocol.

Signed-off-by: Juergen Gross 
---
  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, >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


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature


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

2023-11-07 Thread Jason Andryuk
On Wed, Nov 1, 2023 at 5:34 AM Juergen Gross  wrote:
>
> Add the stat request of the 9pfs protocol.
>
> Signed-off-by: Juergen Gross 
> ---
>  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, >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



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

2023-11-01 Thread Juergen Gross
Add the stat request of the 9pfs protocol.

Signed-off-by: Juergen Gross 
---
 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 @@
 #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_OREAD0   /* 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, >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", );
+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, ) < 0 )
+{
+p9_error(device, hdr->tag, errno);
+return;
+}
+fill_p9_stat(, , strrchr(fidp->path, '/') + 1);
+
+total_length = p9s.size + sizeof(p9s.size);
+fill_buffer(device, hdr->cmd + 1, hdr->tag, "uuuUQUUULSUUU",
+_length, , , , ,
+, , , , p9s.name,
+p9s.uid, p9s.gid, p9s.muid, p9s.extension, _uid,
+_gid, _muid);
+
+}
+
 void *io_thread(void *arg)
 {
 device *device = arg;
@@ -1101,6 +1186,10 @@ void *io_thread(void *arg)
 p9_clunk(device, );
 break;
 
+case P9_CMD_STAT:
+p9_stat(device, );
+break;
+
 default:
 syslog(LOG_DEBUG, "%u.%u sent unhandled command %u\n",
device->domid, device->devid, hdr.cmd);
-- 
2.35.3