Repository navigation
Shut down the socket in homa_sock_destroy if close didn't - #136
Open
Stuartwastaken wants to merge 1 commit into
Open
Stuartwastaken wants to merge 1 commit into
Stuartwastaken wants to merge 1 commit into
Conversation
* Since 2e848bd only homa_sock_shutdown unlinks a socket from the socktab. If inet_create fails after homa_socket (for example, a cgroup BPF program rejects the new socket), it calls only sk_common_release, so the freed socket stayed in the socktab, where homa_timer reads it on every tick.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
homa_socket links a new socket into the socktab, but since 2e848bd only homa_sock_shutdown (via homa_close) unlinks it. When inet_create fails after ->init, which is what happens when a cgroup BPF_CGROUP_INET_SOCK_CREATE program rejects the socket (net/ipv4/af_inet.c), it cleans up with sk_common_release, which calls ->destroy and ->unhash but never ->close. The socket is then freed while its homa_sock_link is still in the socktab, so homa_timer's socktab scan, homa_sock_find and homa_socktab_destroy read and refcount freed memory. It needs a deny program to be attached to the caller's cgroup; after that, any process in that cgroup can leave a stale entry with a single socket() call.
This calls homa_sock_shutdown at the start of homa_sock_destroy. It returns immediately when the socket is already shut down, so the normal close path doesn't change. The new homa_sock_destroy__unlink_socket test creates a socket and calls only homa_sock_destroy: without the fix, homa_sock_find still returns the socket, and AddressSanitizer reports a use-after-return in homa_socktab_fill_scan during teardown. With it, all 885 unit tests and the smoke tests pass.