iouringfs: release MemoryFile allocations on error paths in New#13244
Open
ibondarenko1 wants to merge 1 commit into
Open
iouringfs: release MemoryFile allocations on error paths in New#13244ibondarenko1 wants to merge 1 commit into
ibondarenko1 wants to merge 1 commit into
Conversation
New allocates two MemoryFile ranges (rbfr, sqefr) and then runs several fallible steps: vfsfd.Init, two offset overflow checks, mapSharedBuffers, and the ioRings view/writeback. Every one of those error paths returns without releasing the allocations, and a failed second Allocate leaks the first. The success path hands the allocations to FileDescription.Release, which is never reached on error. New is reachable from io_uring_setup(2). Wrap the allocations in a cleanup.Cleanup so they are released on every error path and retained only on success. The cleanup performs exactly what Release does, so it is the correct teardown for the post-mapSharedBuffers paths as well.
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.
Problem
iouringfs.Newallocates twoMemoryFileranges and then runs several fallible steps. Each error path returns without releasing the allocations:mf.Allocate(sqEntriesSize, ...)failing leaksrbfr(the first allocation).iouringfd.vfsfd.Init(...)failing leaksrbfrandsqefr.CacheLineRoundUpoverflow checks (EOVERFLOW) leak both.iouringfd.mapSharedBuffers()failing leaks both.iouringfd.ioRingsBuf.view(...)and.writeback(...)failing leak both.mf.Allocatereturns a refcountedMemoryFilerange. The success path transfers ownership toFileDescription.Release:Releaseruns only once theFileDescriptionis installed and later released. On any error path inNewtheFileDescriptionis never returned, soReleasenever runs and the allocations leak.Newis reachable fromio_uring_setup(2).Change
Wrap the two allocations in a
cleanup.Cleanup: registermf.DecRef(rbfr)after the first allocation,mf.DecRef(sqefr)after the second,defer cu.Clean(), andcu.Release()on the success path. The cleanup performs exactly the twoDecRefcalls thatReleasedoes, so it is the correct teardown for every error path, including those aftermapSharedBuffers(Releaseitself does no extra unmapping). This is thecleanup.Cleanuppattern already used acrosspkg/sentry/fsimpl.Scope
Hardening. Resource leak confined to the error paths of
New; triggering it requires the allocation or a later step to fail.