Mostly after calling libdrm functions pr_perror is wrong because calling
convention there is to return zero or negative error code - errno is not
guaranteed to be valid. So convert those to pr_err with manually calling
strerror to log user friendly messages. Same conversion for handling
errors from some plugin internal functions.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com> # v1
Cc: Radostin Stoyanov <rstoyanov@fedoraproject.org>
There is no need to initialize and de-initialize libdrm only to use the
duplicated fd for calls to drmPrimeFDToHandle(). From the kernel's
perspective it is exactly the same file so operation is identical to just
using the passed in file descriptor.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Consolidate KFD and DRM VMA handling path to the same (existing) helper.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Currently the code contains multiple places where callers expect the errno
to be valid (implied by calling pr_perror to log file IO errors) after
calling open_img_file(), read_fp() and write_fp() helpers. Problem there
is that those helpers can destroy the errno courtesy of themselves
emitting log messages via fprintf.
Furthermore, the callers then sometimes invent the errors to return back
to the caller, or even outside of the plugin.
On top of that there is no benefit to buffered file IO given the plugin
reads and writes in buffer object size chunks, so it it preferrable to
just go direct and avoid any possibility of extra copying to and from libc
temporary buffers.
So lets just convert it all in one swoop to POSIX IO and make sure correct
errnos are always propagated to the caller (including to CRIU core).
Hopefully this removes all instances of incorrect pr_perror log messages.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Use the recently added image load helper in one more function which
contains the exactly same flow of open-alloc-read.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Given that dump and restore of KFD vs DRM forks into completely parallel
flows inside the respective plugin callbacks, lets split it out by
extracting into helpers. This de-clutters the code flow and makes it
obvious which part is done where.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Currently the plugin is added to the inventory only if there were amdgpu
mapped vmas in the target process. This makes restoring a simple process
with only an open DRM fd and some buffer objects not work.
Fix it by adding the plugin to the inventory when there were open DRM
files as well.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Check SDMA operation type once at the beginning of the copy helper and fix
one incorrect usage of pr_perror while at it.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Open_drm_render_device() is a little bit confused. It can log confusing
errors using errno which hasn't been set, or it has been overwritten. It
can also inspect errno after it had reset it creating effectively
unreachable code. And finally it sometimes returns the negative errno, and
sometimes a plain -1. Fix it all up and make it consistent, while at the
same time correcting two calls site incorrectly using pr_perror(), while
the function does not guarantee a valid errno.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Error logging for just a single error condition will eat the errno so lets
just remove it. All callers use pr_perror() and rely on the helper not
munching it up so no information is lost by removing the internal log.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
We remove some function entry and exit markers, some interim messages
which provide no real value, and only log virtual address allocation
failures on the failure path. For the last one we also fix incorrect
usage of pr_perror.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
The extra debug build log helper does not seem very useful and some of
them do not even compile. Lets just remove it and replace with the
standard pr_debug. In case of too much noise, we can later re-evaluate
to remove some of the not very useful log messages.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Instead of open coding the same ioctl twice we can put it in a loop from
which we break out once we have allocated enough space for all objects.
While at it we add error handling for the memory allocation.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Extra safety so the calls do not stick out as the only ones with no
error checking.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Shallower indentation makes a heavily indented function hopefully a bit
more readable.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Consolidate to one fclose() call with the extra bonus the inconsequential
file descriptor leak is eliminated.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Close the directory when probing images during restore init fails.
While at it, consolidate the identical code between the DRM and KFD images
into a common helper.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
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>
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>
