On 03/18/2012 04:27 AM, Dmitry Fleytman wrote:
Signed-off-by: Dmitry Fleytman<dmi...@daynix.com>
Signed-off-by: Yan Vugenfirer<y...@daynix.com>
---
hw/vmxnet_pkt.c | 1243 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/vmxnet_pkt.h | 479 +++++++++++++++++++++
2 files changed, 1722 insertions(+), 0 deletions(-)
create mode 100644 hw/vmxnet_pkt.c
create mode 100644 hw/vmxnet_pkt.h
diff --git a/hw/vmxnet_pkt.c b/hw/vmxnet_pkt.c
new file mode 100644
index 0000000..5fe2672
--- /dev/null
+++ b/hw/vmxnet_pkt.c
@@ -0,0 +1,1243 @@
+/*
+ * QEMU VMWARE VMXNET* paravirtual NICs - packets abstractions
+ *
+ * Copyright (c) 2012 Ravello Systems LTD (http://ravellosystems.com)
+ *
+ * Developed by Daynix Computing LTD (http://www.daynix.com)
+ *
+ * Authors:
+ * Dmitry Fleytman<dmi...@daynix.com>
+ * Tamir Shomer<tam...@daynix.com>
+ * Yan Vugenfirer<y...@daynix.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "vmxnet_pkt.h"
+#include "vmxnet_utils.h"
+#include "iov.h"
+
+#include "net/checksum.h"
+
+/*=============================================================================
+ *=============================================================================
+ *
+ * TX CODE
+ *
+ *=============================================================================
+ *===========================================================================*/
+
+enum {
+ VMXNET_TX_PKT_VHDR_FRAG = 0,
+ VMXNET_TX_PKT_L2HDR_FRAG,
+ VMXNET_TX_PKT_L3HDR_FRAG,
+ VMXNET_TX_PKT_PL_START_FRAG
+};
+
+/* TX packet private context */
+typedef struct _Vmxnet_TxPkt {
+ struct virtio_net_hdr virt_hdr;
+ bool has_virt_hdr;
+
+ struct iovec *vec;
+
+ uint8_t l2_hdr[ETH_MAX_L2_HDR_LEN];
+ uint8_t l3_hdr[ETH_MAX_L3_HDR_LEN];
+
+ uint32_t payload_len;
+
+ uint32_t payload_frags;
+ uint32_t max_payload_frags;
+
+ uint16_t hdr_len;
+ eth_pkt_types_e packet_type;
+ uint16_t l3_proto;
+} Vmxnet_TxPkt;
+
+/******************************************************************************
+ *
+ * Function: vmxnet_tx_pkt_init
+ *
+ * Desc: Init function for tx packet functionality.
+ *
+ * Params: (OUT) pkt - private handle.
+ * (IN) max_frags - max tx ip fragments.
+ * (IN) has_virt_hdr - device uses virtio header.
+ *
+ * Return: 0 on success, -1 on error
+ *
+ * Scope: Global
+ *
+ *****************************************************************************/
I applaud the use of comments but I don't think it's necessary to duplicate this
in the .c and .h file. We also are using GtkDoc as our comment format these days.
+int vmxnet_tx_pkt_init(Vmxnet_TxPkt_h *pkt, uint32_t max_frags,
+ bool has_virt_hdr)
+{
+ int rc = 0;
+
+ Vmxnet_TxPkt *p = g_malloc(sizeof *p);
+ if (!p) {
+ rc = -1;
+ goto Exit;
+ }
g_malloc cannot return NULL.
+
+ memset(p, 0, sizeof *p);
g_malloc0 will memset for you.
+
+ p->vec = g_malloc((sizeof *p->vec) *
+ (max_frags + VMXNET_TX_PKT_PL_START_FRAG));
+ if (!p->vec) {
+ rc = -1;
+ goto Exit;
+ }
+
+ p->max_payload_frags = max_frags;
+ p->has_virt_hdr = has_virt_hdr;
+ p->vec[VMXNET_TX_PKT_VHDR_FRAG].iov_base =&p->virt_hdr;
+ p->vec[VMXNET_TX_PKT_VHDR_FRAG].iov_len =
+ p->has_virt_hdr ? sizeof p->virt_hdr : 0;
+ p->vec[VMXNET_TX_PKT_L2HDR_FRAG].iov_base =&p->l2_hdr;
+ p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_base =&p->l3_hdr;
+
+ *pkt = p;
+
+Exit:
labels should be all lower case.
+ if (rc) {
+ vmxnet_tx_pkt_uninit(p);
+ }
+ return rc;
+}
+
+/******************************************************************************
+ *
+ * Function: vmxnet_tx_pkt_uninit
+ *
+ * Desc: Clean all tx packet resources.
+ *
+ * Params: (IN) pkt - private handle.
+ *
+ * Return: nothing
+ *
+ * Scope: Global
+ *
+ *****************************************************************************/
+void vmxnet_tx_pkt_uninit(Vmxnet_TxPkt_h pkt)
+{
+ Vmxnet_TxPkt *p = (Vmxnet_TxPkt *)pkt;
+
+ if (p) {
+ if (p->vec) {
+ g_free(p->vec);
+ }
+
+ g_free(p);
+ }
+}
+
+/******************************************************************************
+ *
+ * Function: vmxnet_tx_pkt_update_ip_checksums
+ *
+ * Desc: fix ip header fields and calculate checksums needed.
+ *
+ * Params: (IN) pkt - private handle.
+ *
+ * Return: Nothing.
+ *
+ * Scope: Global
+ *
+ *****************************************************************************/
+void vmxnet_tx_pkt_update_ip_checksums(Vmxnet_TxPkt_h pkt)
+{
+ uint16_t csum;
+ Vmxnet_TxPkt *p = (Vmxnet_TxPkt *)pkt;
+ assert(p);
+ uint8_t gso_type = p->virt_hdr.gso_type& ~VIRTIO_NET_HDR_GSO_ECN;
+ struct ip_header *ip_hdr;
+ target_phys_addr_t payload = (target_phys_addr_t)
+ (uint64_t) p->vec[VMXNET_TX_PKT_PL_START_FRAG].iov_base;
+
+ if (VIRTIO_NET_HDR_GSO_TCPV4 != gso_type&&
+ VIRTIO_NET_HDR_GSO_UDP != gso_type) {
+ return;
+ }
+
+ ip_hdr = p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_base;
+
+ if (p->payload_len + p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_len>
+ ETH_MAX_IP_DGRAM_LEN) {
+ return;
+ }
+
+ ip_hdr->ip_len = cpu_to_be16(p->payload_len +
+ p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_len);
+
+ /* Calculate IP header checksum */
+ ip_hdr->ip_sum = 0;
+ csum = net_raw_checksum((uint8_t *)ip_hdr,
+ p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_len);
+ ip_hdr->ip_sum = cpu_to_be16(csum);
+
+ /* Calculate IP pseudo header checksum */
+ csum = cpu_to_be16(eth_calc_pseudo_hdr_csum(ip_hdr, p->payload_len));
+ cpu_physical_memory_write(payload + p->virt_hdr.csum_offset,
+&csum, sizeof(csum));
+}
+
+/******************************************************************************
+ *
+ * Function: vmxnet_tx_pkt_get_l4_proto
+ *
+ * Desc: get l4 protocol.
+ *
+ * Params: (IN) p - module context
+ *
+ * Return: l4 protocol
+ *
+ * Scope: Local
+ *
+ *****************************************************************************/
+static uint8_t vmxnet_tx_pkt_get_l4_proto(Vmxnet_TxPkt *p)
+{
+ struct ip_header *ip_hdr;
+ struct ip6_header *ip6_hdr;
+
+ if (!p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_len) {
+ return 0;
+ }
+
+ if (ETH_P_IP == p->l3_proto) {
+ ip_hdr = p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_base;
+ return ip_hdr->ip_p;
+ } else if (p->l3_proto == ETH_P_IPV6) {
+ ip6_hdr = p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_base;
+ return ip6_hdr->ip6_nxt;
+ }
+
+ return 0;
+}
+
+/******************************************************************************
+ *
+ * Function: vmxnet_tx_pkt_calculate_hdr_len
+ *
+ * Desc: store l2 and l3 headers length.
+ *
+ * Params: (IN) p - module context
+ *
+ * Return: nothing
+ *
+ * Scope: Local
+ *
+ *****************************************************************************/
+static void vmxnet_tx_pkt_calculate_hdr_len(Vmxnet_TxPkt *p)
+{
+ p->hdr_len = p->vec[VMXNET_TX_PKT_L2HDR_FRAG].iov_len +
+ p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_len;
+}
+
+/******************************************************************************
+ *
+ * Function: vmxnet_tx_pkt_prepare
+ *
+ * Desc: populates headers and parses them to gether some metadata for later
+ * use.
+ *
+ * Params: (IN) pkt - private handle.
+ * (IN) pa - physical address of tx data
+ * (IN) len - length of data
+ *
+ * Return: number of bytes populated by the function.
+ *
+ * Scope: Global
+ *
+ *****************************************************************************/
+size_t vmxnet_tx_pkt_prepare(Vmxnet_TxPkt_h pkt, target_phys_addr_t pa,
+ size_t len)
+{
+ Vmxnet_TxPkt *p = (Vmxnet_TxPkt *)pkt;
+ /* some pointers that my lines stay nice and short. */
+ void *l2_iov_base = NULL, *l3_iov_base = NULL;
+ size_t *l2_iov_len = NULL, *l3_iov_len = NULL;
+ assert(p);
+
+ l2_iov_base = p->vec[VMXNET_TX_PKT_L2HDR_FRAG].iov_base;
+ l2_iov_len =&p->vec[VMXNET_TX_PKT_L2HDR_FRAG].iov_len;
+ l3_iov_base = p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_base;
+ l3_iov_len =&p->vec[VMXNET_TX_PKT_L3HDR_FRAG].iov_len;
+
+ cpu_physical_memory_read(pa, l2_iov_base, ETH_MAX_L2_HDR_LEN);
As best as I can tell from looking through the patches, this is a buffer
overflow.
I don't think you validate that VMXNET_TX_PKT_L2HDR_FRAG's length is >=
ETH_MAX_L2_HDR_LEN but you still read it unconditionally.
+ *l2_iov_len = eth_get_l2_hdr_length(l2_iov_base);
+
+ p->l3_proto = eth_get_l3_proto(l2_iov_base, *l2_iov_len);
+
+ switch (p->l3_proto) {
+ case ETH_P_IP:
+ assert(len>= *l2_iov_len + sizeof(struct ip_header));
+
+ cpu_physical_memory_read(pa + *l2_iov_len, l3_iov_base,
+ sizeof(struct ip_header));
While you check len here, I think it's still possible for l3_iov_len to be
smaller than sizeof(struct ip_header).
Regards,
Anthony Liguori
