[PATCH] Fixed crash when requesting a BFD session on an unnumbered interface

[Miao Wang]

> 2023年3月30日 23:23，Ondrej Zajicek <santiago at crfreenet.org> 写道：
> 
> On Thu, Mar 30, 2023 at 03:17:46PM +0800, Miao Wang wrote:
>>> 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.
>>> 
>>> 
>>> 
>>> 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).
>> 
>> Hi, according to your suggestion, I revised my patch. This time, 
>> bfd_get_iface is changed to accept an address family number, followed by
>> bfd_open_rx_sk_bound and bfd_open_tx_sk, where sk->subtype is set
>> explicitly, like that in bfd_open_rx_sk.
> 
> Hi
> 
> That makes sense, will merge that change.
> 
> But i am more sceptical about the whole idea of putting IPA_NONE as local
> address for BFD session. If the local IP is not well-defined (because it
> is some other address on some other interface chosen by kernel), how it
> can be configured on the other side? Also, it is incompatible with
> 'strict bind' option, but there is no check for it. For multihop BFD
> sessions, when local addresses cannot be determined from ifaces, we
> require them to be configured explicitly. What is your use case for this?

Hi,

You are right about that. I currently have no idea about the use case. I
raised the same question setting up a test environment for this. I just
came across this bug when trying to set up a bfd controlled static route,
in which case, I would like to set up a multi-hop session with a remote
station to monitor the whole link path. 

After investigating into the code, I realized that currently bird cannot
support this function, because the peer address of a bfd session requested
by a static route is fixed to its next hop.

I implemented a “bfd track” feature, letting a static route track an arbitrary
bfd session requested by the manually configured bfd neighbors in the bfd
protocol section.

The patch of the bfd track feature was sent in my following e-mail.

Cheers,

Miao Wang

> 
> -- 
> Elen sila lumenn' omentielvo
> 
> Ondrej 'Santiago' Zajicek (email: santiago at 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."