Commit 166c2c8a authored by Jakub Kicinski's avatar Jakub Kicinski Committed by David S. Miller
Browse files

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: default avatarMichal Swiatkowski <michal.swiatkowski@linux.intel.com>
Fixes: e5cf1baf ("act_mirred: use TC_ACT_REINSERT when possible")
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 52f671db

net/sched/act_mirred.c

+10 −12
Original line number Diff line number Diff line
@@ -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,14 +316,15 @@ 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);



	return retval;



err_cant_do:

	if (is_redirect)

		retval = TC_ACT_SHOT;

	}



	tcf_action_inc_overlimit_qstats(&m->common);

	return retval;

}