From 0e277a39c8b6f905e289b75e8ad0594e6b3562ca Mon Sep 17 00:00:00 2001 From: Rahat Mahmood Date: Thu, 1 Nov 2018 15:53:25 -0700 Subject: Prevent premature destruction of shm segments. Shm segments can be marked for lazy destruction via shmctl(IPC_RMID), which destroys a segment once it is no longer attached to any processes. We were unconditionally decrementing the segment refcount on shmctl(IPC_RMID) which allowed a user to force a segment to be destroyed by repeatedly calling shmctl(IPC_RMID), with outstanding memory maps to the segment. This is problematic because the memory released by a segment destroyed this way can be reused by a different process while remaining accessible by the process with outstanding maps to the segment. PiperOrigin-RevId: 219713660 Change-Id: I443ab838322b4fb418ed87b2722c3413ead21845 --- pkg/sentry/kernel/shm/shm.go | 13 +++++++++++-- pkg/sentry/syscalls/linux/sys_shm.go | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) (limited to 'pkg/sentry') diff --git a/pkg/sentry/kernel/shm/shm.go b/pkg/sentry/kernel/shm/shm.go index 2feffe612..f760f5f76 100644 --- a/pkg/sentry/kernel/shm/shm.go +++ b/pkg/sentry/kernel/shm/shm.go @@ -575,10 +575,19 @@ func (s *Shm) destroy() { func (s *Shm) MarkDestroyed() { s.mu.Lock() defer s.mu.Unlock() + // Prevent the segment from being found in the registry. s.key = linux.IPC_PRIVATE - s.pendingDestruction = true - s.DecRef() + + // Only drop the segment's self-reference once, when destruction is + // requested. Otherwise, repeated calls shmctl(IPC_RMID) would force a + // segment to be destroyed prematurely, potentially with active maps to the + // segment's address range. Remaining references are dropped when the + // segment is detached or unmaped. + if !s.pendingDestruction { + s.pendingDestruction = true + s.DecRef() + } } // checkOwnership verifies whether a segment may be accessed by ctx as an diff --git a/pkg/sentry/syscalls/linux/sys_shm.go b/pkg/sentry/syscalls/linux/sys_shm.go index 5f887523a..8753c2e58 100644 --- a/pkg/sentry/syscalls/linux/sys_shm.go +++ b/pkg/sentry/syscalls/linux/sys_shm.go @@ -144,7 +144,7 @@ func Shmctl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscal return 0, nil, nil case linux.SHM_LOCK, linux.SHM_UNLOCK: - // We currently do not support memmory locking anywhere. + // We currently do not support memory locking anywhere. // mlock(2)/munlock(2) are currently stubbed out as no-ops so do the // same here. t.Kernel().EmitUnimplementedEvent(t) -- cgit v1.2.3