[lxc-devel] [lxd/master] seccomp: fixes and improvements
brauner on Github
lxc-bot at linuxcontainers.org
Thu Aug 13 11:13:43 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/20200813/93ae5e5f/attachment-0001.bin>
-------------- next part --------------
From d95d70e35f8cf7d8a9b0e926cad9167eb76dc15a Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 13 Aug 2020 12:01:08 +0200
Subject: [PATCH 1/4] main_checkfeature: remove logging failed shiftfs mounts
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
lxd/main_checkfeature.go | 1 -
1 file changed, 1 deletion(-)
diff --git a/lxd/main_checkfeature.go b/lxd/main_checkfeature.go
index 4cebf7a5d0..a11ac9de74 100644
--- a/lxd/main_checkfeature.go
+++ b/lxd/main_checkfeature.go
@@ -584,7 +584,6 @@ func canUseShiftfs() bool {
err = unix.Mount(shared.VarPath(), shared.VarPath(), "shiftfs", 0, "mark")
if err != nil {
- logger.Debugf("%s - Failed to mount shiftfs", err)
return false
}
From 2171b5e4714f8c1b7206a14628c12d043dbafdf1 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 13 Aug 2020 12:04:13 +0200
Subject: [PATCH 2/4] seccomp: log errors to convert unix connection to file
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
lxd/seccomp/seccomp.go | 1 +
1 file changed, 1 insertion(+)
diff --git a/lxd/seccomp/seccomp.go b/lxd/seccomp/seccomp.go
index abeb307cd2..7467912b07 100644
--- a/lxd/seccomp/seccomp.go
+++ b/lxd/seccomp/seccomp.go
@@ -1008,6 +1008,7 @@ func NewSeccompServer(s *state.State, path string, findPID func(pid int32, state
unixFile, err := c.(*net.UnixConn).File()
if err != nil {
+ logger.Debugf("Failed to turn unix socket client into file")
return
}
From 0285fe3c23f1987f2238bdaee4fe9b865911b595 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 13 Aug 2020 13:04:35 +0200
Subject: [PATCH 3/4] unixfd: improve SCM_RIGHTs file descriptor retrieval
- Restrict the maximum number of file descriptors to be sent to us to 253. This
is the limit the kernel enforces and so isn't an arbitrary number.
If we receive more file descriptors than we will simple close all of the
received fds and report -EFBIG to the caller. There's no reason we should
receive insane amounts of file descriptors via the seccomp proxy even if the
kernel were to ever lift this limit.
- If we receive less file descriptors than we expect we'll place -EBADF into
the additional slots so the caller can check how many valid fds they actually
received.
- If we receive more file descriptors than we expect we'll simply close the
additional ones. The logic being that we might deal with a revised version of
the seccomp proxy in LXC that sends additional file descriptors. A LXD
version that doesn't know about the additional fds doesn't need to worry
about them.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
shared/netutils/unixfd.c | 37 +++++++++++++++++++++++++++++--------
1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/shared/netutils/unixfd.c b/shared/netutils/unixfd.c
index d3c3b1dacf..32f5c0ded6 100644
--- a/shared/netutils/unixfd.c
+++ b/shared/netutils/unixfd.c
@@ -64,6 +64,8 @@ ssize_t lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds, int num_recvfds,
struct iovec *iov, size_t iovlen)
{
__do_free char *cmsgbuf = NULL;
+ int new_fds[253]; /* Maximum number of supported fds to be sent in one message is 253. */
+ size_t num_new_fds = 0;
ssize_t ret;
struct msghdr msg = { 0 };
struct cmsghdr *cmsg = NULL;
@@ -95,14 +97,33 @@ ssize_t lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds, int num_recvfds,
// If SO_PASSCRED is set we will always get a ucred message.
for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
- if (cmsg->cmsg_type != SCM_RIGHTS)
- continue;
-
- memset(recvfds, -1, num_recvfds * sizeof(int));
- if (cmsg &&
- cmsg->cmsg_len == CMSG_LEN(num_recvfds * sizeof(int)) &&
- cmsg->cmsg_level == SOL_SOCKET)
- memcpy(recvfds, CMSG_DATA(cmsg), num_recvfds * sizeof(int));
+ if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) {
+ num_new_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int);
+
+ /*
+ * We received an insane amount of file descriptors
+ * which exceeds the kernel limit we know about so
+ * close them and return an error.
+ */
+ if (num_new_fds >= 253) {
+ int *fd_ptr = (int *)CMSG_DATA(cmsg);
+ for (size_t i = 0; i < num_new_fds; i++)
+ close(fd_ptr[i]);
+ return -EFBIG;
+ }
+
+ if (num_recvfds > num_new_fds) {
+ for (int i = num_new_fds; i < num_recvfds; i++)
+ recvfds[i] = -EBADF;
+ num_recvfds = num_new_fds;
+ }
+
+ memcpy(new_fds, CMSG_DATA(cmsg), num_new_fds * sizeof(int));
+ for (int i = num_recvfds; i < num_new_fds; i++)
+ close(new_fds[i]);
+
+ memcpy(recvfds, new_fds, num_recvfds * sizeof(int));
+ }
break;
}
From 63c950462ab011c833e6e75f3d63415db9c6ac00 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 13 Aug 2020 13:09:31 +0200
Subject: [PATCH 4/4] seccomp: simplify the seccomp message retrieval
Instead of checking whether the LXC version we're dealing with supports the
seccomp_proxy_send_notify_fd API extension we can simply use the new SCM_RIGHTS
file descriptor retrieval logic we introduced earlier. This allows us to
simplify the code.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
lxd/seccomp/seccomp.go | 40 ++++------------------------------------
1 file changed, 4 insertions(+), 36 deletions(-)
diff --git a/lxd/seccomp/seccomp.go b/lxd/seccomp/seccomp.go
index 7467912b07..61825f3abf 100644
--- a/lxd/seccomp/seccomp.go
+++ b/lxd/seccomp/seccomp.go
@@ -870,21 +870,8 @@ func (siov *Iovec) PutSeccompIovec() {
C.free(unsafe.Pointer(siov.iov))
}
-// ReceiveSeccompIovecV1 receives a v1 seccomp iovec.
-func (siov *Iovec) ReceiveSeccompIovecV1(fd int) (uint64, error) {
- bytes, fds, err := netutils.AbstractUnixReceiveFdData(fd, 2, unsafe.Pointer(siov.iov), 4)
- if err != nil || err == io.EOF {
- return 0, err
- }
-
- siov.procFd = int(fds[0])
- siov.memFd = int(fds[1])
-
- return bytes, nil
-}
-
-// ReceiveSeccompIovecV2 receives a v2 seccomp iovec.
-func (siov *Iovec) ReceiveSeccompIovecV2(fd int) (uint64, error) {
+// ReceiveSeccompIovec receives a seccomp iovec.
+func (siov *Iovec) ReceiveSeccompIovec(fd int) (uint64, error) {
bytes, fds, err := netutils.AbstractUnixReceiveFdData(fd, 3, unsafe.Pointer(siov.iov), 4)
if err != nil || err == io.EOF {
return 0, err
@@ -893,6 +880,7 @@ func (siov *Iovec) ReceiveSeccompIovecV2(fd int) (uint64, error) {
siov.procFd = int(fds[0])
siov.memFd = int(fds[1])
siov.notifyFd = int(fds[2])
+ logger.Debugf("Syscall handler received fds %d(/proc/<pid>), %d(/proc/<pid>/mem), and %d([seccomp notify])", siov.procFd, siov.memFd, siov.notifyFd)
return bytes, nil
}
@@ -1013,15 +1001,8 @@ func NewSeccompServer(s *state.State, path string, findPID func(pid int32, state
}
for {
- var bytes uint64
- var err error
-
siov := NewSeccompIovec(ucred)
- if lxcSupportSeccompV2(server.s) {
- bytes, err = siov.ReceiveSeccompIovecV2(int(unixFile.Fd()))
- } else {
- bytes, err = siov.ReceiveSeccompIovecV1(int(unixFile.Fd()))
- }
+ bytes, err := siov.ReceiveSeccompIovec(int(unixFile.Fd()))
if err != nil {
logger.Debugf("Disconnected from seccomp socket after failed receive: pid=%v, err=%s", ucred.Pid, err)
c.Close()
@@ -1902,19 +1883,6 @@ func (s *Server) Stop() error {
return s.l.Close()
}
-func lxcSupportSeccompV2(state *state.State) bool {
- err := lxcSupportSeccompNotify(state)
- if err != nil {
- return false
- }
-
- if !state.OS.LXCFeatures["seccomp_proxy_send_notify_fd"] {
- return false
- }
-
- return true
-}
-
func lxcSupportSeccompNotifyContinue(state *state.State) error {
err := lxcSupportSeccompNotify(state)
if err != nil {
More information about the lxc-devel
mailing list