Page MenuHomeFreeBSD

MAC: improve handling of listening sockets
ClosedPublic

Authored by tuexen on Sep 22 2024, 8:59 PM.
Tags
None
Referenced Files
F102338961: D46755.id143663.diff
Sun, Nov 10, 9:51 PM
Unknown Object (File)
Tue, Nov 5, 4:04 AM
Unknown Object (File)
Mon, Nov 4, 5:04 PM
Unknown Object (File)
Sun, Nov 3, 7:22 PM
Unknown Object (File)
Sun, Nov 3, 1:05 AM
Unknown Object (File)
Thu, Oct 31, 11:29 PM
Unknown Object (File)
Fri, Oct 25, 5:56 PM
Unknown Object (File)
Mon, Oct 21, 6:49 PM
Subscribers

Details

Summary

so_peerlabel can only be used when the socket is not listening.

Test Plan
kldload mac_test
nc -l 1234

and then terminate nc using CTRL-C.

Diff Detail

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

Event Timeline

Hum, this bug is quite old. :(

sys/kern/uipc_socket.c
4178

The right place to check this is in mac_getsockopt_peerlabel(). Without the socket lock (which is acquired in that function), it is possible for listen() and getsockopt(SO_PEERLABEL) to race with each other.

tuexen added inline comments.
sys/kern/uipc_socket.c
4178

I totally agree. Fixed. But doesn't this comment apply also to the others usages of SOLISTENING() in sogetopt()?

sys/kern/uipc_socket.c
4178

Most likely, yes. There are many SOLISTENING races, unfortunately. Some of the ones below are mostly harmless in that they'll just return a garbage value if the race is lost.

sys/security/mac/mac_socket.c
584

Shouldn't this be in mac_getsockopt_peerlabel()?

Fix mac_getsockopt_label(), not mac_setsockopt_label() as pointed out by Mark.

tuexen added inline comments.
sys/kern/uipc_socket.c
4178

OK, I'll fix them. I decided to not call SOCK_LOCK(), because it wan't called in that places...

sys/security/mac/mac_socket.c
584

Yes, of course. Thanks for catching that.

markj added a subscriber: olivier.

I think this is ok. The bug suggests that we should optionally run the regression test suite with mac_test loaded. I tried it locally and easily reproduced the original panic. Perhaps @olivier would want to try this in his test environment?

This revision is now accepted and ready to land.Sep 24 2024, 8:47 AM
This revision was automatically updated to reflect the committed changes.
tuexen marked 2 inline comments as done.