User space is currently sending a OVS_FLOW_ATTR_PROBE for both flow
and packet messages. This leads to an out-of-bounds access in
ovs_packet_cmd_execute() because OVS_FLOW_ATTR_PROBE >
OVS_PACKET_ATTR_MAX.

Introduce a new OVS_PACKET_ATTR_PROBE with the same numeric value
as OVS_FLOW_ATTR_PROBE to grow the range of accepted packet attributes
while maintaining binary compatibility with existing OVS binaries.

Fixes: 9233ce ("datapath: Add support for OVS_FLOW_ATTR_PROBE.")
Reported-by: Sander Eikelenboom <li...@eikelenboom.it>
Signed-off-by: Thomas Graf <tg...@noironetworks.com>
---
 AUTHORS                                           | 1 +
 datapath/datapath.c                               | 3 ++-
 datapath/linux/compat/include/linux/openvswitch.h | 4 ++++
 lib/dpif-netlink.c                                | 2 +-
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 3356ee8..ab82e24 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -301,6 +301,7 @@ Roger Leigh             rle...@codelibre.net
 Rogério Vinhal Nunes
 Roman Sokolkov          rsokol...@gmail.com
 Ronaldo A. Ferreira     ronal...@cs.princeton.edu
+Sander Eikelenboom      li...@eikelenboom.it
 Saul St. John           sstj...@cs.wisc.edu
 Scott Hendricks         shendri...@nicira.com
 Sean Brady              sbr...@gtfservices.com
diff --git a/datapath/datapath.c b/datapath/datapath.c
index de912f6..c562e89 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -531,7 +531,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, 
struct genl_info *info)
        struct vport *input_vport;
        int len;
        int err;
-       bool log = !a[OVS_FLOW_ATTR_PROBE];
+       bool log = !a[OVS_PACKET_ATTR_PROBE];
 
        err = -EINVAL;
        if (!a[OVS_PACKET_ATTR_PACKET] || !a[OVS_PACKET_ATTR_KEY] ||
@@ -618,6 +618,7 @@ static const struct nla_policy 
packet_policy[OVS_PACKET_ATTR_MAX + 1] = {
        [OVS_PACKET_ATTR_PACKET] = { .len = ETH_HLEN },
        [OVS_PACKET_ATTR_KEY] = { .type = NLA_NESTED },
        [OVS_PACKET_ATTR_ACTIONS] = { .type = NLA_NESTED },
+       [OVS_PACKET_ATTR_PROBE] = { .type = NLA_FLAG },
 };
 
 static struct genl_ops dp_packet_genl_ops[] = {
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 67715f8..a59e109 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -197,6 +197,10 @@ enum ovs_packet_attr {
        OVS_PACKET_ATTR_USERDATA,    /* OVS_ACTION_ATTR_USERSPACE arg. */
        OVS_PACKET_ATTR_EGRESS_TUN_KEY,  /* Nested OVS_TUNNEL_KEY_ATTR_*
                                            attributes. */
+       OVS_PACKET_ATTR_UNUSED1,
+       OVS_PACKET_ATTR_UNUSED2,
+       OVS_PACKET_ATTR_PROBE,      /* Packet operation is a feature probe,
+                                      error logging should be suppressed. */
        __OVS_PACKET_ATTR_MAX
 };
 
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 8f0eca6..a9d60f7 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1530,7 +1530,7 @@ dpif_netlink_encode_execute(int dp_ifindex, const struct 
dpif_execute *d_exec,
     nl_msg_put_unspec(buf, OVS_PACKET_ATTR_ACTIONS,
                       d_exec->actions, d_exec->actions_len);
     if (d_exec->probe) {
-        nl_msg_put_flag(buf, OVS_FLOW_ATTR_PROBE);
+        nl_msg_put_flag(buf, OVS_PACKET_ATTR_PROBE);
     }
 }
 
-- 
1.9.3

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to