BIRD 2.0.7 segfault in route filters
Hi, It appears there is a segfault in the route specific filters, this was initially observed during a `configure`, but appears to be easily reproducible with startup config. Environment: OS: Ubuntu Focal/Xenial (from latest image on DockerHub) GCC: 9.3.0 / 5.4.0 Bird: 2.0.7 (from https://bird.network.cz/download/bird-2.0.7.tar.gz) / 61dae32 from Git Build options: --enable-debug / no options Reproduction configuration file; ``` router id 192.0.2.0; protocol device {} protocol static { ipv4 {}; route 192.0.2.0/24 via "lo" { return false; }; } ``` Backtrace; ``` Reading symbols from bird... BIRD pretty printers loaded OK. BIRD pretty printers loaded OK. (gdb) r -d -c test.conf Starting program: /bird-2.0.7/bird -d -c test.conf warning: Error disabling address space randomization: Operation not permitted [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Filter line 0000558f4c848340 (len=2) Instruction FI_CONSTANT at line 8 value FALSE Instruction FI_RETURN at line 8 Filter line 0000558f4c848340 dump done Interpreting line. Filter line 0000558f4c848340 (len=2) Instruction FI_CONSTANT at line 8 value FALSE Instruction FI_RETURN at line 8 Filter line 0000558f4c848340 dump done Program received signal SIGSEGV, Segmentation fault. 0x0000558f4b269796 in interpret (fs=0x7efd219df730, line=0x558f4c848340, val=0x0) at filter/f-inst.c:890 890 fstk->vcnt = fstk->estk[fstk->ecnt].ventry - 1; (gdb) bt #0 0x0000558f4b269796 in interpret (fs=0x7efd219df730, line=0x558f4c848340, val=0x0) at filter/f-inst.c:890 #1 0x0000558f4b26da99 in f_eval_rte (expr=0x558f4c848340, rte=0x7ffc421efc40, tmp_pool=0x558f4c856060) at filter/filter.c:352 #2 0x0000558f4b2f9c5e in static_announce_rte (p=0x558f4c84e7b0, r=0x558f4c8481e0) at proto/static/static.c:106 #3 0x0000558f4b2fa13a in static_add_rte (p=0x558f4c84e7b0, r=0x558f4c8481e0) at proto/static/static.c:228 #4 0x0000558f4b2fa9d9 in static_start (P=0x558f4c84e7b0) at proto/static/static.c:432 #5 0x0000558f4b29501f in proto_rethink_goal (p=0x558f4c84e7b0) at nest/proto.c:1173 #6 0x0000558f4b294e7f in protos_commit (new=0x558f4c842400, old=0x0, force_reconfig=0, type=1) at nest/proto.c:1132 #7 0x0000558f4b261018 in config_do_commit (c=0x558f4c842400, type=1) at conf/conf.c:271 #8 0x0000558f4b2611ef in config_commit (c=0x558f4c842400, type=1, timeout=0) at conf/conf.c:361 #9 0x0000558f4b30b793 in main (argc=4, argv=0x7ffc421eff58) at sysdep/unix/main.c:908 ``` It would be useful to be able to reject routes from their filter (to enable suppression in function logic, either via return or reject), however in the worst case the syntax should result in an invalid configuration error rather than a crash. Let me know if you need any more info. Cheers - Damian
On Sun, Dec 27, 2020 at 09:58:57PM +0100, Damian Zaremba wrote:
Hi,
It appears there is a segfault in the route specific filters, this was initially observed during a `configure`, but appears to be easily reproducible with startup config.
protocol static { ipv4 {}; route 192.0.2.0/24 via "lo" { return false; }; }
Hi Thanks for the bugreport, seems to me that the issue is related to 'return' at the top-level of the filter, it causes the crash also in regular filter. Will check that. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On Mon, Dec 28, 2020 at 04:29:25AM +0100, Ondrej Zajicek wrote:
On Sun, Dec 27, 2020 at 09:58:57PM +0100, Damian Zaremba wrote:
Hi,
It appears there is a segfault in the route specific filters, this was initially observed during a `configure`, but appears to be easily reproducible with startup config.
protocol static { ipv4 {}; route 192.0.2.0/24 via "lo" { return false; }; }
Hi
Thanks for the bugreport, seems to me that the issue is related to 'return' at the top-level of the filter, it causes the crash also in regular filter. Will check that.
Hi Attached patch fixes the issue. Top-level return now behaves like accept/reject and not crash BIRD. But route-specific filters in static protocol cannot really 'reject' route, they can just modify attributes (so reject / 'return false' is ignored). -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On 28/12/2020 15:06, Ondrej Zajicek wrote:
On Mon, Dec 28, 2020 at 04:29:25AM +0100, Ondrej Zajicek wrote:
On Sun, Dec 27, 2020 at 09:58:57PM +0100, Damian Zaremba wrote:
Hi,
It appears there is a segfault in the route specific filters, this was initially observed during a `configure`, but appears to be easily reproducible with startup config.
protocol static { ipv4 {}; route 192.0.2.0/24 via "lo" { return false; }; } Hi
Thanks for the bugreport, seems to me that the issue is related to 'return' at the top-level of the filter, it causes the crash also in regular filter. Will check that. Hi
Attached patch fixes the issue. Top-level return now behaves like accept/reject and not crash BIRD. But route-specific filters in static protocol cannot really 'reject' route, they can just modify attributes (so reject / 'return false' is ignored).
Hi, Thanks for the fast response and fix - I can confirm that is working as expected on my side. Unfortunately I did discover another segfault (testing from current master) which appears to be due to `net` being uninitialised in the route specific filters; Minimal reproduction config; ``` router id 192.0.2.0; protocol device {} protocol static { ipv4 {}; route 192.0.2.0/24 via "lo" { print net; }; } ``` Backtrace; ``` (gdb) r Starting program: /bird/bird -c /etc/bird/bird.conf -d warning: Error disabling address space randomization: Operation not permitted [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Program received signal SIGSEGV, Segmentation fault. 0x0000000000445872 in net_format (N=0xcdcdcdcdcdcdcde5, buf=0x7fff514b7ae0 "", buflen=257) at lib/net.c:82 82 switch (n->n.type) (gdb) bt #0 0x0000000000445872 in net_format (N=0xcdcdcdcdcdcdcde5, buf=0x7fff514b7ae0 "", buflen=257) at lib/net.c:82 #1 0x0000000000446db9 in bvsnprintf (buf=0x7fff514b8380 "\036", size=1024, fmt=0x4fb373 "N", args=0x7fff514b7c20) at lib/printf.c:246 #2 0x000000000044816b in buffer_print (buf=0x7fa22cfeb758, fmt=0x4fb372 "%N") at lib/printf.c:547 #3 0x0000000000431f17 in val_format (v=0x7fa22cfeb780, buf=0x7fa22cfeb758) at filter/data.c:563 #4 0x0000000000427df9 in interpret (fs=0x7fa22cfeb730, line=0xb46090, val=0x7fff514b8bc0) at filter/f-inst.c:478 #5 0x000000000042fecc in f_eval_rte (expr=0xb46090, rte=0x7fff514b8d08, tmp_pool=0xb537f0) at filter/filter.c:357 #6 0x00000000004c1260 in static_announce_rte (p=0xb4bd70, r=0xb45f00) at proto/static/static.c:109 #7 0x00000000004c172c in static_add_rte (p=0xb4bd70, r=0xb45f00) at proto/static/static.c:231 #8 0x00000000004c2083 in static_start (P=0xb4bd70) at proto/static/static.c:449 #9 0x0000000000459c03 in proto_rethink_goal (p=0xb4bd70) at nest/proto.c:1220 #10 0x0000000000459a6e in protos_commit (new=0xb40170, old=0x0, force_reconfig=0, type=1) at nest/proto.c:1179 #11 0x0000000000423093 in config_do_commit (c=0xb40170, type=1) at conf/conf.c:271 #12 0x000000000042325a in config_commit (c=0xb40170, type=1, timeout=0) at conf/conf.c:361 #13 0x00000000004d27f9 in main (argc=4, argv=0x7fff514b9018) at sysdep/unix/main.c:912 ``` This was originally triggered in a function call that used an `if net ~ set` operation, so while the above config doesn't really make practical sense it's the smallest filter I could make to repeatedly trigger the issue. I suspect my interpretation of the per-route filter expression support is a little too liberal compared to the intended usage. For now I will go back to applying filtering on a pipe from one "static table" into another "static table", which should solve my needs, but I wanted to highlight the second crash as it's not ideal behavior. Many Thanks - Damian
On Mon, Dec 28, 2020 at 06:47:29PM +0100, Damian Zaremba wrote:
Hi,
Thanks for the fast response and fix - I can confirm that is working as expected on my side.
Unfortunately I did discover another segfault (testing from current master) which appears to be due to `net` being uninitialised in the route specific filters;
This was originally triggered in a function call that used an `if net ~ set` operation, so while the above config doesn't really make practical sense it's the smallest filter I could make to repeatedly trigger the issue.
I suspect my interpretation of the per-route filter expression support is a little too liberal compared to the intended usage.
Well, this is something that likely should work (or at least not crash). Will check that. Seems like per-route filters are undertested feature.
For now I will go back to applying filtering on a pipe from one "static table" into another "static table", which should solve my needs, but I wanted to highlight the second crash as it's not ideal behavior.
You could probably just do that in import filter of the static protocol. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On Mon, Dec 28, 2020 at 07:23:51PM +0100, Ondrej Zajicek wrote:
On Mon, Dec 28, 2020 at 06:47:29PM +0100, Damian Zaremba wrote:
Hi,
Thanks for the fast response and fix - I can confirm that is working as expected on my side.
Unfortunately I did discover another segfault (testing from current master) which appears to be due to `net` being uninitialised in the route specific filters;
This was originally triggered in a function call that used an `if net ~ set` operation, so while the above config doesn't really make practical sense it's the smallest filter I could make to repeatedly trigger the issue.
Hi Fixed by the attached patch. Implementation of 'net' attribute is kind of ugly and requires some hacks, which were missing here. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
participants (2)
-
Damian Zaremba -
Ondrej Zajicek