net/sched: act_mirred: don't override retval if we already lost the skb

If we're redirecting the skb, and haven't called tcf_mirred_forward(),
yet, we need to tell the core to drop the skb by setting the retcode
to SHOT. If we have called tcf_mirred_forward(), however, the skb
is out of our hands and returning SHOT will lead to UaF.

Move the retval override to the error path which actually need it.

Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Fixes: e5cf1baf92cb ("act_mirred: use TC_ACT_REINSERT when possible")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 291d47c9eb69dedb30faca00c3c9145470ae12f6..6faa7d00da09771ae130581604c3b14c50472966 100644 (file)
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -266,8 +266,7 @@ static int tcf_mirred_to_dev(struct sk_buff *skb, struct tcf_mirred *m,
        if (unlikely(!(dev->flags & IFF_UP)) || !netif_carrier_ok(dev)) {
                net_notice_ratelimited("tc mirred to Houston: device %s is down\n",
                                       dev->name);
-               err = -ENODEV;
-               goto out;
+               goto err_cant_do;
        }
 
        /* we could easily avoid the clone only if called by ingress and clsact;
@@ -279,10 +278,8 @@ static int tcf_mirred_to_dev(struct sk_buff *skb, struct tcf_mirred *m,
                tcf_mirred_can_reinsert(retval);
        if (!dont_clone) {
                skb_to_send = skb_clone(skb, GFP_ATOMIC);
-               if (!skb_to_send) {
-                       err =  -ENOMEM;
-                       goto out;
-               }
+               if (!skb_to_send)
+                       goto err_cant_do;
        }
 
        want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
@@ -319,15 +316,16 @@ static int tcf_mirred_to_dev(struct sk_buff *skb, struct tcf_mirred *m,
        } else {
                err = tcf_mirred_forward(at_ingress, want_ingress, skb_to_send);
        }
-
-       if (err) {
-out:
+       if (err)
                tcf_action_inc_overlimit_qstats(&m->common);
-               if (is_redirect)
-                       retval = TC_ACT_SHOT;
-       }
 
        return retval;
+
+err_cant_do:
+       if (is_redirect)
+               retval = TC_ACT_SHOT;
+       tcf_action_inc_overlimit_qstats(&m->common);
+       return retval;
 }
 
 static int tcf_blockcast_redir(struct sk_buff *skb, struct tcf_mirred *m,