Message ID | 44f4cc7dd6ceb5aac6045e59ff49dce70dd53e74.1525363636.git.sbrivio@redhat.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,net] openvswitch: Don't swap table in nlattr_set() after OVS_ATTR_NESTED is found | expand |
From: Stefano Brivio <sbrivio@redhat.com> Date: Thu, 3 May 2018 18:13:25 +0200 > If an OVS_ATTR_NESTED attribute type is found while walking > through netlink attributes, we call nlattr_set() recursively > passing the length table for the following nested attributes, if > different from the current one. > > However, once we're done with those sub-nested attributes, we > should continue walking through attributes using the current > table, instead of using the one related to the sub-nested > attributes. > > For example, given this sequence: > > 1 OVS_KEY_ATTR_PRIORITY > 2 OVS_KEY_ATTR_TUNNEL > 3 OVS_TUNNEL_KEY_ATTR_ID > 4 OVS_TUNNEL_KEY_ATTR_IPV4_SRC > 5 OVS_TUNNEL_KEY_ATTR_IPV4_DST > 6 OVS_TUNNEL_KEY_ATTR_TTL > 7 OVS_TUNNEL_KEY_ATTR_TP_SRC > 8 OVS_TUNNEL_KEY_ATTR_TP_DST > 9 OVS_KEY_ATTR_IN_PORT > 10 OVS_KEY_ATTR_SKB_MARK > 11 OVS_KEY_ATTR_MPLS > > we switch to the 'ovs_tunnel_key_lens' table on attribute #3, > and we don't switch back to 'ovs_key_lens' while setting > attributes #9 to #11 in the sequence. As OVS_KEY_ATTR_MPLS > evaluates to 21, and the array size of 'ovs_tunnel_key_lens' is > 15, we also get this kind of KASan splat while accessing the > wrong table: ... > Reported-by: Hangbin Liu <liuhangbin@gmail.com> > Fixes: 982b52700482 ("openvswitch: Fix mask generation for nested attributes.") > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > Reviewed-by: Sabrina Dubroca <sd@queasysnail.net> Looks good, applied and queued up for -stable. Thanks.
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c index 7322aa1e382e..492ab0c36f7c 100644 --- a/net/openvswitch/flow_netlink.c +++ b/net/openvswitch/flow_netlink.c @@ -1712,13 +1712,10 @@ static void nlattr_set(struct nlattr *attr, u8 val, /* The nlattr stream should already have been validated */ nla_for_each_nested(nla, attr, rem) { - if (tbl[nla_type(nla)].len == OVS_ATTR_NESTED) { - if (tbl[nla_type(nla)].next) - tbl = tbl[nla_type(nla)].next; - nlattr_set(nla, val, tbl); - } else { + if (tbl[nla_type(nla)].len == OVS_ATTR_NESTED) + nlattr_set(nla, val, tbl[nla_type(nla)].next ? : tbl); + else memset(nla_data(nla), val, nla_len(nla)); - } if (nla_type(nla) == OVS_KEY_ATTR_CT_STATE) *(u32 *)nla_data(nla) &= CT_SUPPORTED_MASK;