Commit a03ede64 authored by Ondřej Zajíček's avatar Ondřej Zajíček

Fixes a tricky bug in route filtering.

Route attributes was used after rta was freed during copy-on-write in
filter code. This causes some random crashes, esp. with multipath
parent 69a8259c
......@@ -485,24 +485,42 @@ val_print(struct f_val v)
static struct rte **f_rte, *f_rte_old;
static struct linpool *f_pool;
static struct rte **f_rte;
static struct rta *f_old_rta;
static struct ea_list **f_tmp_attrs;
static struct linpool *f_pool;
static int f_flags;
static inline void f_rte_cow(void)
*f_rte = rte_cow(*f_rte);
* rta_cow - prepare rta for modification by filter
static void
if ((*f_rte)->attrs->aflags & RTAF_CACHED) {
rta *f_rta_copy = lp_alloc(f_pool, sizeof(rta));
memcpy(f_rta_copy, (*f_rte)->attrs, sizeof(rta));
f_rta_copy->aflags = 0;
*f_rte = rte_cow(*f_rte);
(*f_rte)->attrs = f_rta_copy;
/* Prepare to modify rte */
/* Store old rta to free it later */
f_old_rta = (*f_rte)->attrs;
* Alloc new rta, do shallow copy and update rte. Fields eattrs
* and nexthops of rta are shared with f_old_rta (they will be
* copied when the cached rta will be obtained at the end of
* f_run()), also the lock of hostentry is inherited (we suppose
* hostentry is not changed by filters).
rta *ra = lp_alloc(f_pool, sizeof(rta));
memcpy(ra, f_old_rta, sizeof(rta));
ra->aflags = 0;
(*f_rte)->attrs = ra;
......@@ -819,7 +837,7 @@ interpret(struct f_inst *what)
if (what->aux != v1.type)
runtime( "Attempt to set static attribute to incompatible type" );
struct rta *rta = (*f_rte)->attrs;
switch (what->aux) {
......@@ -955,7 +973,7 @@ interpret(struct f_inst *what)
if (!(what->aux & EAF_TEMP) && (!(f_flags & FF_FORCE_TMPATTR))) {
l->next = (*f_rte)->attrs->eattrs;
(*f_rte)->attrs->eattrs = l;
} else {
......@@ -974,7 +992,7 @@ interpret(struct f_inst *what)
runtime( "Can't set preference to non-integer" );
if ((v1.val.i < 0) || (v1.val.i > 0xFFFF))
runtime( "Setting preference value out of bounds" );
*f_rte = rte_cow(*f_rte);
(*f_rte)->pref = v1.val.i;
case 'L': /* Get length of */
......@@ -1300,29 +1318,64 @@ i_same(struct f_inst *f1, struct f_inst *f2)
* f_run - external entry point to filters
* @filter: pointer to filter to run
* @tmp_attrs: where to store newly generated temporary attributes
* @rte: pointer to pointer to &rte being filtered. When route is modified, this is changed with rte_cow().
* f_run - run a filter for a route
* @filter: filter to run
* @rte: route being filtered, may be modified
* @tmp_attrs: temporary attributes, prepared by caller or generated by f_run()
* @tmp_pool: all filter allocations go from this pool
* @flags: flags
* If filter needs to modify the route, there are several
* posibilities. @rte might be read-only (with REF_COW flag), in that
* case rw copy is obtained by rte_cow() and @rte is replaced. If
* @rte is originally rw, it may be directly modified (and it is never
* copied).
* The returned rte may reuse the (possibly cached, cloned) rta, or
* (if rta was modificied) contains a modified uncached rta, which
* uses parts allocated from @tmp_pool and parts shared from original
* rta. There is one exception - if @rte is rw but contains a cached
* rta and that is modified, rta in returned rte is also cached.
* Ownership of cached rtas is consistent with rte, i.e.
* if a new rte is returned, it has its own clone of cached rta
* (and cached rta of read-only source rte is intact), if rte is
* modified in place, old cached rta is possibly freed.
f_run(struct filter *filter, struct rte **rte, struct ea_list **tmp_attrs, struct linpool *tmp_pool, int flags)
struct f_inst *inst;
struct f_val res;
int rte_cow = ((*rte)->flags & REF_COW);
DBG( "Running filter `%s'...", filter->name );
f_flags = flags;
f_tmp_attrs = tmp_attrs;
f_rte = rte;
f_rte_old = *rte;
f_old_rta = NULL;
f_tmp_attrs = tmp_attrs;
f_pool = tmp_pool;
inst = filter->root;
f_flags = flags;
res = interpret(inst);
struct f_val res = interpret(filter->root);
if (f_old_rta) {
* Cached rta was modified and f_rte contains now an uncached one,
* sharing some part with the cached one. The cached rta should
* be freed (if rte was originally COW, f_old_rta is a clone
* obtained during rte_cow()).
* This also implements the exception mentioned in f_run()
* description. The reason for this is that rta reuses parts of
* f_old_rta, and these may be freed during rta_free(f_old_rta).
* This is not the problem if rte was COW, because original rte
* also holds the same rta.
if (!rte_cow)
(*f_rte)->attrs = rta_lookup((*f_rte)->attrs);
if (res.type != T_RETURN) {
log( L_ERR "Filter %s did not return accept nor reject. Make up your mind", filter->name);
......@@ -1341,7 +1394,6 @@ f_eval_int(struct f_inst *expr)
f_flags = 0;
f_tmp_attrs = NULL;
f_rte = NULL;
f_rte_old = NULL;
f_pool = cfg_mem;
......@@ -60,6 +60,7 @@ static struct resclass sl_class = {
sizeof(struct slab),
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment