On Fri, Aug 17, 2018 at 1:15 AM Jiecheng Wu <[email protected]> wrote:
>
> Function queue_userspace_packet() defined in net/openvswitch/datapath.c calls
> nla_nest_start() to allocate memory for struct nlattr which is dereferenced
> immediately. As nla_nest_start() may return NULL on failure, this code piece
> may cause NULL pointer dereference bug.
> ---
> net/openvswitch/datapath.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 0f5ce77..ff4457d 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -460,6 +460,8 @@ static int queue_userspace_packet(struct datapath *dp,
> struct sk_buff *skb,
>
> if (upcall_info->egress_tun_info) {
> nla = nla_nest_start(user_skb,
> OVS_PACKET_ATTR_EGRESS_TUN_KEY);
> + if (!nla)
> + return -EMSGSIZE;
It is not possible, since user_skb is allocated to accommodate all
netlink attributes.
> err = ovs_nla_put_tunnel_info(user_skb,
> upcall_info->egress_tun_info);
> BUG_ON(err);
> @@ -468,6 +470,8 @@ static int queue_userspace_packet(struct datapath *dp,
> struct sk_buff *skb,
>
> if (upcall_info->actions_len) {
> nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_ACTIONS);
> + if (!nla)
> + return -EMSGSIZE;
same as above, the check is not required.
> err = ovs_nla_put_actions(upcall_info->actions,
> upcall_info->actions_len,
> user_skb);
> --
> 2.6.4
>
