Skip to content

Commit

Permalink
Merge branch 'mirred-recurse'
Browse files Browse the repository at this point in the history
John Hurley says:

====================
Track recursive calls in TC act_mirred

These patches aim to prevent act_mirred causing stack overflow events from
recursively calling packet xmit or receive functions. Such events can
occur with poor TC configuration that causes packets to travel in loops
within the system.

Florian Westphal advises that a recursion crash and packets looping are
separate issues and should be treated as such. David Miller futher points
out that pcpu counters cannot track the precise skb context required to
detect loops. Hence these patches are not aimed at detecting packet loops,
rather, preventing stack flows arising from such loops.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Jun 28, 2019
2 parents 5cdda5f + e2ca070 commit 8747d82
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 6 deletions.
2 changes: 1 addition & 1 deletion include/net/pkt_cls.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include <net/net_namespace.h>

/* TC action not accessible from user space */
#define TC_ACT_REINSERT (TC_ACT_VALUE_MAX + 1)
#define TC_ACT_CONSUMED (TC_ACT_VALUE_MAX + 1)

/* Basic packet classifier frontend definitions. */

Expand Down
2 changes: 1 addition & 1 deletion include/net/sch_generic.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ struct tcf_result {
};
const struct tcf_proto *goto_tp;

/* used by the TC_ACT_REINSERT action */
/* used in the skb_tc_reinsert function */
struct {
bool ingress;
struct gnet_stats_queue *qstats;
Expand Down
4 changes: 1 addition & 3 deletions net/core/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -4689,9 +4689,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
__skb_push(skb, skb->mac_len);
skb_do_redirect(skb);
return NULL;
case TC_ACT_REINSERT:
/* this does not scrub the packet, and updates stats on error */
skb_tc_reinsert(skb, &cl_res);
case TC_ACT_CONSUMED:
return NULL;
default:
break;
Expand Down
17 changes: 16 additions & 1 deletion net/sched/act_mirred.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
static LIST_HEAD(mirred_list);
static DEFINE_SPINLOCK(mirred_list_lock);

#define MIRRED_RECURSION_LIMIT 4
static DEFINE_PER_CPU(unsigned int, mirred_rec_level);

static bool tcf_mirred_is_act_redirect(int action)
{
return action == TCA_EGRESS_REDIR || action == TCA_INGRESS_REDIR;
Expand Down Expand Up @@ -210,13 +213,22 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
struct sk_buff *skb2 = skb;
bool m_mac_header_xmit;
struct net_device *dev;
unsigned int rec_level;
int retval, err = 0;
bool use_reinsert;
bool want_ingress;
bool is_redirect;
int m_eaction;
int mac_len;

rec_level = __this_cpu_inc_return(mirred_rec_level);
if (unlikely(rec_level > MIRRED_RECURSION_LIMIT)) {
net_warn_ratelimited("Packet exceeded mirred recursion limit on dev %s\n",
netdev_name(skb->dev));
__this_cpu_dec(mirred_rec_level);
return TC_ACT_SHOT;
}

tcf_lastuse_update(&m->tcf_tm);
bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);

Expand Down Expand Up @@ -277,7 +289,9 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
if (use_reinsert) {
res->ingress = want_ingress;
res->qstats = this_cpu_ptr(m->common.cpu_qstats);
return TC_ACT_REINSERT;
skb_tc_reinsert(skb, res);
__this_cpu_dec(mirred_rec_level);
return TC_ACT_CONSUMED;
}
}

Expand All @@ -292,6 +306,7 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
if (tcf_mirred_is_act_redirect(m_eaction))
retval = TC_ACT_SHOT;
}
__this_cpu_dec(mirred_rec_level);

return retval;
}
Expand Down

0 comments on commit 8747d82

Please sign in to comment.