getsockopt: improve locking for SOL_SOCKET level socket options

[tuexen]

Summary

Ensure SOLISTENING() is done inside SOCK_LOCK()/SOCK_UNLOCK() for getsockopt() handling SOL_SOCKET-level socket options.

[markj]

All of the races are harmless though, i.e., the getsockopt call would return garbage in the worst case. There might be applications which query these socket options frequently, especially the socket buffer sizes. Do you think the extra overhead of acquiring the socket lock is unlikely to matter?

[tuexen]

In D46881#1069245, @markj wrote:

All of the races are harmless though, i.e., the getsockopt call would return garbage in the worst case. There might be applications which query these socket options frequently, especially the socket buffer sizes. Do you think the extra overhead of acquiring the socket lock is unlikely to matter?

I would not expect these socket options in the fast path. But if you prefer, I can just add comments, that the caller might get garbage and needs to deal with it.

[markj]

In D46881#1069317, @tuexen wrote:

In D46881#1069245, @markj wrote:

All of the races are harmless though, i.e., the getsockopt call would return garbage in the worst case. There might be applications which query these socket options frequently, especially the socket buffer sizes. Do you think the extra overhead of acquiring the socket lock is unlikely to matter?

I would not expect these socket options in the fast path. But if you prefer, I can just add comments, that the caller might get garbage and needs to deal with it.

I'm on the fence. The lock is unlikely to be contended, and this patch will help KCSAN. We've had more severe bugs involving lack of locking for SOLISTENING checks, so let's aim to be correct and avoid the lock only when it's demonstrably important for performance. I agree that these options aren't likely to be queried frequently.