summaryrefslogtreecommitdiffstats
path: root/lib
diff options
context:
space:
mode:
authorDonald Sharp <sharpd@nvidia.com>2021-02-07 20:59:53 +0100
committerDonald Sharp <sharpd@nvidia.com>2021-02-07 20:59:53 +0100
commit37a74717c7d50eacb1bad842aa84546bbe5b954b (patch)
tree2dd9f5561780cab141c36f986bce6b2f3c95518f /lib
parentMerge pull request #7937 from pjdruddy/topotest-evpn-svi-advertise (diff)
downloadfrr-37a74717c7d50eacb1bad842aa84546bbe5b954b.tar.xz
frr-37a74717c7d50eacb1bad842aa84546bbe5b954b.zip
*: Fix usage of bfd_adj_event
Valgrind reports: 469901-==469901== 469901-==469901== Conditional jump or move depends on uninitialised value(s) 469901:==469901== at 0x3A090D: bgp_bfd_dest_update (bgp_bfd.c:416) 469901-==469901== by 0x497469E: zclient_read (zclient.c:3701) 469901-==469901== by 0x4955AEC: thread_call (thread.c:1684) 469901-==469901== by 0x48FF64E: frr_run (libfrr.c:1126) 469901-==469901== by 0x213AB3: main (bgp_main.c:540) 469901-==469901== Uninitialised value was created by a stack allocation 469901:==469901== at 0x3A0725: bgp_bfd_dest_update (bgp_bfd.c:376) 469901-==469901== 469901-==469901== Conditional jump or move depends on uninitialised value(s) 469901:==469901== at 0x3A093C: bgp_bfd_dest_update (bgp_bfd.c:421) 469901-==469901== by 0x497469E: zclient_read (zclient.c:3701) 469901-==469901== by 0x4955AEC: thread_call (thread.c:1684) 469901-==469901== by 0x48FF64E: frr_run (libfrr.c:1126) 469901-==469901== by 0x213AB3: main (bgp_main.c:540) 469901-==469901== Uninitialised value was created by a stack allocation 469901:==469901== at 0x3A0725: bgp_bfd_dest_update (bgp_bfd.c:376) On looking at bgp_bfd_dest_update the function call into bfd_get_peer_info when it fails to lookup the ifindex ifp pointer just returns leaving the dest and src prefix pointers pointing to whatever was passed in. Let's do two things: a) The src pointer was sometimes assumed to be passed in and sometimes not. Forget that. Make it always be passed in b) memset the src and dst pointers to be all zeros. Then when we look at either of the pointers we are not making decisions based upon random data in the pointers. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Diffstat (limited to 'lib')
-rw-r--r--lib/bfd.c22
1 files changed, 16 insertions, 6 deletions
diff --git a/lib/bfd.c b/lib/bfd.c
index cdf700860..3ab0e21af 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -224,6 +224,17 @@ struct interface *bfd_get_peer_info(struct stream *s, struct prefix *dp,
int plen;
int local_remote_cbit;
+ /*
+ * If the ifindex lookup fails the
+ * rest of the data in the stream is
+ * not read. All examples of this function
+ * call immediately use the dp->family which
+ * is not good. Ensure we are not using
+ * random data
+ */
+ memset(dp, 0, sizeof(*dp));
+ memset(sp, 0, sizeof(*sp));
+
/* Get interface index. */
ifindex = stream_getl(s);
@@ -249,13 +260,12 @@ struct interface *bfd_get_peer_info(struct stream *s, struct prefix *dp,
/* Get BFD status. */
*status = stream_getl(s);
- if (sp) {
- sp->family = stream_getc(s);
+ sp->family = stream_getc(s);
+
+ plen = prefix_blen(sp);
+ stream_get(&sp->u.prefix, s, plen);
+ sp->prefixlen = stream_getc(s);
- plen = prefix_blen(sp);
- stream_get(&sp->u.prefix, s, plen);
- sp->prefixlen = stream_getc(s);
- }
local_remote_cbit = stream_getc(s);
if (remote_cbit)
*remote_cbit = local_remote_cbit;