1
0
mirror of https://git.FreeBSD.org/src.git synced 2025-01-20 15:43:16 +00:00

udp: improve handling of cached route

The inp_route pointer should only be provided to the network
layer, when no destination address is provided. This is only
one of the conditions, where a write lock is needed.
If, for example, the route is also cached, when the socket is
unbound, problems show up, when the sendto is called, then
connect and finally send, when the route for the addresses
provided in the sendto and connect call use different outgoing
interfaces.
While there, clearly document why the write lock is taken.

Reported by:		syzbot+59122d2e848087d3355a@syzkaller.appspotmail.com
Reviewed by:		Peter Lei, glebius
MFC after:		3 days
Sponsored by:		Netflix, Inc.
Differential Revision:	https://reviews.freebsd.org/D46056
This commit is contained in:
Michael Tuexen 2024-07-28 23:25:22 +02:00
parent 34f746cc7f
commit 7186765300
2 changed files with 13 additions and 9 deletions

View File

@ -1087,7 +1087,6 @@ udp_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
uint16_t cscov = 0;
uint32_t flowid = 0;
uint8_t flowtype = M_HASHTYPE_NONE;
bool use_cached_route;
inp = sotoinpcb(so);
KASSERT(inp != NULL, ("udp_send: inp == NULL"));
@ -1116,16 +1115,21 @@ udp_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
sin = (struct sockaddr_in *)addr;
/*
* udp_send() may need to temporarily bind or connect the current
* inpcb. As such, we don't know up front whether we will need the
* pcbinfo lock or not. Do any work to decide what is needed up
* front before acquiring any locks.
* In the following cases we want a write lock on the inp for either
* local operations or for possible route cache updates in the IPv6
* output path:
* - on connected sockets (sin6 is NULL) for route cache updates,
* - when we are not bound to an address and source port (it is
* in_pcbinshash() which will require the write lock).
* - when we are using a mapped address on an IPv6 socket (for
* updating inp_vflag).
*
* We will need network epoch in either case, to safely lookup into
* pcb hash.
*/
use_cached_route = sin == NULL || (inp->inp_laddr.s_addr == INADDR_ANY && inp->inp_lport == 0);
if (use_cached_route || (flags & PRUS_IPV6) != 0)
if (sin == NULL ||
(inp->inp_laddr.s_addr == INADDR_ANY && inp->inp_lport == 0) ||
(flags & PRUS_IPV6) != 0)
INP_WLOCK(inp);
else
INP_RLOCK(inp);
@ -1476,7 +1480,7 @@ udp_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
else
UDP_PROBE(send, NULL, inp, &ui->ui_i, inp, &ui->ui_u);
error = ip_output(m, inp->inp_options,
use_cached_route ? &inp->inp_route : NULL, ipflags,
sin == NULL ? &inp->inp_route : NULL, ipflags,
inp->inp_moptions, inp);
INP_UNLOCK(inp);
NET_EPOCH_EXIT(et);

View File

@ -937,7 +937,7 @@ udp6_send(struct socket *so, int flags_arg, struct mbuf *m,
else
UDP_PROBE(send, NULL, inp, ip6, inp, udp6);
error = ip6_output(m, optp,
INP_WLOCKED(inp) ? &inp->inp_route6 : NULL, flags,
sin6 == NULL ? &inp->inp_route6 : NULL, flags,
inp->in6p_moptions, NULL, inp);
INP_UNLOCK(inp);
NET_EPOCH_EXIT(et);