| Message ID | 149868266150.23385.2902323487462062636.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
On Wed, 28 Jun 2017, Greg Kurz wrote: > The 9p spec at http://man.cat-v.org/plan_9/5/intro reads: > > "Each 9P message begins with a four-byte size field specify- > ing the length in bytes of the complete message including > the four bytes of the size field itself. The next byte is > the message type, one of the constants in the enumeration in > the include file <fcall.h>. The next two bytes are an iden- > tifying tag, described below." > > ie, each message starts with a 7-byte long header. > > The core 9P code already assumes this pretty much everywhere. This patch > does the following: > - makes the assumption explicit in the common 9p.h header, since it isn't > related to the transport > - open codes the header size in handle_9p_output() and hardens the sanity > check on the space needed for the reply message > > Signed-off-by: Greg Kurz <groug@kaod.org> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > hw/9pfs/9p.h | 5 +++++ > hw/9pfs/virtio-9p-device.c | 8 +++----- > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h > index c886ba78d2ee..aac1b0b2ce3d 100644 > --- a/hw/9pfs/9p.h > +++ b/hw/9pfs/9p.h > @@ -124,6 +124,11 @@ typedef struct { > uint8_t id; > uint16_t tag_le; > } QEMU_PACKED P9MsgHeader; > +/* According to the specification, 9p messages start with a 7-byte header. > + * Since most of the code uses this header size in literal form, we must be > + * sure this is indeed the case. > + */ > +QEMU_BUILD_BUG_ON(sizeof(P9MsgHeader) != 7); > > struct V9fsPDU > { > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c > index 3380bfc0c551..1a68c1622d3a 100644 > --- a/hw/9pfs/virtio-9p-device.c > +++ b/hw/9pfs/virtio-9p-device.c > @@ -53,17 +53,15 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq) > goto out_free_pdu; > } > > - if (elem->in_num == 0) { > + if (iov_size(elem->in_sg, elem->in_num) < 7) { > virtio_error(vdev, > "The guest sent a VirtFS request without space for " > "the reply"); > goto out_free_req; > } > - QEMU_BUILD_BUG_ON(sizeof(out) != 7); > > - len = iov_to_buf(elem->out_sg, elem->out_num, 0, > - &out, sizeof(out)); > - if (len != sizeof(out)) { > + len = iov_to_buf(elem->out_sg, elem->out_num, 0, &out, 7); > + if (len != 7) { > virtio_error(vdev, "The guest sent a malformed VirtFS request: " > "header size is %zd, should be 7", len); > goto out_free_req; >
On Wed, Jun 28, 2017 at 10:44:21PM +0200, Greg Kurz wrote: > The 9p spec at http://man.cat-v.org/plan_9/5/intro reads: > > "Each 9P message begins with a four-byte size field specify- > ing the length in bytes of the complete message including > the four bytes of the size field itself. The next byte is > the message type, one of the constants in the enumeration in > the include file <fcall.h>. The next two bytes are an iden- > tifying tag, described below." > > ie, each message starts with a 7-byte long header. > > The core 9P code already assumes this pretty much everywhere. This patch > does the following: > - makes the assumption explicit in the common 9p.h header, since it isn't > related to the transport > - open codes the header size in handle_9p_output() and hardens the sanity > check on the space needed for the reply message > > Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/9pfs/9p.h | 5 +++++ > hw/9pfs/virtio-9p-device.c | 8 +++----- > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h > index c886ba78d2ee..aac1b0b2ce3d 100644 > --- a/hw/9pfs/9p.h > +++ b/hw/9pfs/9p.h > @@ -124,6 +124,11 @@ typedef struct { > uint8_t id; > uint16_t tag_le; > } QEMU_PACKED P9MsgHeader; > +/* According to the specification, 9p messages start with a 7-byte header. > + * Since most of the code uses this header size in literal form, we must be > + * sure this is indeed the case. > + */ > +QEMU_BUILD_BUG_ON(sizeof(P9MsgHeader) != 7); > > struct V9fsPDU > { > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c > index 3380bfc0c551..1a68c1622d3a 100644 > --- a/hw/9pfs/virtio-9p-device.c > +++ b/hw/9pfs/virtio-9p-device.c > @@ -53,17 +53,15 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq) > goto out_free_pdu; > } > > - if (elem->in_num == 0) { > + if (iov_size(elem->in_sg, elem->in_num) < 7) { > virtio_error(vdev, > "The guest sent a VirtFS request without space for " > "the reply"); > goto out_free_req; > } > - QEMU_BUILD_BUG_ON(sizeof(out) != 7); > > - len = iov_to_buf(elem->out_sg, elem->out_num, 0, > - &out, sizeof(out)); > - if (len != sizeof(out)) { > + len = iov_to_buf(elem->out_sg, elem->out_num, 0, &out, 7); > + if (len != 7) { > virtio_error(vdev, "The guest sent a malformed VirtFS request: " > "header size is %zd, should be 7", len); > goto out_free_req;
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index c886ba78d2ee..aac1b0b2ce3d 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -124,6 +124,11 @@ typedef struct { uint8_t id; uint16_t tag_le; } QEMU_PACKED P9MsgHeader; +/* According to the specification, 9p messages start with a 7-byte header. + * Since most of the code uses this header size in literal form, we must be + * sure this is indeed the case. + */ +QEMU_BUILD_BUG_ON(sizeof(P9MsgHeader) != 7); struct V9fsPDU { diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 3380bfc0c551..1a68c1622d3a 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -53,17 +53,15 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq) goto out_free_pdu; } - if (elem->in_num == 0) { + if (iov_size(elem->in_sg, elem->in_num) < 7) { virtio_error(vdev, "The guest sent a VirtFS request without space for " "the reply"); goto out_free_req; } - QEMU_BUILD_BUG_ON(sizeof(out) != 7); - len = iov_to_buf(elem->out_sg, elem->out_num, 0, - &out, sizeof(out)); - if (len != sizeof(out)) { + len = iov_to_buf(elem->out_sg, elem->out_num, 0, &out, 7); + if (len != 7) { virtio_error(vdev, "The guest sent a malformed VirtFS request: " "header size is %zd, should be 7", len); goto out_free_req;
The 9p spec at http://man.cat-v.org/plan_9/5/intro reads: "Each 9P message begins with a four-byte size field specify- ing the length in bytes of the complete message including the four bytes of the size field itself. The next byte is the message type, one of the constants in the enumeration in the include file <fcall.h>. The next two bytes are an iden- tifying tag, described below." ie, each message starts with a 7-byte long header. The core 9P code already assumes this pretty much everywhere. This patch does the following: - makes the assumption explicit in the common 9p.h header, since it isn't related to the transport - open codes the header size in handle_9p_output() and hardens the sanity check on the space needed for the reply message Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/9pfs/9p.h | 5 +++++ hw/9pfs/virtio-9p-device.c | 8 +++----- 2 files changed, 8 insertions(+), 5 deletions(-)