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

[Jason Andryuk <jandryuk@xxxxxxxxx>]

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