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 <[email protected]>
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
