Hi Elias, Thanks for spotting that. Just make collector_port a u32 and add a boundary check?
Best regards, Ole > On 27 May 2020, at 03:27, Elias Rudberg <elias.rudb...@bahnhof.net> wrote: > > Hello VPP experts, > > When testing the current master branch for NAT with ipfix logging > enabled we encountered a problem with a segmentation fault crash. It > seems like this was caused by a bug in set_ipfix_exporter_command_fn() > in vnet/ipfix-export/flow_report.c where the variable collector_port > is declared as u16: > > u16 collector_port = UDP_DST_PORT_ipfix; > > and then a few lines later the address of that variable is given as > argument to unformat() with %u like this: > > else if (unformat (input, "port %u", &collector_port)) > > I think that is wrong because %u should correspond to a 32-bit > variable, so when passing the address of a 16-bit variable some data > next to it can get corrupted. In our case what happened was that the > "fib_index" variable that happened to be nearby on the stack got > corrupted, leading to a crash later on. > > The problem only appears for release build and not for debug, perhaps > because compiler optimization affects how variables are stored on the > stack. It could be that the compiler (clang or gcc) also matters, that > could explain why the problem was not seen earlier. > > Here is a fix, please check it and merge if you agree: > https://gerrit.fd.io/r/c/vpp/+/27280 > > Best regards, > Elias >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#16522): https://lists.fd.io/g/vpp-dev/message/16522 Mute This Topic: https://lists.fd.io/mt/74491544/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
