Page MenuHomeFreeBSD

sctp: Fix a LOR in sctp_listen()
ClosedPublic

Authored by markj on Sep 8 2021, 3:29 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 24, 11:09 AM
Unknown Object (File)
Thu, Oct 24, 11:09 AM
Unknown Object (File)
Thu, Oct 24, 11:09 AM
Unknown Object (File)
Thu, Oct 24, 10:50 AM
Unknown Object (File)
Sun, Oct 20, 12:49 PM
Unknown Object (File)
Fri, Oct 18, 10:22 PM
Unknown Object (File)
Thu, Oct 10, 5:49 AM
Unknown Object (File)
Oct 9 2024, 2:44 PM
Subscribers

Details

Summary

When port reuse is enabled in a one-to-one-style socket, sctp_listen()
may call sctp_swap_inpcb_for_listen() to move the PCB out of the "TCP
pool". In so doing it will drop the PCB lock, yielding an LOR since we
now hold several socket locks. Reorder sctp_listen() so that it
performs this operation before beginning the conversion to a listening
socket.

I'm not sure why I didn't hit this in my pre-commit testing, as I had
been running syzkaller for quite a while with those changes.

While here, remove a bogus lock upgrade: the PCB lock is just a mutex,
not a reader-writer lock as the macro names imply.

Fixes: bd4a39cc93d9 ("socket: Properly interlock when transitioning to a listening socket")
Reported by: syzkaller

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj requested review of this revision.Sep 8 2021, 3:29 AM

While here, remove a bogus lock upgrade: the PCB lock is just a mutex,
not a reader-writer lock as the macro names imply.

Where are you doing this? Even if it is a mutex on FreeBSD, it might not be on all platforms... So if possible, I would like to keep the rw-semantic, if possible.

While here, remove a bogus lock upgrade: the PCB lock is just a mutex,
not a reader-writer lock as the macro names imply.

Where are you doing this? Even if it is a mutex on FreeBSD, it might not be on all platforms... So if possible, I would like to keep the rw-semantic, if possible.

Sorry, the explanation is wrong (should be "downgrade"). sctp_swap_inpcb_for_listen() is now called with the inp write lock held, so we should return with the write lock held. As a result we don't need to do a lock downgrade. The fact that the PCB lock is a mutex on FreeBSD just meant that WITNESS didn't catch this.

While here, remove a bogus lock upgrade: the PCB lock is just a mutex,
not a reader-writer lock as the macro names imply.

Where are you doing this? Even if it is a mutex on FreeBSD, it might not be on all platforms... So if possible, I would like to keep the rw-semantic, if possible.

Sorry, the explanation is wrong (should be "downgrade"). sctp_swap_inpcb_for_listen() is now called with the inp write lock held, so we should return with the write lock held. As a result we don't need to do a lock downgrade. The fact that the PCB lock is a mutex on FreeBSD just meant that WITNESS didn't catch this.

OK. Sounds good.

This revision is now accepted and ready to land.Sep 8 2021, 1:19 PM