From: Stefan Hajnoczi <stefanha@gmail.com>
To: akong@redhat.com
Cc: virtualization@lists.linux-foundation.org, stefanha@redhat.com,
qemu-devel@nongnu.org, kvm@vger.kernel.org, mst@redhat.com
Subject: Re: [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq
Date: Mon, 21 Jan 2013 17:03:30 +0100 [thread overview]
Message-ID: <20130121160330.GC24473@stefanha-thinkpad.redhat.com> (raw)
In-Reply-To: <1358560468-10865-2-git-send-email-akong@redhat.com>
On Sat, Jan 19, 2013 at 09:54:26AM +0800, akong@redhat.com wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
>
> Virtio-net code makes assumption about virtqueue descriptor layout
> (e.g. sg[0] is the header, sg[1] is the data buffer).
>
> This patch makes code not rely on the layout of descriptors.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 74 insertions(+), 54 deletions(-)
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 3bb01b1..113e194 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -315,44 +315,44 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
> }
>
> static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
> - VirtQueueElement *elem)
> + struct iovec *iov, unsigned int iov_cnt)
> {
> uint8_t on;
> + size_t s;
>
> - if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(on)) {
> - error_report("virtio-net ctrl invalid rx mode command");
> - exit(1);
> + s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on));
> + if (s != sizeof(on)) {
> + return VIRTIO_NET_ERR;
> }
>
> - on = ldub_p(elem->out_sg[1].iov_base);
> -
> - if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC)
> + if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) {
> n->promisc = on;
> - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI)
> + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) {
> n->allmulti = on;
> - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI)
> + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) {
> n->alluni = on;
> - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI)
> + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) {
> n->nomulti = on;
> - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI)
> + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) {
> n->nouni = on;
> - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST)
> + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) {
> n->nobcast = on;
> - else
> + } else {
> return VIRTIO_NET_ERR;
> + }
>
> return VIRTIO_NET_OK;
> }
>
> static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> - VirtQueueElement *elem)
> + struct iovec *iov, unsigned int iov_cnt)
> {
> struct virtio_net_ctrl_mac mac_data;
> + size_t s;
>
> - if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 ||
> - elem->out_sg[1].iov_len < sizeof(mac_data) ||
> - elem->out_sg[2].iov_len < sizeof(mac_data))
> + if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) {
> return VIRTIO_NET_ERR;
> + }
>
> n->mac_table.in_use = 0;
> n->mac_table.first_multi = 0;
> @@ -360,54 +360,71 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> n->mac_table.multi_overflow = 0;
> memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
>
> - mac_data.entries = ldl_p(elem->out_sg[1].iov_base);
> + s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
> + sizeof(mac_data.entries));
>
> - if (sizeof(mac_data.entries) +
> - (mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len)
> + if (s != sizeof(mac_data.entries)) {
> return VIRTIO_NET_ERR;
> + }
> + iov_discard_front(&iov, &iov_cnt, s);
> +
> + if (mac_data.entries * ETH_ALEN > iov_size(iov, iov_cnt)) {
The (possible) byteswap was lost. ldl_p() copies from target endianness
to host endianness.
> + return VIRTIO_NET_ERR;
> + }
>
> if (mac_data.entries <= MAC_TABLE_ENTRIES) {
> - memcpy(n->mac_table.macs, elem->out_sg[1].iov_base + sizeof(mac_data),
> - mac_data.entries * ETH_ALEN);
> + s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
> + mac_data.entries * ETH_ALEN);
> + if (s != mac_data.entries * ETH_ALEN) {
> + return VIRTIO_NET_OK;
s/VIRTIO_NET_OK/VIRTIO_NET_ERR/
> + }
> n->mac_table.in_use += mac_data.entries;
> } else {
> n->mac_table.uni_overflow = 1;
> }
>
> + iov_discard_front(&iov, &iov_cnt, mac_data.entries * ETH_ALEN);
> +
> n->mac_table.first_multi = n->mac_table.in_use;
>
> - mac_data.entries = ldl_p(elem->out_sg[2].iov_base);
> + s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
> + sizeof(mac_data.entries));
Same deal with mac_data.entries byteswap.
>
> - if (sizeof(mac_data.entries) +
> - (mac_data.entries * ETH_ALEN) > elem->out_sg[2].iov_len)
> + if (s != sizeof(mac_data.entries)) {
> return VIRTIO_NET_ERR;
> + }
>
> - if (mac_data.entries) {
> - if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
> - memcpy(n->mac_table.macs + (n->mac_table.in_use * ETH_ALEN),
> - elem->out_sg[2].iov_base + sizeof(mac_data),
> - mac_data.entries * ETH_ALEN);
> - n->mac_table.in_use += mac_data.entries;
> - } else {
> - n->mac_table.multi_overflow = 1;
> + iov_discard_front(&iov, &iov_cnt, s);
> +
> + if (mac_data.entries * ETH_ALEN != iov_size(iov, iov_cnt)) {
> + return VIRTIO_NET_ERR;
> + }
> +
> + if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
> + s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
> + mac_data.entries * ETH_ALEN);
> + if (s != mac_data.entries * ETH_ALEN) {
> + return VIRTIO_NET_OK;
VIRTIO_NET_ERR
> }
> + n->mac_table.in_use += mac_data.entries;
> + } else {
> + n->mac_table.multi_overflow = 1;
> }
>
> return VIRTIO_NET_OK;
> }
>
> static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
> - VirtQueueElement *elem)
> + struct iovec *iov, unsigned int iov_cnt)
> {
> uint16_t vid;
> + size_t s;
>
> - if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(vid)) {
> - error_report("virtio-net ctrl invalid vlan command");
> + s = iov_to_buf(iov, iov_cnt, 0, &vid, sizeof(vid));
> + if (s != sizeof(vid)) {
> return VIRTIO_NET_ERR;
> }
>
> - vid = lduw_p(elem->out_sg[1].iov_base);
> -
Byteswap missing.
next prev parent reply other threads:[~2013-01-21 16:03 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-19 1:54 [QEMU PATCH v4 0/3] virtio-net: fix of ctrl commands akong
2013-01-19 1:54 ` [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq akong
2013-01-19 2:08 ` [Qemu-devel] " Amos Kong
2013-01-21 16:03 ` Stefan Hajnoczi [this message]
2013-01-22 14:38 ` Amos Kong
[not found] ` <20130122143814.GB4066@t430s.nay.redhat.com>
2013-01-22 14:49 ` Stefan Hajnoczi
2013-01-19 1:54 ` [QEMU PATCH v4 2/3] virtio-net: introduce a new macaddr control akong
2013-01-19 1:54 ` [QEMU PATCH v4 3/3] virtio-net: rename ctrl rx commands akong
[not found] ` <1358560468-10865-3-git-send-email-akong@redhat.com>
2013-01-21 16:08 ` [QEMU PATCH v4 2/3] virtio-net: introduce a new macaddr control Stefan Hajnoczi
2013-01-22 11:37 ` Amos Kong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130121160330.GC24473@stefanha-thinkpad.redhat.com \
--to=stefanha@gmail.com \
--cc=akong@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).