Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1163314AbdDWUXJ (ORCPT ); Sun, 23 Apr 2017 16:23:09 -0400 Received: from slow1-d.mail.gandi.net ([217.70.178.86]:33701 "EHLO slow1-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1046029AbdDWUXA (ORCPT ); Sun, 23 Apr 2017 16:23:00 -0400 X-Originating-IP: 209.85.128.181 MIME-Version: 1.0 In-Reply-To: <1492929782-1112-1-git-send-email-bianpan2016@163.com> References: <1492929782-1112-1-git-send-email-bianpan2016@163.com> From: Pravin Shelar Date: Sun, 23 Apr 2017 13:22:53 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [ovs-dev] [PATCH 1/1] openvswitch: check return value of nla_nest_start To: Pan Bian Cc: Pravin Shelar , "David S. Miller" , Linux Kernel Network Developers , ovs dev , linux-kernel@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1986 Lines: 48 On Sat, Apr 22, 2017 at 11:43 PM, Pan Bian wrote: > Function nla_nest_start() will return a NULL pointer on error, and its > return value should be validated before it is used. However, in function > queue_userspace_packet(), its return value is ignored. This may result > in NULL dereference when calling nla_nest_end(). This patch fixes the > bug. > > Signed-off-by: Pan Bian > --- > net/openvswitch/datapath.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c > index 9c62b63..34c0fbd 100644 > --- a/net/openvswitch/datapath.c > +++ b/net/openvswitch/datapath.c > @@ -489,7 +489,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb, > err = ovs_nla_put_tunnel_info(user_skb, > upcall_info->egress_tun_info); > BUG_ON(err); > - nla_nest_end(user_skb, nla); > + if (nla) > + nla_nest_end(user_skb, nla); There is enough memory allocated for the netlink message accounting for all attributes beforehand. So it is not required to check retuned 'nla' after each attributes addition to the msg. > } > > if (upcall_info->actions_len) { > @@ -497,7 +498,7 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb, > err = ovs_nla_put_actions(upcall_info->actions, > upcall_info->actions_len, > user_skb); > - if (!err) > + if (!err && nla) > nla_nest_end(user_skb, nla); > else > nla_nest_cancel(user_skb, nla); > -- > 1.9.1 > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev