Hi Ben!

> It is probably a bug but I could not reproduce it.
> Note that commit 30cca512c (build: remove valgrind
> leftovers, 2019-11-25) is present in stable/2001
> so probably not the culprit...

Agreed.

> Can you share how you built VPP and your complete startup.conf?
> You seems to be running those commands from startup.conf directly.

Yes, I had those three commands in a file and then pointed to that file
as "exec /path/to/file" in the unix { } part of startup.conf.

Anyway, I got inspired and debugged the issue further myself: the
problem seems to be that the variable payload_proto in
vnet_ip_mroute_cmd() does not get set to anything, it end up having
whatever value was on the stack which could be any garbage.

My test works correctly after initializing it to zero, like this:

--- a/src/vnet/ip/lookup.c
+++ b/src/vnet/ip/lookup.c
@@ -661,7 +661,7 @@ vnet_ip_mroute_cmd (vlib_main_t * vm,
   unformat_input_t _line_input, *line_input = &_line_input;
   fib_route_path_t rpath, *rpaths = NULL;
   clib_error_t *error = NULL;
-  u32 table_id, is_del, payload_proto;
+  u32 table_id, is_del, payload_proto = 0;

If you want to reproduce the problem, you can simply set
payload_proto=77 (or whatever) instead of payload_proto=0 there, to
mimic garbage on the stack.

Just setting payload_proto=0 is probably not a good fix though, I guess
that just means hard-coding the FIB_PROTOCOL_IP4 value which happens to
work in my case.

To fix it properly I think payload_proto should be set to the
appropriate protocol in the different "else if" clauses, when
pfx.fp_proto is set then payload_proto should also be set, in the same
way as it is done in the vnet_ip_route_cmd() function.

I pushed a fix like that to gerrit, please have a look: 
https://gerrit.fd.io/r/c/vpp/+/27416

Best regards,
Elias

P.S.
By the way, do you think address sanitizer could be used to find this
kind of bugs?
(Or perhaps if there was a compiler option to poison the stack at each
function call, or something like that. I think it's a common problem
that code relies on uninitialized things being zero and that can
sometimes go undetected for a long time because things often happen to
be zero, forcing something nonzero could help detecting such bugs.)

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#16649): https://lists.fd.io/g/vpp-dev/message/16649
Mute This Topic: https://lists.fd.io/mt/74649468/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to