Skip to content

iouringfs: release MemoryFile allocations on error paths in New#13244

Open
ibondarenko1 wants to merge 1 commit into
google:masterfrom
ibondarenko1:hardening/iouringfs-cleanup-error-paths
Open

iouringfs: release MemoryFile allocations on error paths in New#13244
ibondarenko1 wants to merge 1 commit into
google:masterfrom
ibondarenko1:hardening/iouringfs-cleanup-error-paths

Conversation

@ibondarenko1
Copy link
Copy Markdown

Problem

iouringfs.New allocates two MemoryFile ranges and then runs several fallible steps. Each error path returns without releasing the allocations:

  • mf.Allocate(sqEntriesSize, ...) failing leaks rbfr (the first allocation).
  • iouringfd.vfsfd.Init(...) failing leaks rbfr and sqefr.
  • the two CacheLineRoundUp overflow checks (EOVERFLOW) leak both.
  • iouringfd.mapSharedBuffers() failing leaks both.
  • iouringfd.ioRingsBuf.view(...) and .writeback(...) failing leak both.

mf.Allocate returns a refcounted MemoryFile range. The success path transfers ownership to FileDescription.Release:

func (fd *FileDescription) Release(ctx context.Context) {
	fd.mf.DecRef(fd.rbmf.fr)
	fd.mf.DecRef(fd.sqemf.fr)
}

Release runs only once the FileDescription is installed and later released. On any error path in New the FileDescription is never returned, so Release never runs and the allocations leak. New is reachable from io_uring_setup(2).

Change

Wrap the two allocations in a cleanup.Cleanup: register mf.DecRef(rbfr) after the first allocation, mf.DecRef(sqefr) after the second, defer cu.Clean(), and cu.Release() on the success path. The cleanup performs exactly the two DecRef calls that Release does, so it is the correct teardown for every error path, including those after mapSharedBuffers (Release itself does no extra unmapping). This is the cleanup.Cleanup pattern already used across pkg/sentry/fsimpl.

Scope

Hardening. Resource leak confined to the error paths of New; triggering it requires the allocation or a later step to fail.

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.
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.

2 participants