| Message ID | 20240215143346.1715054-2-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 166c2c8a6a4dc2e4ceba9e10cfe81c3e469e3210 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v3,1/2] net/sched: act_mirred: use the backlog for mirred ingress | expand |
On Thu, Feb 15, 2024 at 9:33 AM Jakub Kicinski <kuba@kernel.org> wrote: > > 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> cheers, jamal > --- > CC: jhs@mojatatu.com > CC: xiyou.wangcong@gmail.com > CC: jiri@resnulli.us > --- > net/sched/act_mirred.c | 22 ++++++++++------------ > 1 file changed, 10 insertions(+), 12 deletions(-) > > diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c > index 291d47c9eb69..6faa7d00da09 100644 > --- 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, > -- > 2.43.0 >
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 291d47c9eb69..6faa7d00da09 100644 --- 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,