Copilot reports a possible false positive error on strtoul conversion due
errno not being reset on success. Fix it by clearing errnor prior to the
call.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Copilot reports %d is correct for boolean and %zx for size_t, fix it.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Fixes: 1961c5e0de ("plugins/amdgpu: Cleanup env variable handling")
A bunch of globals defined at the top of amdgpu_plugin.c only needs local
scope so make it so.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
It is a bit tidier to make the helpers return the value instead of passing
in the pointer, so lets make them take the default and return the overall
result, allowing the call sites to consolidate into a single line.
While at it we make the helpers static since they are not used outside the
file and demote the parsing errors to warning log messages since the
plugin does continue operating.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
In restorer_get_vma_hint, if a VMA from self_vma_list or tgt_vma_list
overlaps with the current candidate region (prev_vma_end + vma_len), we
advance prev_vma_end and move to the next VMA in the list. However, we
must then restart the check from the beginning of the while loop to
ensure that the updated candidate region doesn't overlap with any other
VMAs.
Fixes#3018
Fixes: 2c9ee0abd2 ("restore: update prev_vma_end after processing the last vma in a list")
Signed-off-by: Andrei Vagin <avagin@google.com>
mmaps done just for reserving address space something like
void *addr = mmap(NULL, SIZE, PROT_NONE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
Ensure that these do not have ac flag set in /proc/pid/smaps.
Signed-off-by: Bhavik Sachdev <b.sachdev1904@gmail.com>
An application might call mmap with PROT_NONE to reserve a large amount
of the virtual address space. Something like:
void *addr = mmap(NULL, SIZE, PROT_NONE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
During restore, when we have to premap private vmas (ie pieok is false),
CRIU tries to mmap this large PROT_NONE and fails.
If a vma has never been written to, it does not have "ac" flag set. Use
this fact to safely premap vmas that were done for address reservation.
Suggested-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Bhavik Sachdev <b.sachdev1904@gmail.com>
Here is actually two issues:
First, prev_vma_end isn't updated properly after processing the last vma
in a list.
Second, a dummy end_vma isn't fully initialized and vma_area_is can
access uninitilized data.
Signed-off-by: Andrei Vagin <avagin@google.com>
Linking on arm64 using lld fails with:
LINK criu/pie/parasite.built-in.o
ld: error: discarding .shstrtab section is not allowed
keep the missing sections to enhance portability
Fixes: #2978
Signed-off-by: Pepper Gray <hello@peppergray.xyz>
building on a system without gcc fails with error:
make[1]: gcc: No such file or directory
use system default cc to enhance portability
fixes: #2976
Signed-off-by: Pepper Gray <hello@peppergray.xyz>
Add tests for pre-dump, dump, and restore workflow using the loop
workload, a shell-job test with PTY setup, and a service test that
exercises the full workflow through a criu service daemon.
Pre-dump tests are skipped on architectures without soft-dirty page
tracking (e.g. aarch64). Only dump/restore are tested on these cases.
To help with CI debugging, the CRIU log files are shown on test failure.
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Add a reference C client that uses libcriu to checkpoint and restore
processes. Set LOCAL=0 to build against the system-installed libcriu
instead of the source tree.
Status messages use fprintf(stderr) so they are not delayed by stdout
buffering when running without a terminal (e.g. in CI).
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Starting with Linux kernel commit 7b735ef81286 ("rtnetlink: add
missing netlink_ns_capable() check for peer netns"), creating a
veth pair with a peer in a different network namespace requires
CAP_NET_ADMIN in the peer namespace as well:
rtnetlink: add missing netlink_ns_capable() check for peer netns
rtnl_newlink() lacks a CAP_NET_ADMIN capability check on the peer
network namespace when creating paired devices (veth, vxcan,
netkit). This allows an unprivileged user with a user namespace
to create interfaces in arbitrary network namespaces, including
init_net.
Add a netlink_ns_capable() check for CAP_NET_ADMIN in the peer
namespace before allowing device creation to proceed.
Link: 7b735ef812
When CRIU restores a veth in user namespace mode, it sends the
RTM_NEWLINK request from a netlink socket inside the child user
namespace. The veth peer is placed into the root network namespace
via IFLA_NET_NS_FD, but the child user namespace does not have
CAP_NET_ADMIN in the root namespace, so the new kernel check
rejects the request with EPERM.
Fix this by routing the veth creation through usernsd when the
peer lives in an external (host) namespace specified via the
--external veth[name] restore option. usernsd runs with real
root privileges in the init user namespace, so it passes the
capability check in both namespaces.
The usernsd path is only used for external veth mappings, not for
child-to-child namespace veths (has_peer_nsid), because in that
case both namespaces share the same user namespace and already
have CAP_NET_ADMIN in each other.
The approach mirrors restore_one_macvlan(), which already solves
the same CAP_NET_ADMIN-in-both-namespaces problem for macvlan
devices. A new veth_link_info_userns() builds the RTM_NEWLINK
request without IFLA_NET_NS_FD for the peer -- since usernsd
sends the request from the root network namespace, the peer
naturally stays there. userns_restore_one_link() then adds a
top-level IFLA_NET_NS_FD to move the main device into the child
namespace.
Assisted-by: Claude Code (claude-opus-4-6):claude-opus-4-6@default
Signed-off-by: Adrian Reber <areber@redhat.com>
Protobuf 7.x (shipped in Arch Linux) removed the `label` instance
attribute from `FieldDescriptor`. Accessing `field.label` now
raises `AttributeError`, which breaks all CRIU tests at the
`check_pages_counts` step when pycriu tries to convert protobuf
stats images to dicts:
File "test/pycriu/images/pb2dict.py", line 345, in pb2dict
if field.label == FD.LABEL_REPEATED:
AttributeError: 'FieldDescriptor' object has no attribute 'label'
Protobuf 7.x replaced `field.label` with boolean properties
`field.is_repeated` and `field.is_required`. These properties
do not exist in protobuf 3.x/4.x.
Add a `_is_repeated()` compatibility helper that checks for the
`label` attribute first (protobuf <= 5.x) and falls back to
`is_repeated` (protobuf >= 7.x), keeping pycriu working across
both old and new protobuf versions.
Assisted-by: Claude Code (claude-opus-4-6):claude-opus-4-6@default
Signed-off-by: Adrian Reber <areber@redhat.com>
Starting with Linux 7.0, the rseq feature size has grown to 33 bytes
and AT_RSEQ_ALIGN has increased to 64. This causes sizeof(struct rseq)
from the uapi header to be 64 (33 bytes padded to aligned(32)), while
the actual registration size used by glibc (__rseq_size) is 33.
The rseq01 test was using sizeof(struct rseq) as the registration
size and a test-local __rseq_abi variable (with only 32-byte
alignment from the uapi header) as the rseq area. Both are
wrong on kernel 7.0:
- The kernel now checks alignment against __alignof__(struct rseq)
which is 64 internally; the 32-byte-aligned test variable may
not satisfy this.
- sizeof(struct rseq) = 64 does not match the feature size of 33
that glibc registered with, so re-registration with a different
size fails.
Fix by:
- Using __rseq_size (the feature size reported by glibc) as the
registration size instead of sizeof(struct rseq).
- Using the glibc-provided rseq area (at thread_pointer() +
__rseq_offset) which is allocated with proper AT_RSEQ_ALIGN
alignment, instead of a test-local variable.
Both fall back to the previous behavior when glibc rseq support
is not available.
This mirrors the glibc fix:
https://sourceware.org/cgit/glibc/commit/?id=67f303b47dc584f204e3f2441b9832082415eebc
Assisted-by: Claude Code (claude-opus-4-6):claude-opus-4-6@default
Signed-off-by: Adrian Reber <areber@redhat.com>
Kernel 7.1 removed IPPROTO_UDPLITE support. Add a checkskip
script that probes for a UDPLITE socket and skips the test
with EPROTONOSUPPORT instead of failing.
Assisted-by: Claude Code (claude-opus-4-6):claude-opus-4-6@default
Signed-off-by: Adrian Reber <areber@redhat.com>
Refactor lima.sh to extract shared helpers (_common_setup and
_common_test) and add a second variant that uses the
@kernel-vanilla/next COPR repository. Convert the Lima CI job
to a matrix with both stable and next variants so we catch
upstream kernel changes (such as the UDPLITE removal in 7.1)
early.
Assisted-by: Claude Code (claude-opus-4-6):claude-opus-4-6@default
Signed-off-by: Adrian Reber <areber@redhat.com>
Kernel 7.1 removed IPPROTO_UDPLITE support. When the UDPLITE diag
module is absent, the netlink request returns -ENOENT, which CRIU
was treating as a fatal error causing the entire dump to fail.
Apply the same treatment as IPPROTO_RAW: return success from the
error callback so the dump can proceed. If a UDPLITE socket is
actually encountered, it will fail at lookup time with a clear
error rather than failing the entire dump preemptively.
Fixes: https://github.com/checkpoint-restore/criu/issues/3002
Assisted-by: Claude Code (claude-opus-4-6):claude-opus-4-6@default
Signed-off-by: Adrian Reber <areber@redhat.com>
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>
---
v2:
* Handle newly added pr_perror after amdgpu_device_initialize().
* Add newlines.
v3:
* Add some newlines to pr_err.
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>
Only try to restore the dmabuf fd if the render node restore failed due
image not present. Other errors means the file is a render node (image is
present) so we can return an error straight away.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.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> # v1
---
v2:
* Handle short reads and writes.
* Add O_TRUNC.
v3:
* Handle EINTR and EAGAIN.
* Set errno on len == 0.
* Use %zu for size_t printing.
v4:
* Drop shadowed ret.
* Drop needless casts.
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>
---
v2:
* s/for(;;)/while(1)/ (David)
Both CRIU and plugin are built with 64-bit off_t and most of the parallel
bo restore path uses uint64_t for storing offsets.
Lets consolidate by changing int to off_t, and by doing so remove any
possibility that the call to fseeko can overflow the return type.
While at it, add error checking to both seek operations.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com> # v1
---
v2:
- Correct offset type as well.
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>
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>
