From 47d92d8f9d9891a9b40f3fbef1d762537af4ae25 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Thu, 17 Jun 2021 16:56:51 +0200 Subject: [PATCH 1/6] Nest: Clean up main channel handling Remove assumption that main channel is the only channel. --- nest/protocol.h | 2 +- proto/ospf/config.Y | 5 ++--- proto/radv/config.Y | 1 + proto/radv/radv.c | 2 +- proto/rip/rip.c | 2 +- proto/rpki/rpki.c | 2 +- proto/static/static.c | 2 +- sysdep/unix/krt.c | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/nest/protocol.h b/nest/protocol.h index 48eb01d2..abcc505d 100644 --- a/nest/protocol.h +++ b/nest/protocol.h @@ -616,7 +616,7 @@ struct channel { struct channel_config *proto_cf_find_channel(struct proto_config *p, uint net_type); static inline struct channel_config *proto_cf_main_channel(struct proto_config *pc) -{ struct channel_config *cc = HEAD(pc->channels); return NODE_VALID(cc) ? cc : NULL; } +{ return proto_cf_find_channel(pc, pc->net_type); } struct channel *proto_find_channel_by_table(struct proto *p, struct rtable *t); struct channel *proto_find_channel_by_name(struct proto *p, const char *n); diff --git a/proto/ospf/config.Y b/proto/ospf/config.Y index fd2cfe8a..4b7d5a36 100644 --- a/proto/ospf/config.Y +++ b/proto/ospf/config.Y @@ -85,7 +85,7 @@ ospf_proto_finish(void) struct ospf_iface_patt *ic; /* Define default channel */ - if (EMPTY_LIST(this_proto->channels)) + if (! proto_cf_main_channel(this_proto)) { uint net_type = this_proto->net_type = ospf_cfg_is_v2() ? NET_IP4 : NET_IP6; channel_config_new(NULL, net_label[net_type], net_type, this_proto); @@ -248,8 +248,7 @@ ospf_channel_start: net_type ospf_af_mc $$ = this_channel = channel_config_get(NULL, net_label[$1], $1, this_proto); /* Save the multicast flag */ - if (this_channel == proto_cf_main_channel(this_proto)) - OSPF_CFG->af_mc = $2; + OSPF_CFG->af_mc = $2; }; ospf_channel: ospf_channel_start channel_opt_list channel_end; diff --git a/proto/radv/config.Y b/proto/radv/config.Y index dda9cfcd..8d4a3ab9 100644 --- a/proto/radv/config.Y +++ b/proto/radv/config.Y @@ -46,6 +46,7 @@ proto: radv_proto ; radv_proto_start: proto_start RADV { this_proto = proto_config_new(&proto_radv, $1); + this_proto->net_type = NET_IP6; init_list(&RADV_CFG->patt_list); init_list(&RADV_CFG->pref_list); diff --git a/proto/radv/radv.c b/proto/radv/radv.c index b4235917..66e8eb4b 100644 --- a/proto/radv/radv.c +++ b/proto/radv/radv.c @@ -564,7 +564,7 @@ radv_postconfig(struct proto_config *CF) // struct radv_config *cf = (void *) CF; /* Define default channel */ - if (EMPTY_LIST(CF->channels)) + if (! proto_cf_main_channel(CF)) channel_config_new(NULL, net_label[NET_IP6], NET_IP6, CF); } diff --git a/proto/rip/rip.c b/proto/rip/rip.c index 8b4719f7..e1a235a0 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -1105,7 +1105,7 @@ rip_postconfig(struct proto_config *CF) // struct rip_config *cf = (void *) CF; /* Define default channel */ - if (EMPTY_LIST(CF->channels)) + if (! proto_cf_main_channel(CF)) channel_config_new(NULL, net_label[CF->net_type], CF->net_type, CF); } diff --git a/proto/rpki/rpki.c b/proto/rpki/rpki.c index 799cb877..ab0837f3 100644 --- a/proto/rpki/rpki.c +++ b/proto/rpki/rpki.c @@ -923,7 +923,7 @@ rpki_postconfig(struct proto_config *CF) { /* Define default channel */ if (EMPTY_LIST(CF->channels)) - channel_config_new(NULL, net_label[CF->net_type], CF->net_type, CF); + cf_error("Channel not specified"); } static void diff --git a/proto/static/static.c b/proto/static/static.c index 661f1aac..2789c1bb 100644 --- a/proto/static/static.c +++ b/proto/static/static.c @@ -434,7 +434,7 @@ static_postconfig(struct proto_config *CF) struct static_config *cf = (void *) CF; struct static_route *r; - if (EMPTY_LIST(CF->channels)) + if (! proto_cf_main_channel(CF)) cf_error("Channel not specified"); struct channel_config *cc = proto_cf_main_channel(CF); diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c index ceb88563..7c2614b1 100644 --- a/sysdep/unix/krt.c +++ b/sysdep/unix/krt.c @@ -1013,7 +1013,7 @@ krt_postconfig(struct proto_config *CF) if (cf->c.class == SYM_TEMPLATE) return; - if (EMPTY_LIST(CF->channels)) + if (! proto_cf_main_channel(CF)) cf_error("Channel not specified"); #ifdef CONFIG_ALL_TABLES_AT_ONCE From eb2025165546d12114ca5ca4f2b338765f338af6 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Mon, 27 Apr 2020 22:33:10 +0200 Subject: [PATCH 2/6] Filter: Additional consistency checks --- filter/decl.m4 | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/filter/decl.m4 b/filter/decl.m4 index 5242c04c..6e135bbc 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -32,6 +32,7 @@ m4_divert(-1)m4_dnl # # 101 content of per-inst struct # 102 constructor arguments +# 110 constructor attributes # 103 constructor body # 104 dump line item content # (there may be nothing in dump-line content and @@ -45,6 +46,7 @@ m4_divert(-1)m4_dnl # Here are macros to allow you to _divert to the right directions. m4_define(FID_STRUCT_IN, `m4_divert(101)') m4_define(FID_NEW_ARGS, `m4_divert(102)') +m4_define(FID_NEW_ATTRIBUTES, `m4_divert(110)') m4_define(FID_NEW_BODY, `m4_divert(103)') m4_define(FID_DUMP_BODY, `m4_divert(104)m4_define([[FID_DUMP_BODY_EXISTS]])') m4_define(FID_LINEARIZE_BODY, `m4_divert(105)') @@ -106,15 +108,18 @@ FID_STRUCT_IN()m4_dnl struct f_inst * f$1; FID_NEW_ARGS()m4_dnl , struct f_inst * f$1 +FID_NEW_ATTRIBUTES()m4_dnl +NONNULL(m4_eval($1+1)) FID_NEW_BODY()m4_dnl whati->f$1 = f$1; -for (const struct f_inst *child = f$1; child; child = child->next) { - what->size += child->size; +const struct f_inst *child$1 = f$1; +do { + what->size += child$1->size; FID_IFCONST([[ - if (child->fi_code != FI_CONSTANT) + if (child$1->fi_code != FI_CONSTANT) constargs = 0; ]]) -} +} while (child$1 = child$1->next); FID_LINEARIZE_BODY pos = linearize(dest, whati->f$1, pos); FID_INTERPRET_BODY()') @@ -309,7 +314,9 @@ m4_undivert(107)m4_dnl FID_NEW()m4_dnl Constructor and interpreter code together FID_HIC( [[m4_dnl Public declaration of constructor in H file -struct f_inst *f_new_inst_]]INST_NAME()[[(enum f_instruction_code fi_code +struct f_inst * +m4_undivert(110)m4_dnl +f_new_inst_]]INST_NAME()[[(enum f_instruction_code fi_code m4_undivert(102)m4_dnl );]], [[m4_dnl The one case in The Big Switch inside interpreter @@ -321,7 +328,9 @@ m4_undivert(102)m4_dnl break; ]], [[m4_dnl Constructor itself -struct f_inst *f_new_inst_]]INST_NAME()[[(enum f_instruction_code fi_code +struct f_inst * +m4_undivert(110)m4_dnl +f_new_inst_]]INST_NAME()[[(enum f_instruction_code fi_code m4_undivert(102)m4_dnl ) { From 227e2d5541e86210b383c8e8054805692ad3cf14 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 26 May 2021 16:42:02 +0200 Subject: [PATCH 3/6] Debug output uses local buffer to avoid clashes between threads. --- sysdep/unix/log.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/sysdep/unix/log.c b/sysdep/unix/log.c index 14d18c01..4e9df069 100644 --- a/sysdep/unix/log.c +++ b/sysdep/unix/log.c @@ -309,22 +309,15 @@ die(const char *msg, ...) void debug(const char *msg, ...) { -#define MAX_DEBUG_BUFSIZE 65536 +#define MAX_DEBUG_BUFSIZE 16384 va_list args; - static uint bufsize = 4096; - static char *buf = NULL; - - if (!buf) - buf = mb_alloc(&root_pool, bufsize); + char buf[MAX_DEBUG_BUFSIZE]; va_start(args, msg); if (dbgf) { - while (bvsnprintf(buf, bufsize, msg, args) < 0) - if (bufsize >= MAX_DEBUG_BUFSIZE) - bug("Extremely long debug output, split it."); - else - buf = mb_realloc(buf, (bufsize *= 2)); + if (bvsnprintf(buf, MAX_DEBUG_BUFSIZE, msg, args) < 0) + bug("Extremely long debug output, split it."); fputs(buf, dbgf); } From 923a6644b206f70435c3d01b93b6880701938b49 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Thu, 27 May 2021 10:35:33 +0200 Subject: [PATCH 4/6] Fixed memory poisoning in slab --- lib/slab.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/slab.c b/lib/slab.c index b0a01ae7..8d16c433 100644 --- a/lib/slab.c +++ b/lib/slab.c @@ -269,6 +269,9 @@ no_partial: goto okay; } h = alloc_page(); +#ifdef POISON + memset(h, 0xba, get_page_size()); +#endif ASSERT_DIE(SL_GET_HEAD(h) == h); memset(h, 0, s->head_size); add_head(&s->partial_heads, &h->n); @@ -324,7 +327,12 @@ sl_free(slab *s, void *oo) { rem_node(&h->n); if (s->num_empty_heads >= MAX_EMPTY_HEADS) + { +#ifdef POISON + memset(h, 0xde, get_page_size()); +#endif free_page(h); + } else { add_head(&s->empty_heads, &h->n); From ceef6de459b25d8fcc5dd416e0cecf179bd243e7 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Thu, 27 May 2021 10:35:38 +0200 Subject: [PATCH 5/6] OSPF: Setting a list node NULL before use --- proto/ospf/iface.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proto/ospf/iface.c b/proto/ospf/iface.c index f38b8210..4cd45033 100644 --- a/proto/ospf/iface.c +++ b/proto/ospf/iface.c @@ -522,7 +522,10 @@ static inline void add_nbma_node(struct ospf_iface *ifa, struct nbma_node *src, int found) { struct nbma_node *n = mb_alloc(ifa->pool, sizeof(struct nbma_node)); + + n->n = (node) {}; add_tail(&ifa->nbma_list, NODE n); + n->ip = src->ip; n->eligible = src->eligible; n->found = found; From 2c13759136951ef0e70a3e3c2b2d3c9a387f7ed9 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 25 Aug 2021 22:20:48 +0200 Subject: [PATCH 6/6] Reducing filter stack size to allow for lesser thread stack size --- conf/conf.h | 1 + filter/config.Y | 8 ++++++- filter/decl.m4 | 3 ++- filter/f-inst.c | 1 + filter/filter.c | 63 ++++++++++++++++++++++++++++--------------------- test/birdtest.c | 2 +- 6 files changed, 48 insertions(+), 30 deletions(-) diff --git a/conf/conf.h b/conf/conf.h index 3bc37959..55cb9c58 100644 --- a/conf/conf.h +++ b/conf/conf.h @@ -35,6 +35,7 @@ struct config { u32 proto_default_debug; /* Default protocol debug mask */ u32 proto_default_mrtdump; /* Default protocol mrtdump mask */ u32 channel_default_debug; /* Default channel debug mask */ + u16 filter_vstk, filter_estk; /* Filter stack depth */ struct timeformat tf_route; /* Time format for 'show route' */ struct timeformat tf_proto; /* Time format for 'show protocol' */ struct timeformat tf_log; /* Time format for the logfile */ diff --git a/filter/config.Y b/filter/config.Y index 7820e719..e6b59cbe 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -288,7 +288,7 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, PREPEND, FIRST, LAST, LAST_NONAGGREGATED, MATCH, EMPTY, FILTER, WHERE, EVAL, ATTRIBUTE, - BT_ASSERT, BT_TEST_SUITE, BT_CHECK_ASSIGN, BT_TEST_SAME, FORMAT) + BT_ASSERT, BT_TEST_SUITE, BT_CHECK_ASSIGN, BT_TEST_SAME, FORMAT, STACKS) %nonassoc THEN %nonassoc ELSE @@ -312,6 +312,12 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, CF_GRAMMAR +conf: FILTER STACKS expr expr ';' { + new_config->filter_vstk = $3; + new_config->filter_estk = $4; + } + ; + conf: filter_def ; filter_def: FILTER symbol { $2 = cf_define_symbol($2, SYM_FILTER, filter, NULL); cf_push_scope( $2 ); } diff --git a/filter/decl.m4 b/filter/decl.m4 index 6e135bbc..44537aaa 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -195,6 +195,7 @@ FID_INTERPRET_BODY()') # that was needed in the former implementation. m4_define(LINEX, `FID_INTERPRET_EXEC()LINEX_($1)FID_INTERPRET_NEW()return $1 FID_INTERPRET_BODY()') m4_define(LINEX_, `do { + if (fstk->ecnt + 1 >= fstk->elen) runtime("Filter execution stack overflow"); fstk->estk[fstk->ecnt].pos = 0; fstk->estk[fstk->ecnt].line = $1; fstk->estk[fstk->ecnt].ventry = fstk->vcnt; @@ -232,7 +233,7 @@ FID_INTERPRET_BODY()') # state the result and put it to the right place. m4_define(RESULT, `RESULT_TYPE([[$1]]) RESULT_([[$1]],[[$2]],[[$3]])') m4_define(RESULT_, `RESULT_VAL([[ (struct f_val) { .type = $1, .val.$2 = $3 } ]])') -m4_define(RESULT_VAL, `FID_HIC(, [[do { res = $1; fstk->vcnt++; } while (0)]], +m4_define(RESULT_VAL, `FID_HIC(, [[do { res = $1; f_vcnt_check_overflow(1); fstk->vcnt++; } while (0)]], [[return fi_constant(what, $1)]])') m4_define(RESULT_VOID, `RESULT_VAL([[ (struct f_val) { .type = T_VOID } ]])') diff --git a/filter/f-inst.c b/filter/f-inst.c index b876a937..7c757e74 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -1003,6 +1003,7 @@ curline.vbase = curline.ventry; /* Storage for local variables */ + f_vcnt_check_overflow(sym->function->vars); memset(&(fstk->vstk[fstk->vcnt]), 0, sizeof(struct f_val) * sym->function->vars); fstk->vcnt += sym->function->vars; } diff --git a/filter/filter.c b/filter/filter.c index e505d570..7004b96d 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -50,30 +50,28 @@ enum f_exception { FE_RETURN = 0x1, }; - -struct filter_stack { - /* Value stack for execution */ -#define F_VAL_STACK_MAX 4096 - uint vcnt; /* Current value stack size; 0 for empty */ - uint ecnt; /* Current execute stack size; 0 for empty */ - - struct f_val vstk[F_VAL_STACK_MAX]; /* The stack itself */ - - /* Instruction stack for execution */ -#define F_EXEC_STACK_MAX 4096 - struct { - const struct f_line *line; /* The line that is being executed */ - uint pos; /* Instruction index in the line */ - uint ventry; /* Value stack depth on entry */ - uint vbase; /* Where to index variable positions from */ - enum f_exception emask; /* Exception mask */ - } estk[F_EXEC_STACK_MAX]; +struct filter_exec_stack { + const struct f_line *line; /* The line that is being executed */ + uint pos; /* Instruction index in the line */ + uint ventry; /* Value stack depth on entry */ + uint vbase; /* Where to index variable positions from */ + enum f_exception emask; /* Exception mask */ }; /* Internal filter state, to be allocated on stack when executing filters */ struct filter_state { /* Stacks needed for execution */ - struct filter_stack *stack; + struct filter_stack { + /* Current filter stack depth */ + + /* Value stack */ + uint vcnt, vlen; + struct f_val *vstk; + + /* Instruction stack for execution */ + uint ecnt, elen; + struct filter_exec_stack *estk; + } stack; /* The route we are processing. This may be NULL to indicate no route available. */ struct rte **rte; @@ -95,10 +93,13 @@ struct filter_state { }; _Thread_local static struct filter_state filter_state; -_Thread_local static struct filter_stack filter_stack; void (*bt_assert_hook)(int result, const struct f_line_item *assert); +#define _f_stack_init(fs, px, def) ((fs).stack.px##stk = alloca(sizeof(*(fs).stack.px##stk) * ((fs).stack.px##len = (config && config->filter_##px##stk) ? config->filter_##px##stk : (def)))) + +#define f_stack_init(fs) ( _f_stack_init(fs, v, 128), _f_stack_init(fs, e, 128) ) + static inline void f_cache_eattrs(struct filter_state *fs) { fs->eattrs = &((*fs->rte)->attrs->eattrs); @@ -163,15 +164,17 @@ interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val) ASSERT(line->args == 0); /* Initialize the filter stack */ - struct filter_stack *fstk = fs->stack; + struct filter_stack *fstk = &fs->stack; fstk->vcnt = line->vars; memset(fstk->vstk, 0, sizeof(struct f_val) * line->vars); /* The same as with the value stack. Not resetting the stack for performance reasons. */ fstk->ecnt = 1; - fstk->estk[0].line = line; - fstk->estk[0].pos = 0; + fstk->estk[0] = (struct filter_exec_stack) { + .line = line, + .pos = 0, + }; #define curline fstk->estk[fstk->ecnt-1] @@ -191,6 +194,8 @@ interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val) #define v2 vv(1) #define v3 vv(2) +#define f_vcnt_check_overflow(n) do { if (fstk->vcnt + n >= fstk->vlen) runtime("Filter execution stack overflow"); } while (0) + #define runtime(fmt, ...) do { \ if (!(fs->flags & FF_SILENT)) \ log_rl(&rl_runtime_err, L_ERR "filters, line %d: " fmt, what->lineno, ##__VA_ARGS__); \ @@ -276,12 +281,13 @@ f_run(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, i /* Initialize the filter state */ filter_state = (struct filter_state) { - .stack = &filter_stack, .rte = rte, .pool = tmp_pool, .flags = flags, }; + f_stack_init(filter_state); + LOG_BUFFER_INIT(filter_state.buf); /* Run the interpreter itself */ @@ -340,11 +346,12 @@ enum filter_return f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool) { filter_state = (struct filter_state) { - .stack = &filter_stack, .rte = rte, .pool = tmp_pool, }; + f_stack_init(filter_state); + LOG_BUFFER_INIT(filter_state.buf); ASSERT(!((*rte)->flags & REF_COW)); @@ -363,10 +370,11 @@ enum filter_return f_eval(const struct f_line *expr, struct linpool *tmp_pool, struct f_val *pres) { filter_state = (struct filter_state) { - .stack = &filter_stack, .pool = tmp_pool, }; + f_stack_init(filter_state); + LOG_BUFFER_INIT(filter_state.buf); enum filter_return fret = interpret(&filter_state, expr, pres); @@ -383,10 +391,11 @@ f_eval_int(const struct f_line *expr) { /* Called independently in parse-time to eval expressions */ filter_state = (struct filter_state) { - .stack = &filter_stack, .pool = cfg_mem, }; + f_stack_init(filter_state); + struct f_val val; LOG_BUFFER_INIT(filter_state.buf); diff --git a/test/birdtest.c b/test/birdtest.c index a1da078f..745118d0 100644 --- a/test/birdtest.c +++ b/test/birdtest.c @@ -240,7 +240,7 @@ bt_log_result(int result, u64 time, const char *fmt, va_list argptr) printf("%s\n", result_str); if (do_die && !result) - abort(); + exit(1); } static u64