On Thu, Mar 30, 2023 at 06:02:38AM +0800, Miao Wang wrote:
Hi,
The bug can be reproduced with the following steps:
1. create an unnumbered interface:
# ip link add dummy0 type dummy # ip link set dummy0 up # ip neigh replace 169.254.0.1 lladdr <some:mac:add:ress> dev dummy0
2. Create a static route via this interface and request a bfd session
protocol device { } ipv4 table main4; protocol bfd bfd1 { strict bind; accept direct; } protocol static { ipv4{ table main4; export all; }; route 1.2.3.4/32 via 169.254.0.1%dummy0 onlink bfd; }
The crash happens when dereferencing nb->ifa in static protocol module. nb->ifa is null pointer in this case.
Hi Thanks for the bugreport. The code in static_update_bfd() has a clear bug, but i am not sure if just putting IPA_NONE to bfd_request_session() is a reasonable fix, i will have to check BFD code to see how much it assumes that the 'local' arg of bfd_request_session() is a valid address.
This patch also contains a fix in sk_setup(). When the source address and the destination address of a socket are both zero, the socket will be judged as an IPv6 socket since zero address is not a v4-mapped address. As a result, IPV6_V6ONLY is set on this socket. This patch prevents IPV6_V6ONLY from being set when both source address and destination address are zero.
I do not think this is a proper fix. sk_is_ipv6() uses sk->af, which should be here already assigned to proper value. If it is not, then the bug would be in the initial part of sk_open(), where AF value is decided based on subtype/saddr/daddr. I think it is mostly missing ASSERT() and issue in a caller of sk_open() for such socket. For sockets that have saddr and daddr zero, caller should set subtype (AF) manually, so combination of all zeroes in subtype/saddr/daddr is invalid (as for such socket there is no way how to decide whether it is IPv4 or IPv6 socket). -- 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."