netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Separate kernel.Task.AsCopyContext() into CopyContext() and OwnCopyContext().

Browse Source 
kernel.copyContext{t} cannot be used outside of t's task goroutine, for three
reasons:

- t.CopyScratchBuffer() is task-goroutine-local.

- Calling t.MemoryManager() without running on t's task goroutine or locking
  t.mu violates t.MemoryManager()'s preconditions.

- kernel.copyContext passes t as context.Context to MM IO methods, which is
  illegal outside of t's task goroutine (cf. kernel.Task.Value()).

Fix this by splitting AsCopyContext() into CopyContext() (which takes an
explicit context.Context and is usable outside of the task goroutine) and
OwnCopyContext() (which uses t as context.Context, but is only usable by t's
task goroutine).

PiperOrigin-RevId: 339933809
This commit is contained in:
Jamie Liu
2020-10-30 13:54:47 -07:00
committed by gVisor bot
parent c94bf137da
commit 9ad864628d
5 changed files with 119 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/sentry/kernel/ptrace.go
+2 -2
View File
@@ -1000,7 +1000,7 @@ func (t *Task) Ptrace(req int64, pid ThreadID, addr, data usermem.Addr) error {
// at the address specified by the data parameter, and the return value
// is the error flag." - ptrace(2)
word := t.Arch().Native(0)
if _, err := word.CopyIn(target.AsCopyContext(usermem.IOOpts{IgnorePermissions: true}), addr); err != nil {
if _, err := word.CopyIn(target.CopyContext(t, usermem.IOOpts{IgnorePermissions: true}), addr); err != nil {
return err
}
_, err := word.CopyOut(t, data)
@@ -1008,7 +1008,7 @@ func (t *Task) Ptrace(req int64, pid ThreadID, addr, data usermem.Addr) error {
case linux.PTRACE_POKETEXT, linux.PTRACE_POKEDATA:
word := t.Arch().Native(uintptr(data))
_, err := word.CopyOut(target.AsCopyContext(usermem.IOOpts{IgnorePermissions: true}), addr)
_, err := word.CopyOut(target.CopyContext(t, usermem.IOOpts{IgnorePermissions: true}), addr)
return err
case linux.PTRACE_GETREGSET:
pkg/sentry/kernel/task_clone.go
+1 -1
View File
@@ -355,7 +355,7 @@ func (t *Task) Clone(opts *CloneOptions) (ThreadID, *SyscallControl, error) {
}
if opts.ChildSetTID {
ctid := nt.ThreadID()
ctid.CopyOut(nt.AsCopyContext(usermem.IOOpts{AddressSpaceActive: false}), opts.ChildTID)
ctid.CopyOut(nt.CopyContext(t, usermem.IOOpts{AddressSpaceActive: false}), opts.ChildTID)
}
ntid := t.tg.pidns.IDOfTask(nt)
if opts.ParentSetTID {
pkg/sentry/kernel/task_usermem.go
+78 -17
View File
@@ -18,7 +18,8 @@ import (
"math"
"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/marshal"
"gvisor.dev/gvisor/pkg/context"
"gvisor.dev/gvisor/pkg/sentry/mm"
"gvisor.dev/gvisor/pkg/syserror"
"gvisor.dev/gvisor/pkg/usermem"
)
@@ -281,29 +282,89 @@ func (t *Task) IovecsIOSequence(addr usermem.Addr, iovcnt int, opts usermem.IOOp
}, nil
}
// copyContext implements marshal.CopyContext. It wraps a task to allow copying
// memory to and from the task memory with custom usermem.IOOpts.
type copyContext struct {
*Task
type taskCopyContext struct {
ctx context.Context
t *Task
opts usermem.IOOpts
}
// AsCopyContext wraps the task and returns it as CopyContext.
func (t *Task) AsCopyContext(opts usermem.IOOpts) marshal.CopyContext {
return &copyContext{t, opts}
// CopyContext returns a marshal.CopyContext that copies to/from t's address
// space using opts.
func (t *Task) CopyContext(ctx context.Context, opts usermem.IOOpts) *taskCopyContext {
return &taskCopyContext{
ctx: ctx,
t: t,
opts: opts,
}
}
// CopyInString copies a string in from the task's memory.
func (t *copyContext) CopyInString(addr usermem.Addr, maxLen int) (string, error) {
return usermem.CopyStringIn(t, t.MemoryManager(), addr, maxLen, t.opts)
// CopyScratchBuffer implements marshal.CopyContext.CopyScratchBuffer.
func (cc *taskCopyContext) CopyScratchBuffer(size int) []byte {
if ctxTask, ok := cc.ctx.(*Task); ok {
return ctxTask.CopyScratchBuffer(size)
}
return make([]byte, size)
}
// CopyInBytes copies task memory into dst from an IO context.
func (t *copyContext) CopyInBytes(addr usermem.Addr, dst []byte) (int, error) {
return t.MemoryManager().CopyIn(t, addr, dst, t.opts)
func (cc *taskCopyContext) getMemoryManager() (*mm.MemoryManager, error) {
cc.t.mu.Lock()
tmm := cc.t.MemoryManager()
cc.t.mu.Unlock()
if !tmm.IncUsers() {
return nil, syserror.EFAULT
}
return tmm, nil
}
// CopyOutBytes copies src into task memoryfrom an IO context.
func (t *copyContext) CopyOutBytes(addr usermem.Addr, src []byte) (int, error) {
return t.MemoryManager().CopyOut(t, addr, src, t.opts)
// CopyInBytes implements marshal.CopyContext.CopyInBytes.
func (cc *taskCopyContext) CopyInBytes(addr usermem.Addr, dst []byte) (int, error) {
tmm, err := cc.getMemoryManager()
if err != nil {
return 0, err
}
defer tmm.DecUsers(cc.ctx)
return tmm.CopyIn(cc.ctx, addr, dst, cc.opts)
}
// CopyOutBytes implements marshal.CopyContext.CopyOutBytes.
func (cc *taskCopyContext) CopyOutBytes(addr usermem.Addr, src []byte) (int, error) {
tmm, err := cc.getMemoryManager()
if err != nil {
return 0, err
}
defer tmm.DecUsers(cc.ctx)
return tmm.CopyOut(cc.ctx, addr, src, cc.opts)
}
type ownTaskCopyContext struct {
t *Task
opts usermem.IOOpts
}
// OwnCopyContext returns a marshal.CopyContext that copies to/from t's address
// space using opts. The returned CopyContext may only be used by t's task
// goroutine.
//
// Since t already implements marshal.CopyContext, this is only needed to
// override the usermem.IOOpts used for the copy.
func (t *Task) OwnCopyContext(opts usermem.IOOpts) *ownTaskCopyContext {
return &ownTaskCopyContext{
t: t,
opts: opts,
}
}
// CopyScratchBuffer implements marshal.CopyContext.CopyScratchBuffer.
func (cc *ownTaskCopyContext) CopyScratchBuffer(size int) []byte {
return cc.t.CopyScratchBuffer(size)
}
// CopyInBytes implements marshal.CopyContext.CopyInBytes.
func (cc *ownTaskCopyContext) CopyInBytes(addr usermem.Addr, dst []byte) (int, error) {
return cc.t.MemoryManager().CopyIn(cc.t, addr, dst, cc.opts)
}
// CopyOutBytes implements marshal.CopyContext.CopyOutBytes.
func (cc *ownTaskCopyContext) CopyOutBytes(addr usermem.Addr, src []byte) (int, error) {
return cc.t.MemoryManager().CopyOut(cc.t, addr, src, cc.opts)
}
test/syscalls/linux/BUILD
+4
View File
@@ -1802,10 +1802,14 @@ cc_binary(
"@com_google_absl//absl/flags:flag",
"@com_google_absl//absl/time",
gtest,
"//test/util:file_descriptor",
"//test/util:fs_util",
"//test/util:logging",
"//test/util:memory_util",
"//test/util:multiprocess_util",
"//test/util:platform_util",
"//test/util:signal_util",
"//test/util:temp_path",
"//test/util:test_util",
"//test/util:thread_util",
"//test/util:time_util",
test/syscalls/linux/ptrace.cc
+34 -9
View File
@@ -30,10 +30,13 @@
#include "absl/flags/flag.h"
#include "absl/time/clock.h"
#include "absl/time/time.h"
#include "test/util/fs_util.h"
#include "test/util/logging.h"
#include "test/util/memory_util.h"
#include "test/util/multiprocess_util.h"
#include "test/util/platform_util.h"
#include "test/util/signal_util.h"
#include "test/util/temp_path.h"
#include "test/util/test_util.h"
#include "test/util/thread_util.h"
#include "test/util/time_util.h"
@@ -113,10 +116,21 @@ TEST(PtraceTest, AttachParent_PeekData_PokeData_SignalSuppression) {
// except disabled.
SKIP_IF(ASSERT_NO_ERRNO_AND_VALUE(YamaPtraceScope()) > 0);
constexpr long kBeforePokeDataValue = 10;
constexpr long kAfterPokeDataValue = 20;
// Test PTRACE_POKE/PEEKDATA on both anonymous and file mappings.
const auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
ASSERT_NO_ERRNO(Truncate(file.path(), kPageSize));
const FileDescriptor fd =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR));
const auto file_mapping = ASSERT_NO_ERRNO_AND_VALUE(Mmap(
nullptr, kPageSize, PROT_READ | PROT_WRITE, MAP_SHARED, fd.get(), 0));
volatile long word = kBeforePokeDataValue;
constexpr long kBeforePokeDataAnonValue = 10;
constexpr long kAfterPokeDataAnonValue = 20;
constexpr long kBeforePokeDataFileValue = 0; // implicit, due to truncate()
constexpr long kAfterPokeDataFileValue = 30;
volatile long anon_word = kBeforePokeDataAnonValue;
auto* file_word_ptr = static_cast<volatile long*>(file_mapping.ptr());
pid_t const child_pid = fork();
if (child_pid == 0) {
@@ -134,12 +148,22 @@ TEST(PtraceTest, AttachParent_PeekData_PokeData_SignalSuppression) {
MaybeSave();
TEST_CHECK(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
// Replace the value of word in the parent process with kAfterPokeDataValue.
long const parent_word = ptrace(PTRACE_PEEKDATA, parent_pid, &word, 0);
// Replace the value of anon_word in the parent process with
// kAfterPokeDataAnonValue.
long parent_word = ptrace(PTRACE_PEEKDATA, parent_pid, &anon_word, 0);
MaybeSave();
TEST_CHECK(parent_word == kBeforePokeDataValue);
TEST_PCHECK(
ptrace(PTRACE_POKEDATA, parent_pid, &word, kAfterPokeDataValue) == 0);
TEST_CHECK(parent_word == kBeforePokeDataAnonValue);
TEST_PCHECK(ptrace(PTRACE_POKEDATA, parent_pid, &anon_word,
kAfterPokeDataAnonValue) == 0);
MaybeSave();
// Replace the value pointed to by file_word_ptr in the mapped file with
// kAfterPokeDataFileValue, via the parent process' mapping.
parent_word = ptrace(PTRACE_PEEKDATA, parent_pid, file_word_ptr, 0);
MaybeSave();
TEST_CHECK(parent_word == kBeforePokeDataFileValue);
TEST_PCHECK(ptrace(PTRACE_POKEDATA, parent_pid, file_word_ptr,
kAfterPokeDataFileValue) == 0);
MaybeSave();
// Detach from the parent and suppress the SIGSTOP. If the SIGSTOP is not
@@ -160,7 +184,8 @@ TEST(PtraceTest, AttachParent_PeekData_PokeData_SignalSuppression) {
<< " status " << status;
// Check that the child's PTRACE_POKEDATA was effective.
EXPECT_EQ(kAfterPokeDataValue, word);
EXPECT_EQ(kAfterPokeDataAnonValue, anon_word);
EXPECT_EQ(kAfterPokeDataFileValue, *file_word_ptr);
}
TEST(PtraceTest, GetSigMask) {