Message ID | 20231101093325.30302-13-jgross@suse.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | tools: enable xenstore-stubdom to use 9pfs | expand |
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
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
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
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 --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);
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(+)