Repository navigation
core: Propagate make_sock() failures when duplicating listeners - #861
Open
arturobernalg wants to merge 1 commit into
Open
arturobernalg wants to merge 1 commit into
arturobernalg wants to merge 1 commit into
Conversation
Avoid continuing startup or restart with an invalid listening socket when make_sock() fails during listener duplication. PR 70106. Submitted by: Xuqing Yang
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.
ap_duplicate_listeners()ignored the return value ofmake_sock()for a duplicated listener. PR 70106, proposed there by Xuqing Yang.Root cause
On failure
make_sock()logs the error and closes the socket, whose APR descriptor becomes -1. The loop did not stop: it went on withap_apply_accept_filter()and added the dead listener to its bucket. Depending on the configuration:Listen: theAPR_SO_NONBLOCKcall is a no-op, only AH00076 is logged and startup or restart succeeds with fewer working listeners;Listenlines:apr_socket_opt_set()fails with EBADF on the closed socket, so startup fails, but with a misleading "Bad file descriptor" instead of the real cause.This is a descriptor used after its close, not freed memory.
Change
Return the error, so the MPM reports
could not duplicate listeners(AH03273/AH03291/AH03280) with the original status.Reproduction
ListenCoresBucketsRatio 4on 16 CPUs gives 4 buckets. A smallLD_PRELOADshim, kept outside the patch, fails the Nthbind()to the test port and logssocket,bind,closeand the calls made on the closed descriptor. Bind #1 is the original listener, #2 to #4 are the duplicates.Failing the second duplicate (
bind#3), before:After:
Results on unpatched trunk: event, worker and prefork with one
Listenstart silently with 3 of 4 listeners, and the same happens when the failure hits a restart (SIGUSR1 and SIGHUP). event with twoListenlines fails through the EBADF above. With the patch all of these fail with the real error and make no call on the closed descriptor.Without a fault, 24 of 24 combinations behave identically before and after (event, worker, prefork, one or two
Listen, start, graceful restart, restart): 4 listening sockets and a response on each.Limits
Linux only. Not covered: the systemd socket activation branch, IPv6, mpm_motorz, Windows and NetWare. The pytest suite was not run, and no regression test is added, since the failure can only be forced with a fault injection shim.
The same code is in 2.4.x, so this may be backported separately.
(see
SECURITY.mdandhttps://www.apache.org/security/#reporting-a-vulnerability).
APLOGNO()tag;numbers are assigned by a committer at merge time (see
docs/log-message-tags/README).changes-entries/*.txtfile is included,following the template in
README.CHANGES(not needed otherwise).test/modules/xxxdirectory matching themodules/xxxmodule source,or
test/modules/corefor core server changes. On Unix, verify thesepass locally with e.g.
make check-pytest PYTEST_DIRS=test/modules/xxx.