From c3cc8c031bb7be42b5b818c23d0e708a0cc11a26 Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Mon, 2 Apr 2012 13:25:21 -0700 Subject: [PATCH] datapath: Stop using NLA_PUT*(). These macros contain a hidden goto, and are thus extremely error prone and make code hard to audit. Signed-off-by: David S. Miller [jesse: Additional transformations for code not upstream.] Signed-off-by: Jesse Gross --- datapath/brcompat_main.c | 18 ++++++++----- datapath/datapath.c | 57 ++++++++++++++++++++++------------------ datapath/flow.c | 23 +++++++++------- datapath/tunnel.c | 31 +++++++++++++--------- 4 files changed, 75 insertions(+), 54 deletions(-) diff --git a/datapath/brcompat_main.c b/datapath/brcompat_main.c index 5d0f0bba..e7f741b6 100644 --- a/datapath/brcompat_main.c +++ b/datapath/brcompat_main.c @@ -63,10 +63,12 @@ static struct sk_buff *brc_make_request(int op, const char *bridge, goto error; genlmsg_put(skb, 0, 0, &brc_genl_family, 0, op); - if (bridge) - NLA_PUT_STRING(skb, BRC_GENL_A_DP_NAME, bridge); - if (port) - NLA_PUT_STRING(skb, BRC_GENL_A_PORT_NAME, port); + + if (bridge && nla_put_string(skb, BRC_GENL_A_DP_NAME, bridge)) + goto nla_put_failure; + if (port && nla_put_string(skb, BRC_GENL_A_PORT_NAME, port)) + goto nla_put_failure; + return skb; nla_put_failure: @@ -288,8 +290,9 @@ static int brc_get_fdb_entries(struct net_device *dev, void __user *userbuf, request = brc_make_request(BRC_GENL_C_FDB_QUERY, dev->name, NULL); if (!request) return -ENOMEM; - NLA_PUT_U64(request, BRC_GENL_A_FDB_COUNT, maxnum); - NLA_PUT_U64(request, BRC_GENL_A_FDB_SKIP, offset); + if (nla_put_u64(request, BRC_GENL_A_FDB_COUNT, maxnum) || + nla_put_u64(request, BRC_GENL_A_FDB_SKIP, offset)) + goto nla_put_failure; rtnl_unlock(); reply = brc_send_command(dev_net(dev), request, attrs); @@ -401,7 +404,8 @@ static int brc_genl_query(struct sk_buff *skb, struct genl_info *info) err = -ENOMEM; goto err; } - NLA_PUT_U32(ans_skb, BRC_GENL_A_MC_GROUP, brc_mc_group.id); + if (nla_put_u32(ans_skb, BRC_GENL_A_MC_GROUP, brc_mc_group.id)) + goto nla_put_failure; genlmsg_end(ans_skb, data); return genlmsg_reply(ans_skb, info); diff --git a/datapath/datapath.c b/datapath/datapath.c index 4dde50b2..7f313940 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -176,17 +176,17 @@ static int dp_fill_ifinfo(struct sk_buff *skb, hdr->ifi_flags = port->ops->get_dev_flags(port); hdr->ifi_change = 0; - NLA_PUT_STRING(skb, IFLA_IFNAME, port->ops->get_name(port)); - NLA_PUT_U32(skb, IFLA_MASTER, get_dpifindex(dp)); - NLA_PUT_U32(skb, IFLA_MTU, port->ops->get_mtu(port)); + if (nla_put_string(skb, IFLA_IFNAME, port->ops->get_name(port)) || + nla_put_u32(skb, IFLA_MASTER, get_dpifindex(dp)) || + nla_put_u32(skb, IFLA_MTU, port->ops->get_mtu(port)) || #ifdef IFLA_OPERSTATE - NLA_PUT_U8(skb, IFLA_OPERSTATE, - port->ops->is_running(port) - ? port->ops->get_operstate(port) - : IF_OPER_DOWN); + nla_put_u8(skb, IFLA_OPERSTATE, + port->ops->is_running(port) ? + port->ops->get_operstate(port) : + IF_OPER_DOWN) || #endif - - NLA_PUT(skb, IFLA_ADDRESS, ETH_ALEN, port->ops->get_addr(port)); + nla_put(skb, IFLA_ADDRESS, ETH_ALEN, port->ops->get_addr(port))) + goto nla_put_failure; return nlmsg_end(skb, nlh); @@ -918,15 +918,18 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp, tcp_flags = flow->tcp_flags; spin_unlock_bh(&flow->lock); - if (used) - NLA_PUT_U64(skb, OVS_FLOW_ATTR_USED, ovs_flow_used_time(used)); + if (used && + nla_put_u64(skb, OVS_FLOW_ATTR_USED, ovs_flow_used_time(used))) + goto nla_put_failure; - if (stats.n_packets) - NLA_PUT(skb, OVS_FLOW_ATTR_STATS, - sizeof(struct ovs_flow_stats), &stats); + if (stats.n_packets && + nla_put(skb, OVS_FLOW_ATTR_STATS, + sizeof(struct ovs_flow_stats), &stats)) + goto nla_put_failure; - if (tcp_flags) - NLA_PUT_U8(skb, OVS_FLOW_ATTR_TCP_FLAGS, tcp_flags); + if (tcp_flags && + nla_put_u8(skb, OVS_FLOW_ATTR_TCP_FLAGS, tcp_flags)) + goto nla_put_failure; /* If OVS_FLOW_ATTR_ACTIONS doesn't fit, skip dumping the actions if * this is the first flow to be dumped into 'skb'. This is unusual for @@ -1312,7 +1315,8 @@ static int ovs_dp_cmd_fill_info(struct datapath *dp, struct sk_buff *skb, goto nla_put_failure; get_dp_stats(dp, &dp_stats); - NLA_PUT(skb, OVS_DP_ATTR_STATS, sizeof(struct ovs_dp_stats), &dp_stats); + if (nla_put(skb, OVS_DP_ATTR_STATS, sizeof(struct ovs_dp_stats), &dp_stats)) + goto nla_put_failure; return genlmsg_end(skb, ovs_header); @@ -1668,17 +1672,20 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb, ovs_header->dp_ifindex = get_dpifindex(vport->dp); - NLA_PUT_U32(skb, OVS_VPORT_ATTR_PORT_NO, vport->port_no); - NLA_PUT_U32(skb, OVS_VPORT_ATTR_TYPE, vport->ops->type); - NLA_PUT_STRING(skb, OVS_VPORT_ATTR_NAME, vport->ops->get_name(vport)); - NLA_PUT_U32(skb, OVS_VPORT_ATTR_UPCALL_PID, vport->upcall_pid); + if (nla_put_u32(skb, OVS_VPORT_ATTR_PORT_NO, vport->port_no) || + nla_put_u32(skb, OVS_VPORT_ATTR_TYPE, vport->ops->type) || + nla_put_string(skb, OVS_VPORT_ATTR_NAME, vport->ops->get_name(vport)) || + nla_put_u32(skb, OVS_VPORT_ATTR_UPCALL_PID, vport->upcall_pid)) + goto nla_put_failure; ovs_vport_get_stats(vport, &vport_stats); - NLA_PUT(skb, OVS_VPORT_ATTR_STATS, sizeof(struct ovs_vport_stats), - &vport_stats); + if (nla_put(skb, OVS_VPORT_ATTR_STATS, sizeof(struct ovs_vport_stats), + &vport_stats)) + goto nla_put_failure; - NLA_PUT(skb, OVS_VPORT_ATTR_ADDRESS, ETH_ALEN, - vport->ops->get_addr(vport)); + if (nla_put(skb, OVS_VPORT_ATTR_ADDRESS, ETH_ALEN, + vport->ops->get_addr(vport))) + goto nla_put_failure; err = ovs_vport_get_options(vport, skb); if (err == -EMSGSIZE) diff --git a/datapath/flow.c b/datapath/flow.c index 86cbd784..9f935508 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -1206,14 +1206,17 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key *swkey, struct sk_buff *skb) struct ovs_key_ethernet *eth_key; struct nlattr *nla, *encap; - if (swkey->phy.priority) - NLA_PUT_U32(skb, OVS_KEY_ATTR_PRIORITY, swkey->phy.priority); + if (swkey->phy.priority && + nla_put_u32(skb, OVS_KEY_ATTR_PRIORITY, swkey->phy.priority)) + goto nla_put_failure; - if (swkey->phy.tun_id != cpu_to_be64(0)) - NLA_PUT_BE64(skb, OVS_KEY_ATTR_TUN_ID, swkey->phy.tun_id); + if (swkey->phy.tun_id != cpu_to_be64(0) && + nla_put_be64(skb, OVS_KEY_ATTR_TUN_ID, swkey->phy.tun_id)) + goto nla_put_failure; - if (swkey->phy.in_port != DP_MAX_PORTS) - NLA_PUT_U32(skb, OVS_KEY_ATTR_IN_PORT, swkey->phy.in_port); + if (swkey->phy.in_port != DP_MAX_PORTS && + nla_put_u32(skb, OVS_KEY_ATTR_IN_PORT, swkey->phy.in_port)) + goto nla_put_failure; nla = nla_reserve(skb, OVS_KEY_ATTR_ETHERNET, sizeof(*eth_key)); if (!nla) @@ -1223,8 +1226,9 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key *swkey, struct sk_buff *skb) memcpy(eth_key->eth_dst, swkey->eth.dst, ETH_ALEN); if (swkey->eth.tci || swkey->eth.type == htons(ETH_P_8021Q)) { - NLA_PUT_BE16(skb, OVS_KEY_ATTR_ETHERTYPE, htons(ETH_P_8021Q)); - NLA_PUT_BE16(skb, OVS_KEY_ATTR_VLAN, swkey->eth.tci); + if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, htons(ETH_P_8021Q)) || + nla_put_be16(skb, OVS_KEY_ATTR_VLAN, swkey->eth.tci)) + goto nla_put_failure; encap = nla_nest_start(skb, OVS_KEY_ATTR_ENCAP); if (!swkey->eth.tci) goto unencap; @@ -1235,7 +1239,8 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key *swkey, struct sk_buff *skb) if (swkey->eth.type == htons(ETH_P_802_2)) goto unencap; - NLA_PUT_BE16(skb, OVS_KEY_ATTR_ETHERTYPE, swkey->eth.type); + if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, swkey->eth.type)) + goto nla_put_failure; if (swkey->eth.type == htons(ETH_P_IP)) { struct ovs_key_ipv4 *ipv4_key; diff --git a/datapath/tunnel.c b/datapath/tunnel.c index ea97e394..d406dbc4 100644 --- a/datapath/tunnel.c +++ b/datapath/tunnel.c @@ -1553,19 +1553,24 @@ int ovs_tnl_get_options(const struct vport *vport, struct sk_buff *skb) const struct tnl_vport *tnl_vport = tnl_vport_priv(vport); const struct tnl_mutable_config *mutable = rcu_dereference_rtnl(tnl_vport->mutable); - NLA_PUT_U32(skb, OVS_TUNNEL_ATTR_FLAGS, mutable->flags & TNL_F_PUBLIC); - NLA_PUT_BE32(skb, OVS_TUNNEL_ATTR_DST_IPV4, mutable->key.daddr); - - if (!(mutable->flags & TNL_F_IN_KEY_MATCH)) - NLA_PUT_BE64(skb, OVS_TUNNEL_ATTR_IN_KEY, mutable->key.in_key); - if (!(mutable->flags & TNL_F_OUT_KEY_ACTION)) - NLA_PUT_BE64(skb, OVS_TUNNEL_ATTR_OUT_KEY, mutable->out_key); - if (mutable->key.saddr) - NLA_PUT_BE32(skb, OVS_TUNNEL_ATTR_SRC_IPV4, mutable->key.saddr); - if (mutable->tos) - NLA_PUT_U8(skb, OVS_TUNNEL_ATTR_TOS, mutable->tos); - if (mutable->ttl) - NLA_PUT_U8(skb, OVS_TUNNEL_ATTR_TTL, mutable->ttl); + if (nla_put_u32(skb, OVS_TUNNEL_ATTR_FLAGS, + mutable->flags & TNL_F_PUBLIC) || + nla_put_be32(skb, OVS_TUNNEL_ATTR_DST_IPV4, mutable->key.daddr)) + goto nla_put_failure; + + if (!(mutable->flags & TNL_F_IN_KEY_MATCH) && + nla_put_be64(skb, OVS_TUNNEL_ATTR_IN_KEY, mutable->key.in_key)) + goto nla_put_failure; + if (!(mutable->flags & TNL_F_OUT_KEY_ACTION) && + nla_put_be64(skb, OVS_TUNNEL_ATTR_OUT_KEY, mutable->out_key)) + goto nla_put_failure; + if (mutable->key.saddr && + nla_put_be32(skb, OVS_TUNNEL_ATTR_SRC_IPV4, mutable->key.saddr)) + goto nla_put_failure; + if (mutable->tos && nla_put_u8(skb, OVS_TUNNEL_ATTR_TOS, mutable->tos)) + goto nla_put_failure; + if (mutable->ttl && nla_put_u8(skb, OVS_TUNNEL_ATTR_TTL, mutable->ttl)) + goto nla_put_failure; return 0; -- 2.30.2