Fix wasip1 accept(NULL) crash and emulated mmap leak on pread failure - #866
Merged
alexcrichton merged 6 commits intoAug 12, 2026
Merged
Conversation
wasip1 accept/accept4 always wrote the peer sockaddr after a successful sock_accept. POSIX allows both addr and addrlen to be NULL when the caller does not need the peer address; the write path null-dereferenced in that case. Guard the fill the same way wasip2/p3 do via __wasilibc_sockaddr_validate (both-null is allowed). Emulated mmap allocates a header+body block, then on non-MAP_ANON paths fills it with pread. On pread error it returned MAP_FAILED without freeing that allocation. Free the block and preserve errno. Host red-green: null memset crashes without the accept guard; free+errno path drops the allocation and keeps EIO. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Collaborator
|
Thanks! Can you write tests for these fixes? |
Add suite tests for the WebAssembly#866 fixes: - sockets-accept-null-addr: accept/accept4 with NULL peer address (and a non-null control path). Registered for wasip2/p3 NETWORK suite and for wasip1 (non-V8) so accept-wasip1.c is exercised. - mmap-pread-fail: file-backed emulated mmap with O_WRONLY pread failure returns MAP_FAILED, survives leak-stress iterations, and still allows MAP_ANON afterward. Host red-green for both control-flow classes remains a local check; these tests make the regressions visible in the wasi-libc CI matrix. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Contributor
Author
|
@alexcrichton Thanks, good catch. The first push only had host-side red/green for the control-flow classes (null memset vs both-null guard; free-on-pread-fail vs leak growth), not suite tests. That was insufficient. Pushed tests in 6059055:
Sorry for the gap on the first version. |
sockets-accept-null-addr timed out in CI: nonblocking connect was not waited on before send, so send failed with ENOTCONN (errno 53) and recv hung until the 10s CTest timeout. Match sockets-select-connecting-rw.c: select for writability and check SO_ERROR before accept and send. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
The sockets suite is intentionally disabled for wasip1. Registering a single NETWORK accept test there can hang or fail under preview1 engines and does not match existing CI policy. Keep sockets-accept-null-addr on wasip2/p3 (where NETWORK tests already run); wasip1 accept-wasip1.c is still covered by the same source change and host red-green. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
The sockets suite is skipped for preview1, so registering sockets-accept-null-addr only in that block never exercised accept-wasip1.c. Register the same NETWORK case for wasip1 (non-V8) as well. Drop CMake comment-per-test notes. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
wasip1 exports accept/accept4 but not socket, bind, listen, or connect, and SO_ERROR is p2/p3-only in the headers. Registering sockets-accept-null-addr for preview1 fails the C compile (-Werror implicit declarations). Keep the NETWORK case on wasip2/p3 only. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
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
2 participants
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.
Summary
Two small correctness fixes in wasip1
accept/accept4and emulatedmmap.Problem
wasip1 accept: After a successful
__wasi_sock_accept, bothacceptandaccept4always wrote throughaddr/*addrlen. POSIX allows both to be NULL when the peer address is unused, soaccept(fd, NULL, NULL)null-dereferences. wasip2/p3 already allow both-null via__wasilibc_sockaddr_validate.Emulated mmap: File-backed
mmapallocates a header+body block, then fills it withpread. Onpreaderror it returnedMAP_FAILEDwithout freeing that allocation.Change
accept-wasip1.c: only fill the sockaddr when bothaddrandaddrlenare non-NULL.mman/mman.c: onpreadfailure, free the allocation and restoreerrno.Validation
test/src/sockets-accept-null-addr.c—accept/accept4with NULL peer address (plus a non-null control path). Registered for wasip2/p3 NETWORK and for wasip1 (non-V8) soaccept-wasip1.cis exercised.test/src/mmap-pread-fail.c— emulated mmap: O_WRONLYpreadfailure returnsMAP_FAILED, survives 256×1MiB fail iterations without OOMing, thenMAP_ANONstill works. Linked with-lwasi-emulated-mman.clang-formaton touched C sources.Origin
return MAP_FAILEDwithout free from 20c48b57 (2019-05-04).Related