[lxc-devel] [lxd/master] seccomp: bugfixes
brauner on Github
lxc-bot at linuxcontainers.org
Thu Oct 15 08:57:59 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201015/828ca358/attachment-0001.bin>
-------------- next part --------------
From 79d6da7cd0634e13227672189923ed905131d563 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 15 Oct 2020 09:40:27 +0200
Subject: [PATCH 1/3] forkmount: improve
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
lxd/main_forkmount.go | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/lxd/main_forkmount.go b/lxd/main_forkmount.go
index 2e32756b91..a34d3957ea 100644
--- a/lxd/main_forkmount.go
+++ b/lxd/main_forkmount.go
@@ -329,17 +329,14 @@ static void do_lxc_forkumount(void)
void forkmount(void)
{
- char *cur = NULL;
-
- char *command = NULL;
+ char *command = NULL, *cur = NULL;
+ int ns_fd = -EBADF, pidfd = -EBADF;
pid_t pid = 0;
- int ns_fd = -EBADF, pidfd;
// Get the subcommand
command = advance_arg(false);
- if (command == NULL || (strcmp(command, "--help") == 0 || strcmp(command, "--version") == 0 || strcmp(command, "-h") == 0)) {
- return;
- }
+ if (command == NULL || (strcmp(command, "--help") == 0 || strcmp(command, "--version") == 0 || strcmp(command, "-h") == 0))
+ _exit(EXIT_SUCCESS);
// Check that we're root
if (geteuid() != 0) {
@@ -354,9 +351,9 @@ void forkmount(void)
if (strcmp(command, "lxd-mount") == 0) {
// Get the pid
cur = advance_arg(false);
- if (cur == NULL || (strcmp(cur, "--help") == 0 || strcmp(cur, "--version") == 0 || strcmp(cur, "-h") == 0)) {
- return;
- }
+ if (cur == NULL || (strcmp(cur, "--help") == 0 || strcmp(cur, "--version") == 0 || strcmp(cur, "-h") == 0))
+ _exit(EXIT_SUCCESS);
+
pid = atoi(cur);
if (pid <= 0)
_exit(EXIT_FAILURE);
@@ -372,9 +369,9 @@ void forkmount(void)
} else if (strcmp(command, "lxd-umount") == 0) {
// Get the pid
cur = advance_arg(false);
- if (cur == NULL || (strcmp(cur, "--help") == 0 || strcmp(cur, "--version") == 0 || strcmp(cur, "-h") == 0)) {
- return;
- }
+ if (cur == NULL || (strcmp(cur, "--help") == 0 || strcmp(cur, "--version") == 0 || strcmp(cur, "-h") == 0))
+ _exit(EXIT_SUCCESS);
+
pid = atoi(cur);
if (pid <= 0)
_exit(EXIT_FAILURE);
From 317ab6bc5285d849f0c30e35d7f5e73ca519faff Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 15 Oct 2020 10:56:29 +0200
Subject: [PATCH 2/3] seccomp: improve logging for the seccomp notifier
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
lxd/seccomp/seccomp.go | 53 ++++++++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 20 deletions(-)
diff --git a/lxd/seccomp/seccomp.go b/lxd/seccomp/seccomp.go
index e424b31b4b..699fdf85fb 100644
--- a/lxd/seccomp/seccomp.go
+++ b/lxd/seccomp/seccomp.go
@@ -947,6 +947,7 @@ retry:
return fmt.Errorf("Failed to send full response to seccomp client %v", siov.ucred.Pid)
}
+ logger.Debugf("Send seccomp notification for id(%d)", siov.resp.id)
return nil
}
@@ -1186,11 +1187,14 @@ func (s *Server) doDeviceSyscall(c Instance, args *MknodArgs, siov *Iovec) int {
// HandleMknodSyscall handles a mknod syscall.
func (s *Server) HandleMknodSyscall(c Instance, siov *Iovec) int {
ctx := log.Ctx{"container": c.Name(),
- "project": c.Project(),
- "syscall_number": siov.req.data.nr,
- "audit_architecture": siov.req.data.arch,
- "seccomp_notify_id": siov.req.id,
- "seccomp_notify_flags": siov.req.flags,
+ "project": c.Project(),
+ "syscall_number": siov.req.data.nr,
+ "audit_architecture": siov.req.data.arch,
+ "seccomp_notify_id": siov.req.id,
+ "seccomp_notify_flags": siov.req.flags,
+ "seccomp_notify_pid": siov.req.pid,
+ "seccomp_notify_fd": siov.notifyFd,
+ "seccomp_notify_mem_fd": siov.memFd,
}
defer logger.Debug("Handling mknod syscall", ctx)
@@ -1233,11 +1237,14 @@ func (s *Server) HandleMknodSyscall(c Instance, siov *Iovec) int {
// HandleMknodatSyscall handles a mknodat syscall.
func (s *Server) HandleMknodatSyscall(c Instance, siov *Iovec) int {
ctx := log.Ctx{"container": c.Name(),
- "project": c.Project(),
- "syscall_number": siov.req.data.nr,
- "audit_architecture": siov.req.data.arch,
- "seccomp_notify_id": siov.req.id,
- "seccomp_notify_flags": siov.req.flags,
+ "project": c.Project(),
+ "syscall_number": siov.req.data.nr,
+ "audit_architecture": siov.req.data.arch,
+ "seccomp_notify_id": siov.req.id,
+ "seccomp_notify_flags": siov.req.flags,
+ "seccomp_notify_pid": siov.req.pid,
+ "seccomp_notify_fd": siov.notifyFd,
+ "seccomp_notify_mem_fd": siov.memFd,
}
defer logger.Debug("Handling mknodat syscall", ctx)
@@ -1309,11 +1316,14 @@ type SetxattrArgs struct {
// HandleSetxattrSyscall handles setxattr syscalls.
func (s *Server) HandleSetxattrSyscall(c Instance, siov *Iovec) int {
ctx := log.Ctx{"container": c.Name(),
- "project": c.Project(),
- "syscall_number": siov.req.data.nr,
- "audit_architecture": siov.req.data.arch,
- "seccomp_notify_id": siov.req.id,
- "seccomp_notify_flags": siov.req.flags,
+ "project": c.Project(),
+ "syscall_number": siov.req.data.nr,
+ "audit_architecture": siov.req.data.arch,
+ "seccomp_notify_id": siov.req.id,
+ "seccomp_notify_flags": siov.req.flags,
+ "seccomp_notify_pid": siov.req.pid,
+ "seccomp_notify_fd": siov.notifyFd,
+ "seccomp_notify_mem_fd": siov.memFd,
}
defer logger.Debug("Handling setxattr syscall", ctx)
@@ -1588,11 +1598,14 @@ func (s *Server) mountHandleHugetlbfsArgs(c Instance, args *MountArgs, nsuid int
// HandleMountSyscall handles mount syscalls.
func (s *Server) HandleMountSyscall(c Instance, siov *Iovec) int {
ctx := log.Ctx{"container": c.Name(),
- "project": c.Project(),
- "syscall_number": siov.req.data.nr,
- "audit_architecture": siov.req.data.arch,
- "seccomp_notify_id": siov.req.id,
- "seccomp_notify_flags": siov.req.flags,
+ "project": c.Project(),
+ "syscall_number": siov.req.data.nr,
+ "audit_architecture": siov.req.data.arch,
+ "seccomp_notify_id": siov.req.id,
+ "seccomp_notify_flags": siov.req.flags,
+ "seccomp_notify_pid": siov.req.pid,
+ "seccomp_notify_fd": siov.notifyFd,
+ "seccomp_notify_mem_fd": siov.memFd,
}
defer logger.Debug("Handling mount syscall", ctx)
From f71ea4c4497a3a58d30f87c0f54beff032aca642 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 15 Oct 2020 10:56:45 +0200
Subject: [PATCH 3/3] seccomp: make sure that insertMountLXD() doesn't call
into LXC
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
lxd/instance/drivers/driver_lxc.go | 9 +++++++--
lxd/seccomp/seccomp.go | 10 +++++-----
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go
index ac307d6d1e..c2e57e1803 100644
--- a/lxd/instance/drivers/driver_lxc.go
+++ b/lxd/instance/drivers/driver_lxc.go
@@ -6066,7 +6066,12 @@ func (c *lxc) unmount() (bool, error) {
return unmounted, nil
}
-// Mount handling
+// insertMountLXD inserts a mount into a LXD container.
+// This function is used for the seccomp notifier and so cannot call any
+// functions that would cause LXC to talk to the container's monitor. Otherwise
+// we'll have a deadlock (with a timeout but still). The InitPID() call here is
+// the exception since the seccomp notifier will make sure to always pass a
+// valid PID.
func (c *lxc) insertMountLXD(source, target, fstype string, flags int, mntnsPID int, shiftfs bool) error {
pid := mntnsPID
if pid <= 0 {
@@ -6117,7 +6122,7 @@ func (c *lxc) insertMountLXD(source, target, fstype string, flags int, mntnsPID
mntsrc := filepath.Join("/dev/.lxd-mounts", filepath.Base(tmpMount))
pidStr := fmt.Sprintf("%d", pid)
- pidFdNr, pidFd := c.inheritInitPidFd()
+ pidFdNr, pidFd := seccomp.MakePidFd(pid, c.state)
if pidFdNr >= 0 {
defer pidFd.Close()
}
diff --git a/lxd/seccomp/seccomp.go b/lxd/seccomp/seccomp.go
index 699fdf85fb..7a4553dee2 100644
--- a/lxd/seccomp/seccomp.go
+++ b/lxd/seccomp/seccomp.go
@@ -656,8 +656,8 @@ func InstanceNeedsIntercept(s *state.State, c Instance) (bool, error) {
return needed, nil
}
-// InheritInitPidFd prepares a pidfd to inherit for the init process of the container.
-func inheritPidFd(pid int, s *state.State) (int, *os.File) {
+// MakePidFd prepares a pidfd to inherit for the init process of the container.
+func MakePidFd(pid int, s *state.State) (int, *os.File) {
if s.OS.PidFds {
pidFdFile, err := shared.PidFdOpen(pid, 0)
if err != nil {
@@ -1112,7 +1112,7 @@ func CallForkmknod(c Instance, dev deviceConfig.Device, requestPID int, s *state
return int(-C.EPERM)
}
- pidFdNr, pidFd := inheritPidFd(requestPID, s)
+ pidFdNr, pidFd := MakePidFd(requestPID, s)
if pidFdNr >= 0 {
defer pidFd.Close()
}
@@ -1332,7 +1332,7 @@ func (s *Server) HandleSetxattrSyscall(c Instance, siov *Iovec) int {
args.pid = int(siov.req.pid)
- pidFdNr, pidFd := inheritPidFd(args.pid, s.s)
+ pidFdNr, pidFd := MakePidFd(args.pid, s.s)
if pidFdNr >= 0 {
defer pidFd.Close()
}
@@ -1615,7 +1615,7 @@ func (s *Server) HandleMountSyscall(c Instance, siov *Iovec) int {
shift: s.MountSyscallShift(c),
}
- pidFdNr, pidFd := inheritPidFd(args.pid, s.s)
+ pidFdNr, pidFd := MakePidFd(args.pid, s.s)
if pidFdNr >= 0 {
defer pidFd.Close()
}
More information about the lxc-devel
mailing list