This repository was archived by the owner on Feb 16, 2026. It is now read-only.
Repository navigation
ws_emu: Introduce the workspace emulator - #3644
Merged
Merged
Conversation
dridi
added
b=breaking
b=enhancement
c=build
c=H/2
c=varnishd
c=varnishtest
c=vmod
r=trunk
labels
Closed
Member
Author
|
It just occurred to me that the workspace sanitizer pull request has a test
case overflowing a write on purpose with the help of vmod_debug. I don't
remember doing it in this pull request.
TODO for later.
(from my phone)
|
Member
Author
|
All the original commits except the ones touching pipelining or the emulator were approved and merged already: 40471b9...5d7511c |
Contributor
|
I have stared at the pipeline stuff, and I see no obvious flaws in it. OK with me. |
Member
Author
I did find a not obvious flaw. I have not understood yet why the pipeline stuff breaks r02679.vtc for a 32bit build. |
dridi
marked this pull request as draft
August 23, 2021 04:24
Member
Author
|
Rebased against master, it should fail the new shiny 32bit job in circleci. |
Member
Author
|
Rebased against master, with the pipelining handled with more care to not let h2's req0 alone. Funny enough, a use-after-release was introduced in 1dbc688. |
dridi
marked this pull request as ready for review
August 26, 2021 06:19
When we take on a new request on a connection from which something was already received, we need to pipeline it and we do so at the beginning of the request workspace. There's a high probability that the pipeline is coming from the same workspace, which is a form of use-after-free only made safe by the workspace implementation details. To avoid the conceptual use-after-free, we defer req workspace rollbacks and perform them during the next HTC_RxInit() call before the pipelining operation. Because HTTP/1 works directly on the session, a worker can safely switch back and forth between sess and req tasks. This means that unless the session goes idle the same workspace is used from one client request to the next, hence the rollback previously happening in Req_Cleanup(). With h2 however there is a disconnect between the session and streams. The connection is received in req0's workspace, and then copied into a stream's req workspace via the pipelining scheme. Rollbacks can be deferred as well, but they need to happen otherwise the session will soon overflow. Independent HTC_RxInit() calls happen for req0 in the h2 session thread, and for h2 streams in the regular request task code path. PROXY Protocol parsing may result in receiving more than the proxy preamble itself and pipelining will happen, whether it is via a req for HTTP/1 or req0 for h2. On the other end of the spectrum when Varnish acts as a client it only sends one HTTP/1 request at a time for a given connection, so we never expect pipelining to occur in fetch task.
When a session has data pipelined we perform a dirty dance to move it at the beginning of the workspace. The rollbacks used to occur between HTC_RxPipeline() and HTC_RxInit() calls until it was centralized in the latter. With a dedicated WS_ReqPipeline() operation we can capture the semantics of initializing an existing connection for its next task with or without data fetched from the previous task. While conceptually there is still a use-after-free since the pipelined data may belong to the same workspace, it is fine if that happens within the bounds of an atomic workspace operation.
The goal of the workspace emulator is to replicate the regular workspace behavior with individual allocations and make it work transparently. It's the successor of the workspace sanitizer from varnishcache#3320 with notable differences: - enabled at configure time instead of run time - in a separate source file instead of mixed in - using sparse allocations instead of built-in red zones This means that the workspace emulator can be combined with regular sanitizer, in particular asan and lsan. If available, asan's public interface is used to mitigate the possible overflow of a reservation after some of it was released. Even without sanitizers, the fact that we integrate with jemalloc by default and enable its abort and junk options in varnishtest is enough to detect a use-after-free in some cases. With sanitizers though, the workspace emulator can observe varnishcache#3550. One drawback is that the logic is split in two files, and some functions are identical in the two files. It might be possible to split cache_ws.c into something like cache_ws_alloc.c and cache_ws_util.c for example. Closes varnishcache#3320 Refs varnishcache#3550 Refs varnishcache#3600
dridi
added a commit
that referenced
this pull request
Aug 30, 2021
Working on the workspace sanitizer (ancestor of the workspace emulator) final rollbacks were needed to unwind allocations. There was however a branch where error handling was missing a workspace release, and it was fine before the introduction of the final rollbacks. To avoid turning the workspace emulator into a DoS vector the rollbacks are now only enforced for emulator builds. The specific "insufficient workspace" log is amended to ensure future changes to the session workspace footprint don't accidentally remove test coverage for that branch. The same could be done for other "insufficient workspace" logs in the PROXY protocol parsing. Refs 0632b84 (req: Prevent early rollback) Refs ce71896 (sess: Plug conceptual leak) Refs 246b1eb (busyobj: Plug conceptual leak) Refs 5b4f0f1 (htc: Defer workspace rollbacks for request tasks) Refs #3644 Spotted by Alf's single process fuzzing setup that we should eventually revisit. Refs #3152
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The goal of the workspace emulator is to replicate the regular workspace behavior with individual allocations and make it work transparently.
It's the successor of the workspace sanitizer from #3320 with notable differences:
This means that the workspace emulator can be combined with regular sanitizer, in particular asan and lsan. If available, asan's public interface is used to mitigate the possible overflow of a reservation after some of it was released.
Even without sanitizers, the fact that we integrate with jemalloc by default and enable its abort and junk options in varnishtest is enough to detect a use-after-free in some cases.
With sanitizers though, the workspace emulator can observe #3550.
One drawback is that the logic is split in two files, and some functions are identical in the two files. It might be possible to split
cache_ws.cinto something likecache_ws_alloc.candcache_ws_util.cfor example.Closes #3320
Refs #3550
Refs #3600
All the changes leading up to the workspace emulator were either agreed upon already or necessary for the system to work.