[ovs-dev] [bug 7759 05/11] dpif-linux: Use get_32aligned_u64() in an appropriate place.

[Ben Pfaff]

On Tue, Oct 11, 2011 at 11:08:26AM -0700, Jesse Gross wrote:
> On Wed, Oct 5, 2011 at 11:27 AM, Ben Pfaff <blp at nicira.com> wrote:
> > ---
> > ??lib/dpif-linux.c | ?? 10 +++++-----
> > ??1 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
> > index 4ddd464..79e84d6 100644
> > --- a/lib/dpif-linux.c
> > +++ b/lib/dpif-linux.c
> > @@ -102,9 +102,9 @@ struct dpif_linux_flow {
> >
> > ?? ?? /* Attributes.
> > ?? ?? ??*
> > - ?? ?? * The 'stats' and 'used' members point to 64-bit data that might only be
> > - ?? ?? * aligned on 32-bit boundaries, so get_unaligned_u64() should be used to
> > - ?? ?? * access their values.
> > + ?? ?? * The 'stats' member points to 64-bit data that might only be aligned on
> > + ?? ?? * 32-bit boundaries, so get_unaligned_u64() should be used to access its
> > + ?? ?? * values.
> 
> This looks good.  However, when reviewing this I noticed that we seem
> to access vport stats in an unsafe manner since they are also 64-bit
> stats aligned on 32-bit boundaries.

Oops.  Here's a fix.

--8<--------------------------cut here-------------------------->8--

From: Ben Pfaff <blp at nicira.com>
Date: Tue, 11 Oct 2011 12:24:41 -0700
Subject: [PATCH] dpif-linux: Avoid unaligned accesses to vport stats sent by
 the datapath.

Reported-by: Jesse Gross <jesse at nicira.com>
---
 lib/dpif-linux.h   |    6 +++++-
 lib/netdev-vport.c |   33 ++++++++++++++++++++-------------
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/lib/dpif-linux.h b/lib/dpif-linux.h
index c72ea88..a9f8bfe 100644
--- a/lib/dpif-linux.h
+++ b/lib/dpif-linux.h
@@ -32,7 +32,11 @@ struct dpif_linux_vport {
     uint32_t port_no;                      /* UINT32_MAX if unknown. */
     enum ovs_vport_type type;
 
-    /* Attributes. */
+    /* Attributes.
+     *
+     * The 'stats' member points to 64-bit data that might only be aligned on
+     * 32-bit boundaries, so use get_unaligned_u64() to access its values.
+     */
     const char *name;                      /* OVS_VPORT_ATTR_NAME. */
     const uint32_t *upcall_pid;            /* OVS_VPORT_ATTR_UPCALL_PID. */
     const struct ovs_vport_stats *stats;   /* OVS_VPORT_ATTR_STATS. */
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 06ec8fb..70cdf29 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -44,6 +44,7 @@
 #include "route-table.h"
 #include "shash.h"
 #include "socket-util.h"
+#include "unaligned.h"
 #include "vlog.h"
 
 VLOG_DEFINE_THIS_MODULE(netdev_vport);
@@ -361,22 +362,21 @@ netdev_vport_get_etheraddr(const struct netdev *netdev,
     return error;
 }
 
-#define COPY_OVS_STATS                                      \
-    dst->rx_packets = src->rx_packets;                      \
-    dst->tx_packets = src->tx_packets;                      \
-    dst->rx_bytes = src->rx_bytes;                          \
-    dst->tx_bytes = src->tx_bytes;                          \
-    dst->rx_errors = src->rx_errors;                        \
-    dst->tx_errors = src->tx_errors;                        \
-    dst->rx_dropped = src->rx_dropped;                      \
-    dst->tx_dropped = src->tx_dropped;
-
-/* Copies 'src' into 'dst', performing format conversion in the process. */
+/* Copies 'src' into 'dst', performing format conversion in the process.
+ *
+ * 'src' is allowed to be misaligned. */
 static void
 netdev_stats_from_ovs_vport_stats(struct netdev_stats *dst,
                                   const struct ovs_vport_stats *src)
 {
-    COPY_OVS_STATS
+    dst->rx_packets = get_unaligned_u64(&src->rx_packets);
+    dst->tx_packets = get_unaligned_u64(&src->tx_packets);
+    dst->rx_bytes = get_unaligned_u64(&src->rx_bytes);
+    dst->tx_bytes = get_unaligned_u64(&src->tx_bytes);
+    dst->rx_errors = get_unaligned_u64(&src->rx_errors);
+    dst->tx_errors = get_unaligned_u64(&src->tx_errors);
+    dst->rx_dropped = get_unaligned_u64(&src->rx_dropped);
+    dst->tx_dropped = get_unaligned_u64(&src->tx_dropped);
     dst->multicast = 0;
     dst->collisions = 0;
     dst->rx_length_errors = 0;
@@ -397,7 +397,14 @@ static void
 netdev_stats_to_ovs_vport_stats(struct ovs_vport_stats *dst,
                                 const struct netdev_stats *src)
 {
-    COPY_OVS_STATS
+    dst->rx_packets = src->rx_packets;
+    dst->tx_packets = src->tx_packets;
+    dst->rx_bytes = src->rx_bytes;
+    dst->tx_bytes = src->tx_bytes;
+    dst->rx_errors = src->rx_errors;
+    dst->tx_errors = src->tx_errors;
+    dst->rx_dropped = src->rx_dropped;
+    dst->tx_dropped = src->tx_dropped;
 }
 
 int
-- 
1.7.4.4