Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ jobs:
libdbus-1-dev \
libpam0g-dev \
libseccomp-dev \
libselinux1-dev
libselinux1-dev \
liburing-dev

- name: Checkout the packaging branch
run: |
Expand Down
13 changes: 11 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ jobs:
- ubuntu-24.04
- ubuntu-22.04-arm
- ubuntu-24.04-arm
async-api:
- epoll
- io_uring
variant:
- default
- sanitizer
Expand All @@ -54,6 +57,10 @@ jobs:
os: ubuntu-22.04-arm
- variant: sanitizer
os: ubuntu-24.04-arm
- async-api: io_uring
os: ubuntu-22.04-arm
- async-api: io_uring
os: ubuntu-22.04
runs-on: ${{ matrix.os }}
steps:
- name: Checkout code
Expand All @@ -75,7 +82,8 @@ jobs:
libdbus-1-dev \
libpam0g-dev \
libseccomp-dev \
libselinux1-dev
libselinux1-dev \
liburing-dev

- name: Compiler version
env:
Expand All @@ -97,6 +105,7 @@ jobs:
if [ "${{ matrix.variant }}" = "default" ]; then
meson setup build \
-Dprefix=/usr \
-Dio-uring-event-loop=${{ matrix.async-api == 'io_uring' }} \
-Dtests=true \
-Dpam-cgroup=true \
-Dtools-multicall=true \
Expand All @@ -106,12 +115,12 @@ jobs:
elif [ "${{ matrix.variant }}" = "sanitizer" ]; then
meson setup build \
-Dprefix=/usr \
-Dio-uring-event-loop=${{ matrix.async-api == 'io_uring' }} \
-Dtests=true \
-Dpam-cgroup=true \
-Dtools-multicall=true \
-Dwerror=true \
-Db_lto_mode=default \
-Dio-uring-event-loop=false \
-Db_lundef=false \
-Db_sanitize=address,undefined
fi
Expand Down
86 changes: 86 additions & 0 deletions src/lxc/file_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <time.h>
#include <poll.h>

#include "file_utils.h"
#include "macro.h"
Expand Down Expand Up @@ -147,6 +148,91 @@ ssize_t lxc_read_try_buf_at(int dfd, const char *path, void *buf, size_t count)
return ret;
}

static int __lxc_wait_for_io_ready(int fd, int event, int timeout_ms)
{
int ret;
struct pollfd pfd = {
.fd = fd,
.events = event,
.revents = 0
};

do {
ret = poll(&pfd, 1, timeout_ms);
} while (ret < 0 && errno == EINTR);

if (ret < 0)
return -errno;

if (ret == 0)
return -ETIMEDOUT;

if (pfd.revents & (POLLERR | POLLHUP | POLLNVAL)) {
if (pfd.revents & POLLERR)
return -EPIPE;
if (pfd.revents & POLLHUP)
return -EPIPE;
if (pfd.revents & POLLNVAL)
return -EBADF;
}

if (!(pfd.revents & event))
return -EAGAIN;

return ret;
}

ssize_t lxc_write_all(int fd, const void *buf, size_t count)
{
ssize_t left_to_write = count;
const char *ptr = buf;
int flags;

flags = fcntl(fd, F_GETFL);
if (flags < 0)
return ret_set_errno(-1, errno);

/* only non-blocking fds are allowed */
if (!(flags & O_NONBLOCK))
return ret_set_errno(-1, EINVAL);

while (left_to_write > 0) {
int ret = write(fd, ptr, left_to_write);

if (ret > 0) {
left_to_write -= ret;
ptr += ret;
continue;
}

if (ret == 0)
break;

/* ret < 0 */
if (errno == EINTR)
continue;

if (errno == EAGAIN) {
int pret = __lxc_wait_for_io_ready(fd, POLLOUT, 5000);

/* we've got an event on fd */
if (pret > 0)
continue;

if (pret == -ETIMEDOUT)
break;

if (pret < 0)
return ret_set_errno(-1, -pret);
}

/* some other error */
return ret_set_errno(-1, errno);
}

return count - left_to_write;
}

