Unverified Commit cca31bf0 by Stéphane Graber Committed by GitHub

Merge pull request #3697 from brauner/2021-02-25/fixes

commands: improvements and fixes
parents 2be31fed 885bb002
......@@ -113,7 +113,7 @@ int lxc_abstract_unix_connect(const char *path)
}
int lxc_abstract_unix_send_fds_iov(int fd, const int *sendfds, int num_sendfds,
struct iovec *iov, size_t iovlen)
struct iovec *const iov, size_t iovlen)
{
__do_free char *cmsgbuf = NULL;
int ret;
......@@ -176,6 +176,12 @@ static ssize_t lxc_abstract_unix_recv_fds_iov(int fd,
size_t cmsgbufsize = CMSG_SPACE(sizeof(struct ucred)) +
CMSG_SPACE(ret_fds->fd_count_max * sizeof(int));
if (ret_fds->flags & ~UNIX_FDS_ACCEPT_MASK)
return ret_errno(EINVAL);
if (hweight32((ret_fds->flags & ~UNIX_FDS_ACCEPT_NONE)) > 1)
return ret_errno(EINVAL);
cmsgbuf = zalloc(cmsgbufsize);
if (!cmsgbuf)
return ret_errno(ENOMEM);
......@@ -202,7 +208,7 @@ again:
if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) {
__u32 idx;
/*
* This causes some compilers to complaing about
* This causes some compilers to complain about
* increased alignment requirements but I haven't found
* a better way to deal with this yet. Suggestions
* welcome!
......@@ -225,7 +231,22 @@ again:
return syserrno_set(-EFBIG, "Received excessive number of file descriptors");
}
if (msg.msg_flags & MSG_CTRUNC) {
for (idx = 0; idx < num_raw; idx++)
close(fds_raw[idx]);
return syserrno_set(-EFBIG, "Control message was truncated; closing all fds and rejecting incomplete message");
}
if (ret_fds->fd_count_max > num_raw) {
if (!(ret_fds->flags & UNIX_FDS_ACCEPT_LESS)) {
for (idx = 0; idx < num_raw; idx++)
close(fds_raw[idx]);
return syserrno_set(-EINVAL, "Received fewer file descriptors than we expected %u != %u",
ret_fds->fd_count_max, num_raw);
}
/*
* Make sure any excess entries in the fd array
* are set to -EBADF so our cleanup functions
......@@ -234,16 +255,33 @@ again:
for (idx = num_raw; idx < ret_fds->fd_count_max; idx++)
ret_fds->fd[idx] = -EBADF;
WARN("Received fewer file descriptors than we expected %u != %u", ret_fds->fd_count_max, num_raw);
ret_fds->flags |= UNIX_FDS_RECEIVED_LESS;
} else if (ret_fds->fd_count_max < num_raw) {
if (!(ret_fds->flags & UNIX_FDS_ACCEPT_MORE)) {
for (idx = 0; idx < num_raw; idx++)
close(fds_raw[idx]);
return syserrno_set(-EINVAL, "Received more file descriptors than we expected %u != %u",
ret_fds->fd_count_max, num_raw);
}
/* Make sure we close any excess fds we received. */
for (idx = ret_fds->fd_count_max; idx < num_raw; idx++)
close(fds_raw[idx]);
WARN("Received more file descriptors than we expected %u != %u", ret_fds->fd_count_max, num_raw);
/* Cap the number of received file descriptors. */
num_raw = ret_fds->fd_count_max;
ret_fds->flags |= UNIX_FDS_RECEIVED_MORE;
} else {
ret_fds->flags |= UNIX_FDS_RECEIVED_EXACT;
}
if (hweight32((ret_fds->flags & ~UNIX_FDS_ACCEPT_MASK)) > 1) {
for (idx = 0; idx < num_raw; idx++)
close(fds_raw[idx]);
return syserrno_set(-EINVAL, "Invalid flag combination; closing to not risk leaking fds %u != %u",
ret_fds->fd_count_max, num_raw);
}
memcpy(ret_fds->fd, CMSG_DATA(cmsg), num_raw * sizeof(int));
......@@ -252,6 +290,15 @@ again:
}
}
if (ret_fds->fd_count_ret == 0) {
ret_fds->flags |= UNIX_FDS_RECEIVED_NONE;
/* We expected to receive file descriptors. */
if ((ret_fds->flags & UNIX_FDS_ACCEPT_MASK) &&
!(ret_fds->flags & UNIX_FDS_ACCEPT_NONE))
return syserrno_set(-EINVAL, "Received no file descriptors");
}
return ret;
}
......
......@@ -12,15 +12,88 @@
#include "macro.h"
#include "memory_utils.h"
#define KERNEL_SCM_MAX_FD 253
/* Allow the caller to set expectations. */
/*
* UNIX_FDS_ACCEPT_EXACT will only succeed if the exact amount of fds has been
* received (unless combined with UNIX_FDS_ACCEPT_NONE).
*/
#define UNIX_FDS_ACCEPT_EXACT ((__u32)(1 << 0)) /* default */
/*
* UNIX_FDS_ACCEPT_LESS will also succeed if less than the requested number of
* fd has been received. If the UNIX_FDS_ACCEPT_NONE flag is not raised than at
* least one fd must be received.
* */
#define UNIX_FDS_ACCEPT_LESS ((__u32)(1 << 1))
/*
* UNIX_FDS_ACCEPT_MORE will also succeed if more than the requested number of
* fds have been received. Any additional fds will be silently closed. If the
* UNIX_FDS_ACCEPT_NONE flag is not raised than at least one fd must be
* received.
*/
#define UNIX_FDS_ACCEPT_MORE ((__u32)(1 << 2)) /* wipe any extra fds */
/*
* Technically 253 is the kernel limit but we want to the struct to be a
* multiple of 8.
* UNIX_FDS_ACCEPT_NONE can be specified with any of the above flags and
* indicates that the caller will accept no file descriptors to be received.
*/
#define KERNEL_SCM_MAX_FD 252
#define UNIX_FDS_ACCEPT_NONE ((__u32)(1 << 3))
/* UNIX_FDS_ACCEPT_MASK is the value of all the above flags or-ed together. */
#define UNIX_FDS_ACCEPT_MASK (UNIX_FDS_ACCEPT_EXACT | UNIX_FDS_ACCEPT_LESS | UNIX_FDS_ACCEPT_MORE | UNIX_FDS_ACCEPT_NONE)
/* Allow the callee to communicate reality. */
/* UNIX_FDS_RECEIVED_EXACT indicates that the exact number of fds was received. */
#define UNIX_FDS_RECEIVED_EXACT ((__u32)(1 << 16))
/*
* UNIX_FDS_RECEIVED_LESS indicates that less than the requested number of fd
* has been received.
*/
#define UNIX_FDS_RECEIVED_LESS ((__u32)(1 << 17))
/*
* UNIX_FDS_RECEIVED_MORE indicates that more than the requested number of fd
* has been received.
*/
#define UNIX_FDS_RECEIVED_MORE ((__u32)(1 << 18))
/* UNIX_FDS_RECEIVED_NONE indicates that no fds have been received. */
#define UNIX_FDS_RECEIVED_NONE ((__u32)(1 << 19))
/**
* Defines a generic struct to receive file descriptors from unix sockets.
* @fd_count_max : Either the exact or maximum number of file descriptors the
* caller is willing to accept. Must be smaller than
* KERNEL_SCM_MAX_FDs; larger values will be rejected.
* Filled in by the caller.
* @fd_count_ret : The actually received number of file descriptors.
* Filled in by the callee.
* @flags : Flags to negotiate expectations about the number of file
* descriptors to receive.
* Filled in by the caller and callee. The caller's flag space
* is UNIX_FDS_ACCEPT_* other values will be rejected. The
* caller may only set one of {EXACT, LESS, MORE}. In addition
* they can raise the NONE flag. Any combination of {EXACT,
* LESS, MORE} will be rejected.
* The callee's flag space is UNIX_FDS_RECEIVED_*. Only ever
* one of those values will be set.
* @fd : Array to store received file descriptors into. Filled by the
* callee on success. If less file descriptors are received
* than requested in @fd_count_max the callee will ensure that
* all additional slots will be set to -EBADF. Nonetheless, the
* caller should only ever use @fd_count_ret to iterate through
* @fd after a successful receive.
*/
struct unix_fds {
__u32 fd_count_max;
__u32 fd_count_ret;
__u32 flags;
__s32 fd[KERNEL_SCM_MAX_FD];
} __attribute__((aligned(8)));
......
......@@ -3,6 +3,7 @@
#ifndef __LXC_COMMANDS_H
#define __LXC_COMMANDS_H
#include <errno.h>
#include <stdio.h>
#include <sys/types.h>
#include <unistd.h>
......@@ -19,6 +20,7 @@
* have specific reasons to keep the file descriptor alive.
*/
#define LXC_CMD_REAP_CLIENT_FD 1
#define LXC_CMD_KEEP_CLIENT_FD 2
typedef enum {
LXC_CMD_GET_TTY_FD = 0,
......@@ -56,6 +58,8 @@ struct lxc_cmd_req {
const void *data;
};
#define ENCODE_INTO_PTR_LEN 0
struct lxc_cmd_rsp {
int ret; /* 0 on success, -errno on failure */
int datalen;
......@@ -67,6 +71,20 @@ struct lxc_cmd_rr {
struct lxc_cmd_rsp rsp;
};
static inline void lxc_cmd_init(struct lxc_cmd_rr *cmd, lxc_cmd_t command)
{
*cmd = (struct lxc_cmd_rr){
.req = {.cmd = command },
.rsp = {.ret = -ENOSYS },
};
}
static inline void lxc_cmd_data(struct lxc_cmd_rr *cmd, int len_data, const void *data)
{
cmd->req.data = data;
cmd->req.datalen = len_data;
}
struct lxc_cmd_tty_rsp_data {
int ptxfd;
int ttynum;
......@@ -85,10 +103,10 @@ __hidden extern int lxc_cmd_get_tty_fd(const char *name, int *ttynum, int *fd,
const char *lxcpath);
/*
* Get the 'real' cgroup path (as seen in /proc/self/cgroup) for a container
* for a particular subsystem
* for a particular controller
*/
__hidden extern char *lxc_cmd_get_cgroup_path(const char *name, const char *lxcpath,
const char *subsystem);
const char *controller);
__hidden extern int lxc_cmd_get_clone_flags(const char *name, const char *lxcpath);
__hidden extern char *lxc_cmd_get_config_item(const char *name, const char *item,
const char *lxcpath);
......@@ -122,7 +140,7 @@ __hidden extern int lxc_cmd_serve_state_clients(const char *name, const char *lx
struct lxc_epoll_descr;
struct lxc_handler;
__hidden extern int lxc_cmd_init(const char *name, const char *lxcpath, const char *suffix);
__hidden extern int lxc_server_init(const char *name, const char *lxcpath, const char *suffix);
__hidden extern int lxc_cmd_mainloop_add(const char *name, struct lxc_epoll_descr *descr,
struct lxc_handler *handler);
__hidden extern int lxc_try_cmd(const char *name, const char *lxcpath);
......@@ -147,7 +165,7 @@ __hidden extern int lxc_cmd_get_cgroup_fd(const char *name, const char *lxcpath,
struct cgroup_fd *ret_fd);
__hidden extern char *lxc_cmd_get_limit_cgroup_path(const char *name,
const char *lxcpath,
const char *subsystem);
const char *controller);
__hidden extern int lxc_cmd_get_limit_cgroup2_fd(const char *name,
const char *lxcpath);
__hidden extern int lxc_cmd_get_limit_cgroup_fd(const char *name,
......
......@@ -508,10 +508,10 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \
__internal_ret__; \
})
#define sysdebug(__ret__, format, ...) \
#define systrace(__ret__, format, ...) \
({ \
typeof(__ret__) __internal_ret__ = (__ret__); \
SYSDEBUG(format, ##__VA_ARGS__); \
SYSTRACE(format, ##__VA_ARGS__); \
__internal_ret__; \
})
......@@ -525,7 +525,7 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \
#define syserrno_set(__ret__, format, ...) \
({ \
typeof(__ret__) __internal_ret__ = (__ret__); \
errno = abs(__ret__); \
errno = labs(__ret__); \
SYSERROR(format, ##__VA_ARGS__); \
__internal_ret__; \
})
......@@ -533,11 +533,39 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \
#define syswarn_set(__ret__, format, ...) \
({ \
typeof(__ret__) __internal_ret__ = (__ret__); \
errno = abs(__ret__); \
errno = labs(__ret__); \
SYSWARN(format, ##__VA_ARGS__); \
__internal_ret__; \
})
#define syserror(format, ...) \
({ \
SYSERROR(format, ##__VA_ARGS__); \
(-errno); \
})
#define syserror_set(__ret__, format, ...) \
({ \
typeof(__ret__) __internal_ret__ = (__ret__); \
errno = labs(__ret__); \
SYSERROR(format, ##__VA_ARGS__); \
__internal_ret__; \
})
#define sysdebug(format, ...) \
({ \
SYSDEBUG(format, ##__VA_ARGS__); \
(-errno); \
})
#define sysdebug_set(__ret__, format, ...) \
({ \
typeof(__ret__) __internal_ret__ = (__ret__); \
errno = labs(__ret__); \
SYSDEBUG(format, ##__VA_ARGS__); \
__internal_ret__; \
})
#define log_error(__ret__, format, ...) \
({ \
typeof(__ret__) __internal_ret__ = (__ret__); \
......
......@@ -708,4 +708,29 @@ enum {
_min1 < _min2 ? _min1 : _min2; \
})
#define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
/*
* Compile time versions of __arch_hweightN()
*/
#define __const_hweight8(w) \
((unsigned int) \
((!!((w) & (1ULL << 0))) + \
(!!((w) & (1ULL << 1))) + \
(!!((w) & (1ULL << 2))) + \
(!!((w) & (1ULL << 3))) + \
(!!((w) & (1ULL << 4))) + \
(!!((w) & (1ULL << 5))) + \
(!!((w) & (1ULL << 6))) + \
(!!((w) & (1ULL << 7)))))
#define __const_hweight16(w) (__const_hweight8(w) + __const_hweight8((w) >> 8 ))
#define __const_hweight32(w) (__const_hweight16(w) + __const_hweight16((w) >> 16))
#define __const_hweight64(w) (__const_hweight32(w) + __const_hweight32((w) >> 32))
#define hweight8(w) __const_hweight8(w)
#define hweight16(w) __const_hweight16(w)
#define hweight32(w) __const_hweight32(w)
#define hweight64(w) __const_hweight64(w)
#endif /* __LXC_MACRO_H */
......@@ -715,7 +715,7 @@ struct lxc_handler *lxc_init_handler(struct lxc_handler *old,
}
if (handler->conf->reboot == REBOOT_NONE) {
handler->conf->maincmd_fd = lxc_cmd_init(name, lxcpath, "command");
handler->conf->maincmd_fd = lxc_server_init(name, lxcpath, "command");
if (handler->conf->maincmd_fd < 0) {
ERROR("Failed to set up command socket");
goto on_error;
......
......@@ -45,17 +45,21 @@ set -e
allocate_pty="nopty"
ATTACH_LOG=$(mktemp --dry-run)
FAIL() {
echo -n "Failed " >&2
echo "$*" >&2
cat "${ATTACH_LOG}"
rm -f "${ATTACH_LOG}" || true
lxc-destroy -n busy -f
exit 1
}
# Create a container, start it and wait for it to be in running state.
lxc-create -t busybox -n busy || FAIL "creating busybox container"
lxc-start -n busy -d || FAIL "starting busybox container"
lxc-wait -n busy -s RUNNING || FAIL "waiting for busybox container to run"
lxc-create -t busybox -n busy -l trace -o "${ATTACH_LOG}" || FAIL "creating busybox container"
lxc-start -n busy -d -l trace -o "${ATTACH_LOG}" || FAIL "starting busybox container"
lxc-wait -n busy -s RUNNING -l trace -o "${ATTACH_LOG}" || FAIL "waiting for busybox container to run"
if [ -t 0 ] && [ -t 1 ] && [ -t 2 ]; then
allocate_pty="pty"
......@@ -68,7 +72,7 @@ fi
# stdout --> attached to pty
# stderr --> attached to pty
for i in `seq 1 100`; do
attach=$(lxc-attach -n busy -- hostname || FAIL "to allocate or setup pty")
attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- hostname || FAIL "to allocate or setup pty")
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- hostname"
fi
......@@ -77,7 +81,7 @@ done
# stdin --> /dev/null
# stdout --> attached to pty
# stderr --> attached to pty
attach=$(lxc-attach -n busy -- hostname < /dev/null || FAIL "to allocate or setup pty")
attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- hostname < /dev/null || FAIL "to allocate or setup pty")
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- hostname < /dev/null"
fi
......@@ -85,7 +89,7 @@ fi
# stdin --> attached to pty
# stdout --> /dev/null
# stderr --> attached to pty
attach=$(lxc-attach -n busy -- hostname > /dev/null || FAIL "to allocate or setup pty")
attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- hostname > /dev/null || FAIL "to allocate or setup pty")
if [ -n "$attach" ]; then
FAIL "lxc-attach -n busy -- hostname > /dev/null"
fi
......@@ -93,7 +97,7 @@ fi
# stdin --> attached to pty
# stdout --> attached to pty
# stderr --> /dev/null
attach=$(lxc-attach -n busy -- hostname 2> /dev/null || FAIL "to allocate or setup pty")
attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- hostname 2> /dev/null || FAIL "to allocate or setup pty")
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- hostname 2> /dev/null < /dev/null"
fi
......@@ -101,7 +105,7 @@ fi
# stdin --> /dev/null
# stdout --> attached to pty
# stderr --> /dev/null
attach=$(lxc-attach -n busy -- hostname 2> /dev/null < /dev/null || FAIL "to allocate or setup pty")
attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- hostname 2> /dev/null < /dev/null || FAIL "to allocate or setup pty")
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- hostname 2> /dev/null < /dev/null"
fi
......@@ -114,7 +118,7 @@ fi
# stdin --> attached to pty
# stdout --> /dev/null
# stderr --> attached to pty
attach=$( ( lxc-attach -n busy -- sh -c 'hostname >&2' > /dev/null ) 2>&1 || FAIL "to allocate or setup pty")
attach=$( ( lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- sh -c 'hostname >&2' > /dev/null ) 2>&1 || FAIL "to allocate or setup pty")
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- sh -c 'hostname >&2' > /dev/null"
fi
......@@ -126,7 +130,7 @@ fi
# stdin --> attached to pty
# stdout --> attach to pty
# stderr --> /dev/null
attach=$( ( lxc-attach -n busy -- sh -c 'hostname >&2' 2> /dev/null ) 2>&1 || FAIL "to allocate or setup pty")
attach=$( ( lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- sh -c 'hostname >&2' 2> /dev/null ) 2>&1 || FAIL "to allocate or setup pty")
if [ -n "$attach" ]; then
FAIL "lxc-attach -n busy -- sh -c 'hostname >&2' 2> /dev/null"
fi
......@@ -136,7 +140,7 @@ fi
# stdout --> /dev/null
# stderr --> attached to pty
# (As we expect the exit code of the command to be 1 we ignore it.)
attach=$(lxc-attach -n busy -- sh -c 'rm 2>&1' > /dev/null || true)
attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- sh -c 'rm 2>&1' > /dev/null || true)
if [ -n "$attach" ]; then
FAIL "lxc-attach -n busy -- sh -c 'rm 2>&1' > /dev/null"
fi
......@@ -146,7 +150,7 @@ fi
# - stdout --> attached to pty
# - stderr --> /dev/null
# (As we expect the exit code of the command to be 1 we ignore it.)
attach=$(lxc-attach -n busy -- sh -c 'rm 2>&1' 2> /dev/null || true)
attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- sh -c 'rm 2>&1' 2> /dev/null || true)
if [ -z "$attach" ]; then
FAIL "lxc-attach -n busy -- sh -c 'rm 2>&1' 2> /dev/null"
fi
......@@ -154,7 +158,7 @@ fi
# stdin --> $in
# stdout --> attached to pty
# stderr --> attached to pty
attach=$(echo hostname | lxc-attach -n busy -- || FAIL "to allocate or setup pty")
attach=$(echo hostname | lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- || FAIL "to allocate or setup pty")
if [ "$attach" != "busy" ]; then
FAIL "echo hostname | lxc-attach -n busy --"
fi
......@@ -165,7 +169,7 @@ fi
out=$(mktemp /tmp/out_XXXX)
err=$(mktemp /tmp/err_XXXX)
trap "rm -f $out $err" EXIT INT QUIT PIPE
lxc-attach -n busy -- sh -c 'echo OUT; echo ERR >&2' > $out 2> $err || FAIL "to allocate or setup pty"
lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- sh -c 'echo OUT; echo ERR >&2' > $out 2> $err || FAIL "to allocate or setup pty"
outcontent=$(cat $out)
errcontent=$(cat $err)
if [ "$outcontent" != "OUT" ] || [ "$errcontent" != "ERR" ]; then
......@@ -181,7 +185,7 @@ rm -f $out $err
out=$(mktemp /tmp/out_XXXX)
err=$(mktemp /tmp/err_XXXX)
trap "rm -f $out $err" EXIT INT QUIT PIPE
echo "hostname; rm" | lxc-attach -n busy > $out 2> $err || true
echo "hostname; rm" | lxc-attach -n busy -l trace -o "${ATTACH_LOG}" > $out 2> $err || true
outcontent=$(cat $out)
errcontent=$(cat $err)
if [ "$outcontent" != "busy" ] || [ -z "$errcontent" ]; then
......@@ -191,5 +195,6 @@ fi
rm -f $out $err
lxc-destroy -n busy -f
rm -f "${ATTACH_LOG}" || true
exit 0
......@@ -87,6 +87,7 @@ cleanup() {
echo "FAIL"
exit 1
fi
rm -f "${UNPRIV_LOG}" || true
echo "PASS"
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment