diff mbox series

[ovs-dev,V3,06/24] datapath: fix GFP flags in rtnl_net_notifyid()

Message ID 20200916173311.30956-7-gvrose8192@gmail.com
State Changes Requested
Headers show
Series Add support for Linux kernels up to 5.8.x | expand

Commit Message

Gregory Rose Sept. 16, 2020, 5:32 p.m. UTC 
From: Guillaume Nault <gnault@redhat.com>

Upstream commit:
    commit d4e4fdf9e4a27c87edb79b1478955075be141f67
    Author: Guillaume Nault <gnault@redhat.com>
    Date:   Wed Oct 23 18:39:04 2019 +0200

    netns: fix GFP flags in rtnl_net_notifyid()

    In rtnl_net_notifyid(), we certainly can't pass a null GFP flag to
    rtnl_notify(). A GFP_KERNEL flag would be fine in most circumstances,
    but there are a few paths calling rtnl_net_notifyid() from atomic
    context or from RCU critical sections. The later also precludes the use
    of gfp_any() as it wouldn't detect the RCU case. Also, the nlmsg_new()
    call is wrong too, as it uses GFP_KERNEL unconditionally.

    Therefore, we need to pass the GFP flags as parameter and propagate it
    through function calls until the proper flags can be determined.

    In most cases, GFP_KERNEL is fine. The exceptions are:
      * openvswitch: ovs_vport_cmd_get() and ovs_vport_cmd_dump()
        indirectly call rtnl_net_notifyid() from RCU critical section,

      * rtnetlink: rtmsg_ifinfo_build_skb() already receives GFP flags as
        parameter.

    Also, in ovs_vport_cmd_build_info(), let's change the GFP flags used
    by nlmsg_new(). The function is allowed to sleep, so better make the
    flags consistent with the ones used in the following
    ovs_vport_cmd_fill_info() call.

    Found by code inspection.

    Fixes: 9a9634545c70 ("netns: notify netns id events")
    Signed-off-by: Guillaume Nault <gnault@redhat.com>
    Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
    Acked-by: Pravin B Shelar <pshelar@ovn.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Backport the datapath.c portion of this fix.

Cc:  Guillaume Nault <gnault@redhat.com>
Signed-off-by: Greg Rose <gvrose8192@gmail.com>
---
 datapath/datapath.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

Comments

Yi-Hung Wei Sept. 29, 2020, 9:43 p.m. UTC | #1 
On Wed, Sep 16, 2020 at 10:34 AM Greg Rose <gvrose8192@gmail.com> wrote:
>
> From: Guillaume Nault <gnault@redhat.com>
>
> Upstream commit:
>     commit d4e4fdf9e4a27c87edb79b1478955075be141f67
>     Author: Guillaume Nault <gnault@redhat.com>
>     Date:   Wed Oct 23 18:39:04 2019 +0200
>
>     netns: fix GFP flags in rtnl_net_notifyid()
>
>     In rtnl_net_notifyid(), we certainly can't pass a null GFP flag to
>     rtnl_notify(). A GFP_KERNEL flag would be fine in most circumstances,
>     but there are a few paths calling rtnl_net_notifyid() from atomic
>     context or from RCU critical sections. The later also precludes the use
>     of gfp_any() as it wouldn't detect the RCU case. Also, the nlmsg_new()
>     call is wrong too, as it uses GFP_KERNEL unconditionally.
>
>     Therefore, we need to pass the GFP flags as parameter and propagate it
>     through function calls until the proper flags can be determined.
>
>     In most cases, GFP_KERNEL is fine. The exceptions are:
>       * openvswitch: ovs_vport_cmd_get() and ovs_vport_cmd_dump()
>         indirectly call rtnl_net_notifyid() from RCU critical section,
>
>       * rtnetlink: rtmsg_ifinfo_build_skb() already receives GFP flags as
>         parameter.
>
>     Also, in ovs_vport_cmd_build_info(), let's change the GFP flags used
>     by nlmsg_new(). The function is allowed to sleep, so better make the
>     flags consistent with the ones used in the following
>     ovs_vport_cmd_fill_info() call.
>
>     Found by code inspection.
>
>     Fixes: 9a9634545c70 ("netns: notify netns id events")
>     Signed-off-by: Guillaume Nault <gnault@redhat.com>
>     Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>     Acked-by: Pravin B Shelar <pshelar@ovn.org>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
> Backport the datapath.c portion of this fix.
>
> Cc:  Guillaume Nault <gnault@redhat.com>
> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
> ---

