[Virtio-fs] [PATCH v3 09/26] DAX: virtio-fs: Add vhost-user slave commands for mapping
Dr. David Alan Gilbert
dgilbert at redhat.com
Thu May 27 16:57:21 UTC 2021
* Stefan Hajnoczi (stefanha at redhat.com) wrote:
> On Wed, Apr 28, 2021 at 12:00:43PM +0100, Dr. David Alan Gilbert (git) wrote:
> > diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
> > index dd0a02aa99..169a146e72 100644
> > --- a/hw/virtio/vhost-user-fs.c
> > +++ b/hw/virtio/vhost-user-fs.c
> > @@ -45,6 +45,72 @@ static const int user_feature_bits[] = {
> > #define DAX_WINDOW_PROT PROT_NONE
> > #endif
> >
> > +/*
> > + * The message apparently had 'received_size' bytes, check this
> > + * matches the count in the message.
> > + *
> > + * Returns true if the size matches.
> > + */
> > +static bool check_slave_message_entries(const VhostUserFSSlaveMsg *sm,
> > + int received_size)
> > +{
> > + int tmp;
> > +
> > + /*
> > + * VhostUserFSSlaveMsg consists of a body followed by 'n' entries,
> > + * (each VhostUserFSSlaveMsgEntry). There's a maximum of
> > + * VHOST_USER_FS_SLAVE_MAX_ENTRIES of these.
> > + */
> > + if (received_size <= sizeof(VhostUserFSSlaveMsg)) {
>
> received_size is an int and we risk hitting checking against the coerced
> size_t value but then using the signed int received_size later. It's
> safer to convert to size_t once and then use that size_t value
> everywhere later.
Done.
> > +typedef struct {
> > + /* Offsets within the file being mapped */
> > + uint64_t fd_offset;
> > + /* Offsets within the cache */
> > + uint64_t c_offset;
> > + /* Lengths of sections */
> > + uint64_t len;
> > + /* Flags, from VHOST_USER_FS_FLAG_* */
> > + uint64_t flags;
> > +} VhostUserFSSlaveMsgEntry;
> > +
> > +typedef struct {
> > + /* Number of entries */
> > + uint16_t count;
> > + /* Spare */
> > + uint16_t align;
>
> VhostUserFSSlaveMsgEntry is aligned to 64 bits, so the 16-bit align
> field is not sufficient for full alignment.
Ah, interesting; fixed to:
typedef struct {
/* Generic flags for the overall message */
uint32_t flags;
/* Number of entries */
uint16_t count;
/* Spare */
uint16_t align;
} VhostUserFSSlaveMsgHdr;
or do I actually need to have a uint64_t in there?
Dave
--
Dr. David Alan Gilbert / dgilbert at redhat.com / Manchester, UK
More information about the Virtio-fs
mailing list