d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

After parts of the locking fixes in r346595, syzkaller found

Browse Source 
another one in udp_output(). This one is a race condition.
We do check on the laddr and lport without holding a lock in
order to determine whether we want a read or a write lock
(this is in the "sendto/sendmsg" cases where addr (sin) is given).

Instrumenting the kernel showed that after taking the lock, we
had bound to a local port from a parallel thread on the same socket.

If we find that case, unlock, and retry again. Taking the write
lock would not be a problem in first place (apart from killing some
parallelism). However the retry is needed as later on based on
similar condition checks we do acquire the pcbinfo lock and if the
conditions have changed, we might find ourselves with a lock
inconsistency, hence at the end of the function when trying to
unlock, hitting the KASSERT.

Reported by:	syzbot+bdf4caa36f3ceeac198f@syzkaller.appspotmail.com
Reviewed by:	markj
MFC after:	6 weeks
Event:		Waterloo Hackathon 2019
This commit is contained in:
Bjoern A. Zeeb 2019-06-01 14:57:42 +00:00
parent 8adf420203
commit eafaa1bc35
1 changed files with 14 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
sys/netinet/udp_usrreq.c
View File

@ -1156,9 +1156,23 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct sockaddr *addr,
src.sin_family = 0;
sin = (struct sockaddr_in *)addr;
retry:
if (sin == NULL ||
(inp->inp_laddr.s_addr == INADDR_ANY && inp->inp_lport == 0)) {
INP_WLOCK(inp);
/*
* In case we lost a race and another thread bound addr/port
* on the inp we cannot keep the wlock (which still would be
* fine) as further down, based on these values we make
* decisions for the pcbinfo lock. If the locks are not in
* synch the assertions on unlock will fire, hence we go for
* one retry loop.
*/
if (sin != NULL && (inp->inp_laddr.s_addr != INADDR_ANY ||
inp->inp_lport != 0)) {
INP_WUNLOCK(inp);
goto retry;
}
unlock_inp = UH_WLOCKED;
} else {
INP_RLOCK(inp);