Glad to hear it! On 2016-04-27 10:55, Baptiste Jonglez wrote:

Thanks for the quick patch Pavel, it seems to work fine!

On Wed, Apr 27, 2016 at 10:27:40AM +0200, Pavel Tvrdík wrote:

...

Hi Baptiste,

the bug should be fixed! Pull a new commit from GIT or download a mail attachment.

Thanks, Pavel

On 2016-04-26 17:37, Pavel Tvrdík wrote:

...

Hi Baptiste!

On 2016-04-26 16:46, Baptiste Jonglez wrote:

...

Hi,

I have been experimenting with the mrtdump branch (commit cc4eee62) on Debian jessie.

Thank you for testing experimental code from GIT!

...

However, when using a moderately long filename, Bird crashes. For instance:

birdc6 'mrtdump routes to "/bird/mrtdump/rib.ipv6.20160426.1209"' BIRD 1.5.0 ready. Connection closed by server.

The logs are the following:

bird6[8314]: Unable to open file "<too-long>" for MRT dump of table master systemd[1]: bird6.service: main process exited, code=killed, status=11/SEGV systemd[1]: Unit bird6.service entered failed state. kernel: bird6[8314]: segfault at 18 ip 00007fb05162a905 sp 00007ffe1194f5a0 error 4 in bird6[7fb051602000+76000]

It looks like TM_DATETIME_BUFFER_SIZE, as used in tm_format_datetime(), is really too small (32 bytes). Also, there seems to be an issue with error handling (segfault when tm_format_datetime returns unexpected data).

When defining a higher value for TM_DATETIME_BUFFER_SIZE, bird does not crash anymore, but there is certainly a better solution.

Yes, exactly. I'll send a patch tomorrow!

...

Thanks, Baptiste

...

From f7cd90113c281022acc50c42bcce1b44b7103395 Mon Sep 17 00:00:00 2001 From: Pavel Tvrdik <pawel.tvrdik@gmail.com> Date: Wed, 27 Apr 2016 10:12:26 +0200 Subject: [PATCH] MRT Dump: Fix bug with longer filename formats

Length of filename format is based on PATH_MAX. Better treating with filename format buffer size overflow, no segmentation fault. --- nest/route.h | 2 +- nest/rt-table.c | 31 ++++++++++++++++++++----------- 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/nest/route.h b/nest/route.h index 0d3a85f..9ea9563 100644 --- a/nest/route.h +++ b/nest/route.h @@ -628,6 +628,6 @@ struct mrt_table_config *mrt_table_new_config(void); struct mrt_table_dump_ctx; int is_route_good_for_table_dump(struct mrt_table_dump_ctx *state, rte *e); //char *mrt_table_dump_config_get_filename_fmt(struct rtable *rtable); -void mrt_table_dump_init_file_descriptor(struct mrt_table_dump_ctx *state); +struct rfile *mrt_table_dump_init_file_descriptor(struct mrt_table_dump_ctx *state);

#endif diff --git a/nest/rt-table.c b/nest/rt-table.c index 4420613..9550dd3 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -2752,10 +2752,13 @@ mrt_table_dump_cmd_step(void *mrt_table_dump_ctx) rfree(state->step); if (state->config.c.config) config_del_obstacle(state->config.c.config); - log(L_INFO "MRT dump of table %s was saved into file \"%s\"", state->rtable->name, state->file_path); - if (state->config.c.cli) + if (state->rfile) { - cli_printf(state->config.c.cli, 13, "Dump of table %s was saved into file \"%s\"", state->rtable->name, state->file_path); + log(L_INFO "MRT dump of table %s was saved into file \"%s\"", state->rtable->name, state->file_path); + if (state->config.c.cli) + { + cli_printf(state->config.c.cli, 13, "Dump of table %s was saved into file \"%s\"", state->rtable->name, state->file_path); + } } rt_unlock_table(state->rtable); mb_free(state->file_path); @@ -2783,10 +2786,12 @@ mrt_table_dump_init(struct mrt_table_dump_ctx *state)

state->state = MRT_STATE_RUNNING; state->rib_sequence_number = 0; - mrt_table_dump_init_file_descriptor(state); mrt_rib_table_alloc(&state->rib_table);

- bgp_mrt_peer_index_table_dump(state); + if (mrt_table_dump_init_file_descriptor(state)) + bgp_mrt_peer_index_table_dump(state); + else + state->state = MRT_STATE_COMPLETED; }

struct mrt_table_dump_ctx * @@ -2887,7 +2892,7 @@ mrt_table_dump_get_realpath(const char *filename) return path; }

-void +struct rfile * mrt_table_dump_init_file_descriptor(struct mrt_table_dump_ctx *state) { char *tablename = state->config.table_cf->name; @@ -2895,12 +2900,15 @@ mrt_table_dump_init_file_descriptor(struct mrt_table_dump_ctx *state)

if (filename_fmt) { - struct timeformat timestamp_fmt = { - .fmt1 = filename_fmt, - }; + char filename[BIRD_PATH_MAX]; + struct tm *tm = localtime(&now_real); + if (!strftime(filename, sizeof(filename), filename_fmt, tm)) + { + log(L_ERR "Invalid filename format \"%s\"", filename_fmt); + mb_free(filename_fmt); + return NULL; + }

- char filename[TM_DATETIME_BUFFER_SIZE]; - tm_format_datetime(filename, ×tamp_fmt, now); state->rfile = tracked_fopen(rt_table_pool, filename, "a");

const char *filename_fullpath = mrt_table_dump_get_realpath(filename); @@ -2933,6 +2941,7 @@ mrt_table_dump_init_file_descriptor(struct mrt_table_dump_ctx *state) cli_msg(13, "Parsing filename filename_fmt \"%s\" for table %s failed", mrt_table_dump_config_get_filename_fmt(state), tablename); } } + return state->rfile; }

static void