On Tue, Aug 21, 2018 at 4:38 PM David Miller <da...@davemloft.net> wrote:
>
> From: Pravin Shelar <pshe...@ovn.org>
> Date: Tue, 21 Aug 2018 15:38:28 -0700
>
> > On Fri, Aug 17, 2018 at 1:15 AM Jiecheng Wu <jasonwood2...@gmail.com> 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.
>
> Pravin, common practice is to always check nla_*() return values even if the
> SKB is allocated with "enough space".
>
> Those calculations can have bugs, and these checks are therefore helpful to
> avoid crashes and memory corruption in such cases.
>
OK, in that case this patch needs to proper error handling.
