crash in ospf lsupd/dbdes (only if authentication enabled?)
We have seen a bird crash due to memory corruption. The call stacks shows that it can happen at different locations, but they all seem to come from that the packet size becomes larger than the socket tx buffer size. Before the crash happens, the following trace can be observed in the log; 2020-01-08 11:06:52.584 <DBG> Assertion '*plen < ifa->sk->tbsize' failed at /usr/local/src/tm3000/ext/bird/proto/ospf/packet.c:97 Debugging shows that *plen is 1504 (after adding auth_len of 32 bytes). The tbsize is set to 1492 which corresponds to the MTU size. The procedure ospf_pkt_maxsize does not take account for authentication, is that correct? valgrind evidence of issue; ==3272== Invalid write of size 4 ==3272== at 0xFFB8594: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-ppc32-linux.so) ==3272== by 0x10015F6F: ??? (in /opt/appl/cuappl04a-r34a-39/sbin/bird) ==3272== by 0x10039B87: ??? (in /opt/appl/cuappl04a-r34a-39/sbin/bird) ==3272== by 0x1002E813: ??? (in /opt/appl/cuappl04a-r34a-39/sbin/bird) ==3272== by 0x1003978F: ??? (in /opt/appl/cuappl04a-r34a-39/sbin/bird) ==3272== by 0x10046963: ??? (in /opt/appl/cuappl04a-r34a-39/sbin/bird) ==3272== by 0x100473B7: ??? (in /opt/appl/cuappl04a-r34a-39/sbin/bird) ==3272== by 0x1000245F: ??? (in /opt/appl/cuappl04a-r34a-39/sbin/bird) (gdb) list *0x1002E813 0x1002e813 is in ospf_do_send_dbdes (/usr/local/src/builduser/rc/rel_34_0/ext/bird/proto/ospf/dbdes.c:196). 191 struct ospf_iface *ifa = n->ifa; 192 193 OSPF_PACKET(ospf_dump_dbdes, n->ldd_buffer, 194 "DBDES packet sent to nbr %R on %s", n->rid, ifa->ifname); 195 sk_set_tbuf(ifa->sk, n->ldd_buffer); 196 ospf_send_to(ifa, n->ip); 197 sk_set_tbuf(ifa->sk, NULL); 198 } 199 200 /** (gdb) list *0x10039B87 0x10039b87 is in ospf_send_to (/usr/local/src/builduser/rc/rel_34_0/ext/bird/proto/ospf/packet.c:110). 105 if (pass->alg < ALG_HMAC) 106 strncpy(auth_tail, pass->password, auth_len); 107 else 108 memset32(auth_tail, HMAC_MAGIC, auth_len / 4); 109 110 mac_fill(pass->alg, pass->password, pass->length, (byte *) pkt, *plen, auth_tail); 111 break; 112 113 default: 114 bug("Unknown authentication type"); (gdb) list *0x10015F6F 0x10015f6f is in mac_fill (/usr/local/src/builduser/rc/rel_34_0/ext/bird/lib/mac.h:112). 107 108 static inline void mac_update(struct mac_context *ctx, const byte *data, uint datalen) 109 { ctx->type->update(ctx, data, datalen); } 110 111 static inline byte *mac_final(struct mac_context *ctx) 112 { return ctx->type->final(ctx); } 113 114 static inline void mac_cleanup(struct mac_context *ctx) 115 { memset(ctx, 0, ctx->type->ctx_length); } 116 (gdb) ASAN evidence of the crash; ================================================================= ==3202==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xb4105a64 at pc 0x38630004 bp 0xbf958200 sp 0xbf958218 WRITE of size 4 at 0xb4105a64 thread T0 #0 0x38630000 (+0x10630000) #1 0xfb2aa10 in __asan_report_error (/usr/lib/gcc/powerpc-unknown-linux-gnu/4.9.3/libasan.so.1+0x65a10) #2 0xfb2bd70 in __asan_report_store4 (/usr/lib/gcc/powerpc-unknown-linux-gnu/4.9.3/libasan.so.1+0x66d70) #3 0x100d26d0 in ospf_send_to (/opt/appl/cuappl04a-r34a-39/sbin/bird+0x100d26d0) 0xb4105a64 is located 0 bytes to the right of 1508-byte region [0xb4105480,0xb4105a64) allocated by thread T0 here: #0 0x10060ab4 in bird_xmalloc (/opt/appl/cuappl04a-r34a-39/sbin/bird+0x10060ab4) SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 ?? Shadow bytes around the buggy address: 0x36820af0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x36820b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x36820b10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x36820b20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x36820b30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x36820b40: 00 00 00 00 00 00 00 00 00 00 00 00[04]fa fa fa 0x36820b50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x36820b60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x36820b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x36820b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x36820b90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Contiguous container OOB:fc ASan internal: fe ==3202==ABORTING
On Wed, Jan 08, 2020 at 11:52:40AM +0000, Kenth Eriksson wrote:
We have seen a bird crash due to memory corruption. The call stacks shows that it can happen at different locations, but they all seem to come from that the packet size becomes larger than the socket tx buffer size.
Before the crash happens, the following trace can be observed in the log;
2020-01-08 11:06:52.584 <DBG> Assertion '*plen < ifa->sk->tbsize' failed at /usr/local/src/tm3000/ext/bird/proto/ospf/packet.c:97
Debugging shows that *plen is 1504 (after adding auth_len of 32 bytes). The tbsize is set to 1492 which corresponds to the MTU size.
The procedure ospf_pkt_maxsize does not take account for authentication, is that correct?
It should take account for authentication: static inline uint ospf_pkt_maxsize(struct ospf_iface *ifa) { return ifa->tx_length - ifa->tx_hdrlen; } ... ifa->tx_hdrlen = ifa_tx_hdrlen(ifa); ... static inline uint ifa_tx_hdrlen(struct ospf_iface *ifa) { struct ospf_proto *p = ifa->oa->po; uint hlen = ospf_is_v2(p) ? IP4_HEADER_LENGTH : IP6_HEADER_LENGTH; /* Relevant just for OSPFv2 */ if (ifa->autype == OSPF_AUTH_CRYPT) { hlen += ospf_is_v2(p) ? 0 : sizeof(struct ospf_auth3); hlen += max_mac_length(ifa->passwords); } return hlen; } What do you have in ifa->tx_length and ifa->tx_hdrlen? -- 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 Wed, Jan 08, 2020 at 12:46:14PM +0000, Kenth Eriksson wrote:
On Wed, 2020-01-08 at 13:25 +0100, Ondrej Zajicek wrote:
What do you have in ifa->tx_length and ifa->tx_hdrlen?
*plen=1504, ifa->sk->tbsize=1492, auth_len=32, ifa->tx_length=1492, ifa->tx_hdrlen=20
Seems like tx_hdrlen is computed before autype and passwords fields are filled. I will send you patch. -- 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."
The OSPF packet size becomes larger than the socket tx buffer leading to memory corruptions (buffer overflow). Make sure that tx_hdrlen is computed after the autype and password lists are set. Signed-off-by: Kenth Eriksson <kenth.eriksson@infinera.com> --- proto/ospf/iface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/ospf/iface.c b/proto/ospf/iface.c index 7586deaf..9c5cf89e 100644 --- a/proto/ospf/iface.c +++ b/proto/ospf/iface.c @@ -587,13 +587,13 @@ ospf_iface_new(struct ospf_area *oa, struct ifa *addr, struct ospf_iface_patt *i ifa->stub = ospf_iface_stubby(ip, addr); ifa->ioprob = OSPF_I_OK; ifa->tx_length = ifa_tx_length(ifa); - ifa->tx_hdrlen = ifa_tx_hdrlen(ifa); ifa->check_link = ip->check_link; ifa->ecmp_weight = ip->ecmp_weight; ifa->check_ttl = (ip->ttl_security == 1); ifa->bfd = ip->bfd; ifa->autype = ip->autype; ifa->passwords = ip->passwords; + ifa->tx_hdrlen = ifa_tx_hdrlen(ifa); ifa->instance_id = ip->instance_id; ifa->ptp_netmask = !(addr->flags & IA_PEER); -- 2.21.0
On Wed, 2020-01-08 at 14:09 +0100, Ondrej Zajicek wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Wed, Jan 08, 2020 at 12:46:14PM +0000, Kenth Eriksson wrote:
On Wed, 2020-01-08 at 13:25 +0100, Ondrej Zajicek wrote:
What do you have in ifa->tx_length and ifa->tx_hdrlen?
*plen=1504, ifa->sk->tbsize=1492, auth_len=32, ifa->tx_length=1492, ifa->tx_hdrlen=20
Seems like tx_hdrlen is computed before autype and passwords fields are filled. I will send you patch.
Yes I think you are right. Probably my OSPF interface does not exist in the kernel when the config is set. And the sequence looks wrong in ospf_iface_new. I'm testing the patch I posted on the list now.
On Wed, 2020-01-08 at 14:09 +0100, Ondrej Zajicek wrote:
Seems like tx_hdrlen is computed before autype and passwords fields are filled. I will send you patch.
We have tested the patch I posten on the list, and results looks good. Noticed commit 9f2670277cc0d56d3364d4784348056174175aba in master repo, is this the final fix for this bug? /k
On Mon, Jan 13, 2020 at 09:25:32AM +0000, Kenth Eriksson wrote:
On Wed, 2020-01-08 at 14:09 +0100, Ondrej Zajicek wrote:
Seems like tx_hdrlen is computed before autype and passwords fields are filled. I will send you patch.
We have tested the patch I posten on the list, and results looks good. Noticed commit 9f2670277cc0d56d3364d4784348056174175aba in master repo, is this the final fix for this bug?
Yes, it also fixed the same issue in vlink code. -- 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 (3)
-
Kenth Eriksson -
Kenth Eriksson
-
Ondrej Zajicek