qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC 3/3] tests/vhost-user-fs-test: add vhost-user-fs test case


From: Dr. David Alan Gilbert
Subject: Re: [RFC 3/3] tests/vhost-user-fs-test: add vhost-user-fs test case
Date: Thu, 7 Nov 2019 12:26:12 +0000
User-agent: Mutt/1.12.1 (2019-06-15)

* Stefan Hajnoczi (address@hidden) wrote:
> On Tue, Oct 29, 2019 at 12:36:05AM +0000, Dr. David Alan Gilbert wrote:
> > * Stefan Hajnoczi (address@hidden) wrote:
> > > +static void after_test(void *arg G_GNUC_UNUSED)
> > > +{
> > > +    unlink(socket_path);
> > > +
> > > +    remove_dir_and_children(shared_dir);
> > 
> > This scares me. Especially since it's running as root.
> > Can we add a bunch of paranoid checks to make sure it doesn't
> > end up rm -rf / ?
> 
> Yes, we can resolve the path and check it is not "/".

I suggest checking for "/", ".", ".." and ""
if any of those get in it's probably bad.

> > > +/* Open a file by nodeid using FUSE_OPEN */
> > > +static int32_t fuse_open(QVirtioFS *vfs, uint64_t nodeid, uint32_t flags,
> > > +                         uint64_t *fh)
> > > +{
> > > +    struct fuse_in_header in_hdr = {
> > > +        .opcode = guest32(FUSE_OPEN),
> > > +        .unique = guest64(virtio_fs_get_unique(vfs)),
> > > +        .nodeid = guest64(nodeid),
> > > +    };
> > > +    struct fuse_open_in in = {
> > > +        .flags = guest32(flags),
> > > +    };
> > > +    struct iovec sg_in[] = {
> > > +        { .iov_base = &in_hdr, .iov_len = sizeof(in_hdr) },
> > > +        { .iov_base = &in, .iov_len = sizeof(in) },
> > > +    };
> > > +    struct fuse_out_header out_hdr;
> > > +    struct fuse_open_out out;
> > > +    struct iovec sg_out[] = {
> > > +        { .iov_base = &out_hdr, .iov_len = sizeof(out_hdr) },
> > > +        { .iov_base = &out, .iov_len = sizeof(out) },
> > > +    };
> > 
> > I wonder if anything can be done to reduce the size of the iovec boiler
> > plate?
> 
> I'm not aware of a clean way to build the iovec array automatically but
> we could do this if you prefer it:
> 
>   #define IOVEC(elem) { .iov_base = &elem, .iov_len = sizeof(elem) }
> 
>   struct iovec sg_in[] = {
>     IOVEC(in_hdr),
>     IOVEC(in),
>   };
> 
> Do you find this nicer?

Only a little; probably not worth it.

Dave

> Stefan


--
Dr. David Alan Gilbert / address@hidden / Manchester, UK




reply via email to

[Prev in Thread] Current Thread [Next in Thread]