LGTM.

Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>
diff mbox series

Patch

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 36f1e7894..1235c4e3d 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1990,7 +1990,7 @@  static struct genl_family dp_datapath_genl_family __ro_after_init = {
 /* Called with ovs_mutex or RCU read lock. */
 static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
 				   struct net *net, u32 portid, u32 seq,
-				   u32 flags, u8 cmd)
+				   u32 flags, u8 cmd, gfp_t gfp)
 {
 	struct ovs_header *ovs_header;
 	struct ovs_vport_stats vport_stats;
@@ -2012,7 +2012,7 @@  static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
 
 #ifdef HAVE_PEERNET2ID_ALLOC
 	if (!net_eq(net, dev_net(vport->dev))) {
-		int id = peernet2id_alloc(net, dev_net(vport->dev));
+		int id = peernet2id_alloc(net, dev_net(vport->dev), gfp);
 
 		if (nla_put_s32(skb, OVS_VPORT_ATTR_NETNSID, id))
 			goto nla_put_failure;
@@ -2054,11 +2054,12 @@  struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, struct net *net,
 	struct sk_buff *skb;
 	int retval;
 
-	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!skb)
 		return ERR_PTR(-ENOMEM);
 
-	retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd);
+	retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd,
+					 GFP_KERNEL);
 	BUG_ON(retval < 0);
 
 	return skb;
@@ -2200,7 +2201,7 @@  restart:
 
 	err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
 				      info->snd_portid, info->snd_seq, 0,
-				      OVS_VPORT_CMD_NEW);
+				      OVS_VPORT_CMD_NEW, GFP_KERNEL);
 	BUG_ON(err < 0);
 
 	new_headroom = netdev_get_fwd_headroom(vport->dev);
@@ -2260,7 +2261,7 @@  static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
 
 	err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
 				      info->snd_portid, info->snd_seq, 0,
-				      OVS_VPORT_CMD_SET);
+				      OVS_VPORT_CMD_SET, GFP_KERNEL);
 	BUG_ON(err < 0);
 	ovs_unlock();
 
@@ -2300,7 +2301,7 @@  static int ovs_vport_cmd_del(struct sk_buff *skb, struct genl_info *info)
 
 	err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
 				      info->snd_portid, info->snd_seq, 0,
-				      OVS_VPORT_CMD_DEL);
+				      OVS_VPORT_CMD_DEL, GFP_KERNEL);
 	BUG_ON(err < 0);
 
 	/* the vport deletion may trigger dp headroom update */
@@ -2347,7 +2348,7 @@  static int ovs_vport_cmd_get(struct sk_buff *skb, struct genl_info *info)
 		goto exit_unlock_free;
 	err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
 				      info->snd_portid, info->snd_seq, 0,
-				      OVS_VPORT_CMD_GET);
+				      OVS_VPORT_CMD_GET, GFP_ATOMIC);
 	BUG_ON(err < 0);
 	rcu_read_unlock();
 
@@ -2383,7 +2384,8 @@  static int ovs_vport_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
 						    NETLINK_CB(cb->skb).portid,
 						    cb->nlh->nlmsg_seq,
 						    NLM_F_MULTI,
-						    OVS_VPORT_CMD_GET) < 0)
+						    OVS_VPORT_CMD_GET,
+						    GFP_ATOMIC) < 0)
 				goto out;
 
 			j++;