Skip to content

core: Propagate make_sock() failures when duplicating listeners - #861

Open
arturobernalg wants to merge 1 commit into
apache:trunkfrom
arturobernalg:fix/httpd-70106-duplicate-listeners
Open

arturobernalg wants to merge 1 commit into
apache:trunkfrom
arturobernalg:fix/httpd-70106-duplicate-listeners

Conversation

@arturobernalg

Copy link
Copy Markdown
Member

ap_duplicate_listeners() ignored the return value of make_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 with ap_apply_accept_filter() and added the dead listener to its bucket. Depending on the configuration:

  • one Listen: the APR_SO_NONBLOCK call is a no-op, only AH00076 is logged and startup or restart succeeds with fewer working listeners;
  • several Listen lines: 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 4 on 16 CPUs gives 4 buckets. A small LD_PRELOAD shim, kept outside the patch, fails the Nth bind() to the test port and logs socket, bind, close and 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:

bind(6) #3 = -1 (EADDRNOTAVAIL)   AH00072 make_sock: could not bind
close(6)
setsockopt(-1, IPPROTO_TCP, TCP_DEFER_ACCEPT) = EBADF   AH00076
AH00489 resuming normal operations      3 of 4 sockets listening

After:

bind(6) #3 = -1 (EADDRNOTAVAIL)   AH00072 make_sock: could not bind
close(6), close(5), close(3)
AH03273 could not duplicate listeners   exit status 1

Results on unpatched trunk: event, worker and prefork with one Listen start silently with 3 of 4 listeners, and the same happens when the failure hits a restart (SIGUSR1 and SIGHUP). event with two Listen lines 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.


  • This PR does not report or address a security vulnerability
    (see SECURITY.md and
    https://www.apache.org/security/#reporting-a-vulnerability).
  • Code follows the httpd style guide.
  • New log messages (level debug or higher) use an empty APLOGNO() tag;
    numbers are assigned by a committer at merge time (see docs/log-message-tags/README).
  • If the change is user-visible, a changes-entries/*.txt file is included,
    following the template in README.CHANGES (not needed otherwise).
  • Test cases based on pyhttpd are included where appropriate, in the
    test/modules/xxx directory matching the modules/xxx module source,
    or test/modules/core for core server changes. On Unix, verify these
    pass locally with e.g. make check-pytest PYTEST_DIRS=test/modules/xxx.

Avoid continuing startup or restart with an invalid listening socket
when make_sock() fails during listener duplication.

PR 70106.
Submitted by: Xuqing Yang
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant