Currently the failure to save content of a buffer object is only logged,
while the checkpointing will continue unaware due the error code being
"eaten" inside sdma_copy_bo() cleanup path, together with the caller not
checking the return value to begin with.
Fix it by preserving and returning the original error code inside
sdma_copy_bo(), and abort the checkpointing process in case of a failure.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Currently failure to open files logs an unitialized stack string as the
failed path. Lets just log the basename and drop the unused stack buffer.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Size needs to be initialized before opening the file for write, otherwise
garbage is written into the file.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
The new_minor field in struct vma_metadata is effectively unused.
Remove it.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Remove unused current_pid global variable.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Instead of logging a zero failure code lets log the actual GPU id which
was not found.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Check that the image unpacking worked before deferencing the returned
pointer during plugin init in order to fail gracefully.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Allocate shared memory for the whole struct shared_handle_ids and not just
for the pointer to it, and by doing so stop relying on mmap rounding up to
page size.
While at it add error checking that the allocations actually succeeded.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Adjusted object sizes need to be used to unmap on the cleanup path so that
the unmap operations match with map for the case sizes were adjusted in
the function prologue.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
---
v2:
* Improve commit message and use adjusted size in error logs too.
The function was not closing the drm fd duplicated by the libdrm call,
could attempt to deinitialize the libdrm device which wasn't successfully
openened, and was using errno based log functions when libdrm functions
used return a negative errno. Fix it all.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Close the file when either memory allocation or a file read fails in
amdgpu_plugin_dmabuf_restore.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Docstring refers to size_t as 8-byte value while in reality this is
architecture dependent. Clarify it.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
In open_path(), when inherit_fd_lookup_id() returns negative for an
external file, execution silently falls through to the normal open
path causing an incorrect size mismatch error. Added a pr_warn() to
make the failure explicit.
Fixes: #2951
Signed-off-by: Cameron Badman <cbadwork@gmail.com>
obj->frozen is uint32_t. scanf() must use the matching SCNu32 macro for
this destination type.
Use SCNu32 in parse_bpfmap_fdinfo() for the frozen field parsing.
Signed-off-by: Farzan Aman Khan <farzanaman99@gmail.com>
The logic in sb_opt_cb converts uid and gid to using userns map, it
means that the option string can extend. Worse case we had original id
"0" and got INVALID_ID ("4294967295") back.
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Build sysctl payloads with snprintf() and check for error/truncation before writing.
Use the returned payload length for write() instead of fixed-size writes, so only formatted bytes are written.
On snprintf failure/truncation, print an explicit error message and return -1. On write/open helper failures, return -1 because callers only use success/failure and pr_perror() already logs errno.
Signed-off-by: Farzan Aman Khan <farzanaman99@gmail.com>
rand_ipc_sysctl() and rand_ipc_sem() returned on write() failure
without closing the opened sysctl fd first.
Close fd in both error paths before returning.
Also return a generic failure code (-1) in these helper paths instead of
propagating errno after close(), because close() may modify errno and
callers only use the return value as success/failure.
Signed-off-by: Farzan Aman Khan <farzanaman99@gmail.com>
check_map_is_there() parses %x:%x major/minor fields from /proc/self/maps.
Store these values in unsigned int variables to match the "%x" specifier
and keep the parser warning-free.
Keep the end-address fragment as %*x to avoid gnu_scanf
"assignment suppression + length modifier" -Werror failures.
Signed-off-by: Farzan Aman Khan <farzanaman99@gmail.com>
shared->opt_files and loop index i are size_t values. Printing them with
"%lu"/"%d" is type-mismatched and can produce incorrect output on some
platforms.
Switch these sites to "%zu" in logging and path formatting.
Signed-off-by: Farzan Aman Khan <farzanaman99@gmail.com>
sb_opt_cb() parses gid=/uid= option values into unsigned id values and
passes mapped userns_*() results to sprintf(). Using "%d" for these
unsigned values is a format/type mismatch.
Switch gid=/uid= parsing and printing to "%u".
In parse_fdinfo_pid_s(), EventpollTfdEntry.tfd is uint32_t, so parse it
with SCNu32 instead of "%d" to match the destination type.
Signed-off-by: Farzan Aman Khan <farzanaman99@gmail.com>
When bfd is unbuffered, bwritev() used writev() directly, which can return
partial writes and leave checkpoint images incomplete.
Fix this by retrying writev() until all iovecs are written or an error occurs.
Temporarily modify the first iovec's base and length to track progress on
partial writes, then restore it before the next iteration.
Changes the function signature to accept non-const iovec, as we need to
modify it to track progress (same pattern as write_all() with buffers).
Signed-off-by: Ahmed Elaidy <elaidya225@gmail.com>
Currently, bfd has a fixed buffer size (BUFSIZE, which is 4096). This
causes issues when reading lines longer than BUFSIZE, as breadline
fails with "The bfd buffer is too small".
This patch introduces dynamic buffer resizing in bfd. When a buffer
is full and more space is needed (e.g., for a very long line), the
buffer is resized using mremap (or a new mmap if it was using a
pre-allocated buffer from the pool).
A new bsize field is added to struct xbuf to keep track of the
current buffer size.
Unit tests for reading long lines and writing large buffers are
added to criu/unittest/unit.c.
Signed-off-by: dong sunchao <dongsunchao@gmail.com>
Co-developed-by: Andrei Vagin <avagin@google.com>
Signed-off-by: Andrei Vagin <avagin@google.com>
In amdgpu_plugin_drm_restore_file(), dmabuf_fd can reach the
drmPrimeFDToHandle() call as -1 if fdstore_get() fails or if
boinfo->handle is -1 while boinfo->is_import is true. Passing
an invalid fd to libdrm causes a hard restore failure instead
of a retry. Check for -1 and set retry_needed so the restore
coordinator can retry later.
When retry_needed is set, some dmabufs entries are still invalid,
so skip record_completed_work() and restore_bo_contents_drm()
to avoid operating on incomplete state.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Several error paths in amdgpu_plugin_drm_dump_file() leak
resources. The drmPrimeHandleToFD() and open_img_file() failures
use break instead of goto exit, which skips per-iteration cleanup
and lets the function continue dumping metadata with a stale ret
value. Replace break with proper resource cleanup and goto exit.
The vm_info_entries buffer is not freed when the ioctl or
allocate_vm_entries() calls fail after allocating it, and the
buffer from posix_memalign() is never freed after sdma_copy_bo(),
leaking memory on every loop iteration. Add the missing xfree()
calls.
Check the amdgpu_device_initialize() return value because h_dev
is undefined on failure. Move list_handles_entries cleanup into
the exit block because error paths that goto exit skip the
existing xfree() call after the loop. Add a NULL check before
free_e(rd) because rd allocation can fail before the struct is
initialized.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Initialize gpu_ids, fds_write, and entries to NULL before
allocating them in init_parallel_restore_cmd() and
init_parallel_restore_cmd_by_head(). The caller declares the
struct on the stack without an initializer, so these members
hold undefined values. If a later xzalloc() fails and the
caller invokes free_parallel_restore_cmd(), it will call
xfree() on those undefined pointers.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Call amdgpu_device_deinitialize() before continuing when
get_gem_handle() fails and before returning when a matching
handle is found. Both paths skip the existing deinitialize
call at the end of the loop body, leaking the device handle.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
open_img_file() can return NULL after a read_fp() or write_fp()
failure without setting errno. The openat() and fdopen() paths
set errno, but the read/write path does not, so callers using
-errno get zero or a stale value. Set errno to EIO on this path
so errno is always valid when the function returns NULL.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Close the file descriptor when fdopen() fails, as fdopen does not
take ownership of the fd on failure.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
The exit label in amdgpu_plugin_drm_restore_file() returns
immediately when ret < 0 without freeing the dmabufs array or
deinitializing the amdgpu device handle. Move cleanup before the
error check so resources are always released.
Replace xzalloc() with xmalloc() and memset(0xff) for the dmabufs
array because zero is a valid fd number and would cause the cleanup
loop to close fd 0 (stdin) for unpopulated entries. Initializing
to KFD_INVALID_FD (0xffffffff) lets the exit block tell which fds
were actually opened. Guard amdgpu_device_deinitialize() behind a
dev_initialized flag to avoid calling it when initialization failed,
and close dmabuf_fd when drmPrimeFDToHandle() fails. To prevent
double-closing fds that restore_bo_contents_drm() already closed,
only run the close loop when the BO content restore was not reached.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Free the protobuf message and buffer when returning early from
amdgpu_plugin_dmabuf_restore() on failure to find a dmabuf fd.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
posix_memalign() returns an error code on failure and does not
guarantee setting the pointer to NULL. Check the return value
instead of the pointer. Since posix_memalign() does not set
errno, assign it explicitly so pr_perror() prints the correct
message. Negate the return code instead of hardcoding -ENOMEM
to preserve the actual error (e.g. EINVAL for bad alignment).
In amdgpu_plugin_drm_dump_file(), replace break with cleanup
and goto exit so per-BO resources are freed on failure.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
When parse_threads() is called with a pre-allocated array (e.g., when
re-parsing threads), it must ensure that the number of threads found in
/proc does not exceed the size of the provided buffer. Add a check to
prevent out-of-bounds access if the thread count increases unexpectedly.
Signed-off-by: Andrei Vagin <avagin@google.com>
The exit status in the wait status word is 8 bits. Using a 0x7f mask was
incorrectly stripping the highest bit of the exit code. Use 0xff mask
instead to correctly preserve the full 8-bit exit status.
Signed-off-by: Andrei Vagin <avagin@google.com>
When writing numeric values to /proc files (like oom_score_adj, loginuid,
and pid_max), we were using hardcoded lengths or full buffer sizes in
write() calls. This could result in writing garbage characters from the
stack if the actual string was shorter than the specified length.
Use the length returned by snprintf() to ensure only the intended string
is written. Also, increase the buffer size for oom_score_adj to avoid
truncation.
Signed-off-by: Andrei Vagin <avagin@google.com>
mmap() returns MAP_FAILED ((void *)-1) on failure, not NULL. Checking
for NULL is incorrect and would miss actual failures, leading to
potential null pointer dereferences later.
Signed-off-by: Andrei Vagin <avagin@google.com>
siginfo_priv_nr was allocated in prepare_siginfo_priv() but never
freed. Add an xfree() call in sigreturn_restore() to clean it up.
Also, remove an incorrect attempt to free thread core entries inside the
main thread loop in sigreturn_restore(), as these are now properly
handled at the end of the function.
Signed-off-by: Andrei Vagin <avagin@google.com>
In open_cores(), we were not freeing already opened core entries if one
of them failed to open. Also, the cores array was not zero-initialized,
which could lead to freeing garbage pointers in the error path. Use
xzalloc() to ensure the array is zeroed.
In sigreturn_restore(), thread core entries were not being freed after
use, leading to a memory leak. Add a loop to free them.
Signed-off-by: Andrei Vagin <avagin@google.com>
The aarch64 Fedora Rawhide test has been moved to GitHub Actions
as part of the fedora-rawhide-test matrix build. Remove it from
the Cirrus CI configuration.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Adrian Reber <areber@redhat.com>
Convert the fedora-rawhide-test job to a matrix build that runs
on both ubuntu-22.04 (x86_64) and ubuntu-24.04-arm (aarch64).
This replaces the aarch64 Fedora Rawhide build-only test
previously running on Cirrus CI with a full test run on GitHub
Actions.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Adrian Reber <areber@redhat.com>
Remove leftover pidfile and criu_service.socket at the start of
the run target. If these files exist from a previous interrupted
run, the service fails to start and read.py hangs forever waiting
on the status fifo.
Acked-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Verify that the page server binds to the address given in
the RPC request when lazy_pages is enabled. Without the fix
in the previous commit, the address was silently ignored and
the page server would bind to 0.0.0.0 instead.
The test sends a PAGE_SERVER request with ps.address set to
127.0.0.1 and lazy_pages enabled, then checks /proc/net/tcp
to confirm the page server actually bound to that address.
Acked-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
The page server address from RPC requests was ignored when
lazy_pages was enabled. The code that copies the address into
opts.addr only ran inside the !opts.lazy_pages branch, so with
lazy pages the page server would bind to 0.0.0.0 instead of
the specified address.
This breaks live migration when the container runtime needs the
page server to listen on a specific address. The zeropod project
worked around this with a downstream patch that adds a raw
pointer assignment (opts.addr = req->ps->address), but that
causes a use-after-free when the protobuf message is freed.
Fix this by moving SET_CHAR_OPTS (which copies via strdup) and
the NULL fallback outside the !opts.lazy_pages block so they
apply unconditionally.
Reproducer (run as root, requires pycriu):
sleep 99999 &
criu dump -t $! -D /tmp/imgs --shell-job -v0
criu service --address /tmp/criu.sock -v0 &
python3 -c "
import os, socket
from pycriu import rpc_pb2 as rpc
s = socket.socket(socket.AF_UNIX, socket.SOCK_SEQPACKET)
s.connect('/tmp/criu.sock')
req = rpc.criu_req()
req.type = rpc.PAGE_SERVER
req.opts.images_dir_fd = os.open('/tmp/imgs', os.O_DIRECTORY)
req.opts.lazy_pages = True
req.opts.ps.port = 27
req.opts.ps.address = '127.0.0.1'
s.send(req.SerializeToString())
"
ss -tlnp sport = :27 # Bug: shows 0.0.0.0:27 instead of 127.0.0.1:27
Acked-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Add a test that verifies pipe ownership (uid/gid) is preserved across
checkpoint/restore. This test exercises the fix for issue #2984 where
accessing pipes via /proc/self/fd/ failed after restore because the
pipe ownership was changed to root.
The test:
- Creates a pipe and optionally changes ownership (if running as root)
- Records ownership before C/R
- Verifies ownership is preserved after restore
- Tests /proc/self/fd access works after restore
Signed-off-by: Ahmed Elaidy <elaidya225@gmail.com>
When CRIU runs as root and restores a pipe, the pipe inode is created
with root ownership. If the restored process runs as a non-root user
and tries to reopen the pipe via /proc/self/fd/<n>, the open() fails
with EACCES because the pipe inode is owned by root.
Fix this by saving the pipe's uid/gid during dump (from stat) and
restoring ownership with fchown() after creating the pipe. This
matches how memfds and TTYs already handle ownership restoration.
Fixes: #2984
Signed-off-by: Ahmed Elaidy <elaidya225@gmail.com>
Cirrus CI reports "Failed to start an instance:
FAILED_PRECONDITION: Monthly compute limit exceeded!" making the
Vagrant Fedora Rawhide test unusable.
Replace the Cirrus CI Vagrant-based Fedora Rawhide test with a
Lima VM-based equivalent in GitHub Actions. This follows the same
pattern used by the runc project (lima-vm/lima-actions).
The new vagrant-fedora-rawhide-test job:
- Starts a Lima Fedora VM with KVM acceleration
- Installs the latest vanilla kernel
- Reboots the VM to activate the new kernel
- Runs the fedora-rawhide CI target inside a podman container
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Adrian Reber <areber@redhat.com>
The --pids-limit=0 value does not disable the pids limit in
current podman versions. The podman documentation specifies
that -1 means unlimited. With --pids-limit=0, the thread-bomb
test fails on the second run (after restore) because each of
the 1024 threads tries to create a new thread, exceeding the
container's pids limit and causing pthread_create() to return
EAGAIN.
Change --pids-limit=0 to --pids-limit=-1 to properly disable
the pids cgroup limit.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Adrian Reber <areber@redhat.com>
struct parasite_ctl, parasite_thread_ctl, and plain_regs_struct
all contain user_fpregs_struct_t (typedef for struct xsave_struct),
which is declared with __aligned(64). When these structs are
allocated on the heap with xmalloc/xzalloc (i.e. malloc/calloc),
the allocator only guarantees 16-byte alignment on x86_64.
The compiler, seeing the __aligned(64) attribute on the struct
type, may emit aligned memory instructions (e.g. movaps, vmovdqa)
for struct copies, assuming the memory is properly aligned. When
the heap pointer is not 64-byte aligned, these instructions trigger
a General Protection Fault (#GP).
This was observed as a crash in save_regs_plain() at infect.c:1314
(prs->regs = *r) on CentOS Stream 10 under QEMU/KVM, where the
compiler generated aligned vector instructions for the struct copy.
The crash did not reproduce on bare metal with a different compiler
version that happened to emit unaligned instructions.
Add xmemalign() wrapper around posix_memalign() to xmalloc.h and
use it for all three allocation sites:
- compel_prepare_noctx(): struct parasite_ctl
- compel_prepare_thread(): struct parasite_thread_ctl
- compel_prepare(): struct plain_regs_struct
Memory from posix_memalign() can be freed with free(), so no
changes to cleanup paths are needed.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Adrian Reber <areber@redhat.com>