ssize_t lxc_write_nointr(int fd, const void *buf, size_t count)
{
ssize_t ret;
Expand Down
2 changes: 2 additions & 0 deletions src/lxc/file_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ __hidden extern int lxc_read_from_file(const char *filename, void *buf, size_t c
__access_w(2, 3);

/* send and receive buffers completely */
__hidden extern ssize_t lxc_write_all(int fd, const void *buf, size_t count) __access_r(2, 3);

__hidden extern ssize_t lxc_write_nointr(int fd, const void *buf, size_t count) __access_r(2, 3);

__hidden extern ssize_t lxc_pwrite_nointr(int fd, const void *buf, size_t count, off_t offset)
Expand Down
4 changes: 2 additions & 2 deletions src/lxc/terminal.c
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ static int lxc_terminal_ptx_io(struct lxc_terminal *terminal)
w_rbuf = w_log = 0;
/* write to peer first */
if (terminal->peer >= 0)
w = lxc_write_nointr(terminal->peer, buf, r);
w = lxc_write_all(terminal->peer, buf, r);

/* write to terminal ringbuffer */
if (terminal->buffer_size > 0)
Expand Down Expand Up @@ -375,7 +375,7 @@ static int lxc_terminal_peer_io(struct lxc_terminal *terminal)
return -1;
}

w = lxc_write_nointr(terminal->ptx, buf, r);
w = lxc_write_all(terminal->ptx, buf, r);
if (w != r)
WARN("Short write on terminal r:%d != w:%d", r, w);

Expand Down
65 changes: 61 additions & 4 deletions src/tests/lxc-test-lxc-attach
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ for i in $(seq 1 100); do
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- hostname"
fi
rm -f "${ATTACH_LOG}"
done

# stdin --> /dev/null
Expand All @@ -67,6 +68,7 @@ attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- hostname < /dev/null
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- hostname < /dev/null"
fi
rm -f "${ATTACH_LOG}"

# stdin --> attached to pty
# stdout --> /dev/null
Expand All @@ -75,6 +77,7 @@ attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- hostname > /dev/null
if [ -n "$attach" ]; then
FAIL "lxc-attach -n busy -- hostname > /dev/null"
fi
rm -f "${ATTACH_LOG}"

# stdin --> attached to pty
# stdout --> attached to pty
Expand All @@ -83,6 +86,7 @@ attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- hostname 2> /dev/null
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- hostname 2> /dev/null < /dev/null"
fi
rm -f "${ATTACH_LOG}"

# stdin --> /dev/null
# stdout --> attached to pty
Expand All @@ -91,6 +95,7 @@ attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- hostname 2> /dev/null
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- hostname 2> /dev/null < /dev/null"
fi
rm -f "${ATTACH_LOG}"

# Use a synthetic reproducer in container to produce output on stderr. stdout on
# the host gets redirect to /dev/null. We should still be able to receive
Expand All @@ -104,6 +109,7 @@ attach=$( ( lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- sh -c 'hostname >&
if [ "$attach" != "busy" ]; then
FAIL "lxc-attach -n busy -- sh -c 'hostname >&2' > /dev/null"
fi
rm -f "${ATTACH_LOG}"

# Use a synthetic reproducer in container to produce output on stderr. stderr on
# the host gets redirect to /dev/null. We should not receive output on stderr on
Expand All @@ -116,7 +122,7 @@ attach=$( ( lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- sh -c 'hostname >&
if [ -n "$attach" ]; then
FAIL "lxc-attach -n busy -- sh -c 'hostname >&2' 2> /dev/null"
fi

rm -f "${ATTACH_LOG}"

# stdin --> attached to pty
# stdout --> /dev/null
Expand All @@ -126,7 +132,7 @@ attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- sh -c 'rm 2>&1' > /de
if [ -n "$attach" ]; then
FAIL "lxc-attach -n busy -- sh -c 'rm 2>&1' > /dev/null"
fi

rm -f "${ATTACH_LOG}"

# - stdin --> attached to pty
# - stdout --> attached to pty
Expand All @@ -136,6 +142,7 @@ attach=$(lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- sh -c 'rm 2>&1' 2> /d
if [ -z "$attach" ]; then
FAIL "lxc-attach -n busy -- sh -c 'rm 2>&1' 2> /dev/null"
fi
rm -f "${ATTACH_LOG}"

# stdin --> $in
# stdout --> attached to pty
Expand All @@ -144,6 +151,7 @@ attach=$(echo hostname | lxc-attach -n busy -l trace -o "${ATTACH_LOG}" -- || FA
if [ "$attach" != "busy" ]; then
FAIL "echo hostname | lxc-attach -n busy --"
fi
rm -f "${ATTACH_LOG}"

# stdin --> attached to pty
# stdout --> $out
Expand All @@ -158,7 +166,7 @@ if [ "$outcontent" != "OUT" ] || [ "$errcontent" != "ERR" ]; then
FAIL "lxc-attach -n busy -- sh -c 'echo OUT; echo ERR >&2' > $out 2> $err"
fi

rm -f $out $err
rm -f $out $err "${ATTACH_LOG}"

# stdin --> $in
# stdout --> $out
Expand All @@ -174,8 +182,57 @@ if [ "$outcontent" != "busy" ] || [ -z "$errcontent" ]; then
FAIL "echo 'hostname; rm' | lxc-attach -n busy > $out 2> $err"
fi

rm -f $out $err
rm -f $out $err "${ATTACH_LOG}"

#
# This testcase covers cases like:
# https://github.com/lxc/lxc/issues/4546
# https://discuss.linuxcontainers.org/t/lxc-attach-long-output-stops-suddenly-possible-bug/22031
# https://discuss.linuxcontainers.org/t/fixing-forgejo-runners-lxc-logging/25918
#
# Idea is simple, we simulate a heavy IO and write relatively large amount of data to overfill
# pts device buffers, then ensure data integrity.
#
# We need to use "script" tool to allocate TTYs properly, otherwise we don't go into a
# problematic LXC code-path we want to cover.
#
# Also, I had to introduce two synthetic sleeps: one before issuing commands to busybox shell inside
# a container and another one after.
#
# First one is needed, because LXC looses some pieces of terminal device input during
# lxc-attach command initialization because of tcsetattr(fd, TCSAFLUSH, &newtios) call
# (see https://github.com/lxc/lxc/blob/5d9839bc1316fa185d8c29b90982684b32e3dfa7/src/lxc/terminal.c#L523)
# I would replace TCSAFLUSH with TCSANOW to avoid TTY buffer flush (and I tested that it helps),
# but taking into account that this code is here since 2010
# (see https://github.com/lxc/lxc/commit/e0dc0de76ed1ad9e284a37bd01268227d4eae8c9)
# I decided to keep it like it is for now (FIXME?).
#
# Second sleep is needed because of a bug in busybox, unfortunately, without this sleep,
# busybox fails to react on the host pipe write-end closure (after full command submission)
# and continues to poll infinitely. This sleep makes pipe closure even to be separated from
# a heavy IO and avoids this bug.
#

# Check test dependencies
command -v script >/dev/null 2>&1 || { echo "'script' command is missing" >&2; exit 1; }
busybox dd --help >/dev/null 2>&1 || FAIL "missing busybox's dd applet"
busybox hexdump --help >/dev/null 2>&1 || FAIL "missing busybox's hexdump applet"
busybox tee --help >/dev/null 2>&1 || FAIL "missing busybox's tee applet"

out=$(mktemp /tmp/out_XXXX)
BS=1000000
( sleep 3; echo "echo DATASTART ; dd if=/dev/urandom bs=$BS count=1 status=none | hexdump | tee /root/large-data.txt ; echo DATAEND" ; sleep 1 ) | \
script -q -e -c "lxc-attach -n busy -l trace -o \"${ATTACH_LOG}\"" | \
sed -n '/DATASTART/,/DATAEND/{/DATASTART/d;/DATAEND/d;s/[\r\n]*$//;p}' > $out

[ $(stat -c%s $out) -gt $BS ] || FAIL "generated file size is too small"
cmp -s /var/lib/lxc/busy/rootfs/root/large-data.txt $out || FAIL "data corruption detected"

md5sum /var/lib/lxc/busy/rootfs/root/large-data.txt $out
ls -lah /var/lib/lxc/busy/rootfs/root/large-data.txt
rm -f /var/lib/lxc/busy/rootfs/root/large-data.txt $out "${ATTACH_LOG}"

# Cleanup stage
lxc-destroy -n busy -f
rm -f "${ATTACH_LOG}" || true

Expand Down