From 5835bc8c3a4bd1b2c41a40ff706282f6224aaffc Mon Sep 17 00:00:00 2001 From: Rahat Mahmood Date: Thu, 24 Mar 2022 15:07:06 -0700 Subject: [PATCH] cgroupfs: Handle invalid PID/PGID on migration. Reported-by: syzbot+670d686c42a0a8d7f8a6@syzkaller.appspotmail.com PiperOrigin-RevId: 437096386 --- pkg/sentry/fsimpl/cgroupfs/base.go | 8 +++++++- test/syscalls/linux/cgroup.cc | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/sentry/fsimpl/cgroupfs/base.go b/pkg/sentry/fsimpl/cgroupfs/base.go index 28a494ba2..ada829793 100644 --- a/pkg/sentry/fsimpl/cgroupfs/base.go +++ b/pkg/sentry/fsimpl/cgroupfs/base.go @@ -302,6 +302,9 @@ func (d *cgroupProcsData) Write(ctx context.Context, fd *vfs.FileDescription, sr t := kernel.TaskFromContext(ctx) currPidns := t.ThreadGroup().PIDNamespace() targetTG := currPidns.ThreadGroupWithID(kernel.ThreadID(tgid)) + if targetTG == nil { + return 0, linuxerr.EINVAL + } return n, targetTG.MigrateCgroup(d.Cgroup(fd)) } @@ -341,6 +344,9 @@ func (d *tasksData) Write(ctx context.Context, fd *vfs.FileDescription, src user t := kernel.TaskFromContext(ctx) currPidns := t.ThreadGroup().PIDNamespace() targetTask := currPidns.TaskWithID(kernel.ThreadID(tid)) + if targetTask == nil { + return 0, linuxerr.EINVAL + } return n, targetTask.MigrateCgroup(d.Cgroup(fd)) } @@ -362,7 +368,7 @@ func parseInt64FromString(ctx context.Context, src usermem.IOSequence) (val, len if err != nil { // Note: This also handles zero-len writes if offset is beyond the end // of src, or src is empty. - ctx.Warningf("cgroupfs.parseInt64FromString: failed to parse %q: %v", str, err) + ctx.Debugf("cgroupfs.parseInt64FromString: failed to parse %q: %v", str, err) return 0, int64(n), linuxerr.EINVAL } diff --git a/test/syscalls/linux/cgroup.cc b/test/syscalls/linux/cgroup.cc index 02a47b23f..6de704c38 100644 --- a/test/syscalls/linux/cgroup.cc +++ b/test/syscalls/linux/cgroup.cc @@ -403,6 +403,20 @@ TEST(Cgroup, MigrateToSubcontainerThread) { EXPECT_FALSE(tasks.contains(tid)); } +TEST(Cgroup, MigrateInvalidPID) { + SKIP_IF(!CgroupsAvailable()); + Mounter m(ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir())); + Cgroup c = ASSERT_NO_ERRNO_AND_VALUE(m.MountCgroupfs("")); + + EXPECT_THAT(c.WriteControlFile("cgroup.procs", "-1"), PosixErrorIs(EINVAL)); + EXPECT_THAT(c.WriteControlFile("cgroup.procs", "not-a-number"), + PosixErrorIs(EINVAL)); + + EXPECT_THAT(c.WriteControlFile("tasks", "-1"), PosixErrorIs(EINVAL)); + EXPECT_THAT(c.WriteControlFile("tasks", "not-a-number"), + PosixErrorIs(EINVAL)); +} + // Regression test for b/222278194. TEST(Cgroup, DuplicateUnlinkOnDirFD) { SKIP_IF(!CgroupsAvailable());