Page MenuHomeFreeBSD

sctp: Fix a LOR in sctp_listen()
ClosedPublic

Authored by markj on Wed, Sep 8, 3:29 AM.

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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj requested review of this revision.Wed, Sep 8, 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.Wed, Sep 8, 1:19 PM