[draft] support of bindmounted unix dgram sockets #1001

Drop the constants for default cache host/port and page size because they are not used anywhere. Signed-off-by: Radostin Stoyanov <[email protected]> Reviewed-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]>

1) fix sfle memory leak on get_fle_for_scm error 2) fix gfd open descriptor leak on get_fle_for_scm error 3-6) fix buf memory leak on read and pwrite errors Signed-off-by: Pavel Tikhomirov <[email protected]>

Cc: Adrian Reber <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Show error message when image-{cache,proxy} is called without --port and image-proxy without --address argument. Signed-off-by: Radostin Stoyanov <[email protected]>

criu/sk-packet.c:443:3: error: 'strncpy' output may be truncated copying 14 bytes from a string of length 15 strncpy(addr_spkt.sa_data, req.ifr_name, sa_data_size); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ criu/img-remote.c:383:3: error: 'strncpy' specified bound 4096 equals destination size strncpy(snapshot_id, li->snapshot_id, PATHLEN); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ criu/img-remote.c:384:3: error: 'strncpy' specified bound 4096 equals destination size strncpy(path, li->name, PATHLEN); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ criu/files.c:288:3: error: 'strncpy' output may be truncated copying 4095 bytes from a string of length 4096 strncpy(buf, link->name, PATH_MAX - 1); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ criu/sk-unix.c:239:36: error: '/' directive output may be truncated writing 1 byte into a region of size between 0 and 4095 snprintf(path, sizeof(path), ".%s/%s", dir, sk->name); ^ criu/sk-unix.c:239:3: note: 'snprintf' output 3 or more bytes (assuming 4098) into a destination of size 4096 snprintf(path, sizeof(path), ".%s/%s", dir, sk->name); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ criu/mount.c:2563:3: error: 'strncpy' specified bound 4096 equals destination size strncpy(path, m->mountpoint, PATH_MAX); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ criu/cr-restore.c:3647:2: error: 'strncpy' specified bound 16 equals destination size strncpy(task_args->comm, core->tc->comm, sizeof(task_args->comm)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Andrei Vagin <[email protected]>

CID 190778 (checkpoint-restore#1 of 1): Read from pointer after free (USE_AFTER_FREE) 7. deref_after_free: Dereferencing freed pointer rop. Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Both functions check_pending_forwards() and check_pending_reads() have very similar functionality. This patch aims to reduce the duplication of code by merging both functions into check_pending() Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

The macro PATHLEN is redundant. It is defined such that its replacement token sequence is the token PATH_MAX. Signed-off-by: Radostin Stoyanov <[email protected]> Acked-by: Adrian Reber <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

The function `setup_TCP_server_socket` (defined in img-remote.c) and `setup_tcp_server` (defined in util.c) have very similar functionality. Replace setup_TCP_server_socket() with setup_tcp_server() to reduce code duplication and to enable IPv6 support for the image-cache action of CRIU. We set SO_REUSEADDR flag to allow reuse of local addresses. Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Remove setup_TCP_client_socket() and use setup_tcp_server() instead as both functions have very similar functionality. Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Check only once if (sockfd < 0) Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

When CRIU calls the ip tool on restore, it passes the fd of remote socket by replacing the STDIN before execvp. The stdin is used by the ip tool to receive input. However, the ip tool calls ftell(stdin) which fails with "Illegal seek" since UNIX sockets do not support file positioning operations. To resolve this issue, read the received content from the UNIX socket and store it into temporary file, then replace STDIN with the fd of this tmp file. # python test/zdtm.py run -t zdtm/static/env00 --remote -f ns === Run 1/1 ================ zdtm/static/env00 ========================= Run zdtm/static/env00 in ns ========================== Start test ./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST Adding image cache Adding image proxy Run criu dump Run criu restore =[log]=> dump/zdtm/static/env00/31/1/restore.log ------------------------ grep Error ------------------------ RTNETLINK answers: File exists (00.229895) 1: do_open_remote_image RDONLY path=route-9.img snapshot_id=dump/zdtm/static/env00/31/1 (00.230316) 1: Running ip route restore Failed to restore: ftell: Illegal seek (00.232757) 1: Error (criu/util.c:712): exited, status=255 (00.232777) 1: Error (criu/net.c:1479): IP tool failed on route restore (00.232803) 1: Error (criu/net.c:2153): Can't create net_ns (00.255091) Error (criu/cr-restore.c:1177): 105 killed by signal 9: Killed (00.255307) Error (criu/mount.c:2960): mnt: Can't remove the directory /tmp/.criu.mntns.dTd7ak: No such file or directory (00.255339) Error (criu/cr-restore.c:2119): Restoring FAILED. ------------------------ ERROR OVER ------------------------ ################# Test zdtm/static/env00 FAIL at CRIU restore ################## ##################################### FAIL ##################################### Fixes checkpoint-restore#311 Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

In Py2 `range` returns a list and `xrange` creates a sequence object that evaluates lazily. In Py3 `range` is equivalent to `xrange` in Py2. Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Pavel Tikhomirov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

As discussed on the mailing list, current .py files formatting does not conform to the world standard, so we should better reformat it. For this the yapf tool is used. The command I used was yapf -i $(find -name *.py) Signed-off-by: Pavel Emelyanov <[email protected]>

Most of the typos were found by codespell. Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

This patch does not introduce any functional changes. Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

The function accept_proxy_to_cache() is a wrapper around accept(). Use accept() directly instead. Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]>

Reduce code duplication by introducing a strflags() macro which maps a flag to corresponding string. Signed-off-by: Radostin Stoyanov <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]>

This patch makes various private variables and procedures static. These changes conform to the following code style conventions: When declaring pointer data or a function that returns a pointer type, the preferred use of ``*`` is adjacent to the data name or function name and not adjacent to the type name. Statements longer than 80 columns will be broken into sensible chunks, unless exceeding 80 columns significantly increases readability and does not hide information. Descendants are always substantially shorter than the parent and are placed substantially to the right. The same applies to function headers with a long argument list. from https://www.kernel.org/doc/Documentation/process/coding-style.rst The function declarations {send,recv}_image() from img-remote.h are removed because they do not have a corresponding implementation. The following functions are made static because they are not used outside img-remote.c: * {send,recv}_image_async() * {read,write}_remote_header() * socket_set_non_blocking() Signed-off-by: Radostin Stoyanov <[email protected]>

There is no need to print an error message, __xalloc() does that. Signed-off-by: Radostin Stoyanov <[email protected]>

When an interrupt signal (even SIGWINCH when strace is used) is received while epoll_wait() sleeps, it will return a value of -1 and set errno to EINTR, which is not an error and should be ignored. Signed-off-by: Radostin Stoyanov <[email protected]>

There is no need to pass the values for address and port as arguments when creating a TCP server. The external `opts` object, which provides opts.addr and opts.port, is accessible in all components that require these values. With this change, a value specified with the `--address` option will used by image-cache in the same way as with page-server. Example: criu image-cache --address 127.0.0.1 --port 1234 criu page-server --address 127.0.0.1 --port 1234 Signed-off-by: Radostin Stoyanov <[email protected]>

The variable name 'remote_sk' is shorter than 'proxy_to_cache_fd' and it is more similar to 'page_server_sk' (used in criu/page-xfer.c). Signed-off-by: Radostin Stoyanov <[email protected]>

The name 'local_sk' is shorter than 'local_req_fd', and it is more similar to the name 'page_server_sk' used in criu/page-xfer.c Signed-off-by: Radostin Stoyanov <[email protected]>

When saddr.ss_family is AF_INET6 we should cast &saddr to (struct sockaddr_in6 *). Signed-off-by: Radostin Stoyanov <[email protected]>

Combine the functionality of socket_set_non_blocking() and socket_set_blocking() into a new function, and move it in criu/util.c to enable reusability throughout the code base. Signed-off-by: Radostin Stoyanov <[email protected]>

Combine the macro constants DUMP_FINISH and RESTORE_FINISH, into a single one, called FINISH. In addition, replace the key-word strings used by the above-mentioned constants, and NULL_SNAPSHOT_ID, with a \0 character that will be used to indicate a "finish" message. Signed-off-by: Radostin Stoyanov <[email protected]>

Sort and remove unused/unnecessary include statements in criu/img-*.c Signed-off-by: Radostin Stoyanov <[email protected]>

The print_data() function was part of the deprecated (and removed) 'show' action, and it was moved in util.c with the following commit: a501b48 The 'show' action has been deprecated since 1.6, let's finally drop it. The print_data() routine is kept for yet another (to be deprecated too) feature called 'criu exec'. The criu exec feature was removed with: 909590a Remove criu exec code It's now obsoleted by compel library. Maybe-TODO: Add compel tool exec action? Therefore, now we can drop print_data() as well. Signed-off-by: Radostin Stoyanov <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]>

class ctypes.c_char_p Represents the C char * datatype when it points to a zero- terminated string. For a general character pointer that may also point to binary data, POINTER(c_char) must be used. The constructor accepts an integer address, or a bytes object. https://docs.python.org/3/library/ctypes.html#ctypes.c_char_p Signed-off-by: Radostin Stoyanov <[email protected]>

There are a few places where spaces have been used instead of tabs for indentation. This patch converts the spaces to tabs for consistency with the rest of the code base. Signed-off-by: Radostin Stoyanov <[email protected]>

criu-3.12/criu/net.c:2043: overwrite_var: Overwriting "img" in "img = open_image_at(-1, CR_FD_IP6TABLES, 0UL, pid)" leaks the storage that "img" points to. Signed-off-by: Adrian Reber <[email protected]>

Signed-off-by: Pavel Emelyanov <[email protected]>

The function clear_ghost_files() has been removed in commit b11eeea "restore: auto-unlink for ghost files (v2)". Signed-off-by: Radostin Stoyanov <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]>

Right now we use pushq, but it pushes sign-extended value, so if the parasite code is placed higher that 2Gb, we will see something like this: 0xf7efd5b0: pushq $0x23 0xf7efd5b2: pushq $0xfffffffff7efd5b9 => 0xf7efd5b7: lretq Actually we want to push 0xf7efd5b9 instead of 0xfffffffff7efd5b9. Fixes: checkpoint-restore#398 Cc: Dmitry Safonov <[email protected]> Cc: Cyrill Gorcunov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]> Acked-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

`pushq` sign-extends the value. Which is a bummer as the label's address may be higher that 2Gb, which means that the sign-bit will be set. As it long-jumps with ia32 selector, %r11 can be scratched. Use %r11 register as a temporary to push the 32-bit address. Complements: a9a7602 ("arch/x86: push correct eip on the stack before lretq") Cc: Cyrill Gorcunov <[email protected]> Reported-by: Andrei Vagin <[email protected]> Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

This change was introduced with c75cb2b and it is no longer necessary. Signed-off-by: Radostin Stoyanov <[email protected]>

Instead of creating cgroup yard in CRIU, now we can create it externally and pass it to CRIU. Useful if somebody doesn't want to grant CAP_SYS_ADMIN to CRIU. Signed-off-by: Michał Cłapiński <[email protected]>

Signed-off-by: Michał Cłapiński <[email protected]>

The following tests fail in Fedora rawhide because /usr/bin/diff is missing. * zdtm/static/bridge(ns) * zdtm/static/cr_veth(uns) * zdtm/static/macvlan(ns) * zdtm/static/netns(uns) * zdtm/static/netns-nf(ns) * zdtm/static/sit(ns) Signed-off-by: Radostin Stoyanov <[email protected]>

Before the 5.2 kernel, only fpu_state->fpu_state_64.xsave has to be 64-byte aligned. But staring with the 5.2 kernel, the same is required for pu_state->fpu_state_ia32.xsave. The behavior was changed in: c2ff9e9a3d9d ("x86/fpu: Merge the two code paths in __fpu__restore_sig()") Signed-off-by: Andrei Vagin <[email protected]>

Prior log initialisation CRIU preserves all (early) log messages in a buffer. In case of error the content of the content of this buffer needs to be printed out (flushed). Suggested-by: Dmitry Safonov <[email protected]> Signed-off-by: Radostin Stoyanov <[email protected]>

Print error messages to stderr (instead of stdout). Suggested-by: Andrei Vagin <[email protected]> Signed-off-by: Radostin Stoyanov <[email protected]>

Right now it is cleaned up from a post-restore hook, but zdtm.py can be executed with the norst option: $ zdtm.py run -t zdtm/static/cgroup_yard --norst ... OSError: [Errno 17] File exists: 'external_yard' Cc: Michał Cłapiński <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Right now, it is created from the pre-dump hook, but if the --snap option is set, the test fails: $ python test/zdtm.py run -t zdtm/static/cgroup_yard -f h --snap --iter 3 ... Running zdtm/static/cgroup_yard.hook(--pre-dump) Traceback (most recent call last): File zdtm/static/cgroup_yard.hook, line 14, in <module> os.mkdir(yard) OSError: [Errno 17] File exists: 'external_yard' Cc: Michał Cłapiński <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

1) s/\s*$// 2) fix snprintf out of bound access Signed-off-by: Pavel Tikhomirov <[email protected]>

Reported-by: Mr Jenkins Signed-off-by: Andrei Vagin <[email protected]>

1) Instead of tampering with the nr argument, do_full_int80() returns the value of the system call. It also avoids copying all registers back into the syscall_args32 argument after the syscall. 2) Additionally, the registers r12-r15 were added in the list of clobbers as kernels older than v4.4 do not preserve these. 3) Further, GCC uses a 128-byte red-zone as defined in the x86_64 ABI optimizing away the correct position of the %rsp register in leaf-functions. We now avoid tampering with the red-zone, fixing a SIGSEGV when running mmap_bug_test() in debug mode (DEBUG=1). Signed-off-by: Nicolas Viennot <[email protected]> Reviewed-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

PATH_MAX == 4096; PATH_MAX*8 == 32k; RESTORE_STACK_SIZE == 32k. Fixes: a3cdf94 ("inotify: cleanup auxiliary events from queue") Cc: Pavel Tikhomirov <[email protected]> Cc: Andrei Vagin <[email protected]> Co-debugged-with: Andrei Vagin <[email protected]> Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Two modes of pre-dump algorithm: 1) splicing memory by parasite --pre-dump-mode=splice (default) 2) using process_vm_readv syscall --pre-dump-mode=read Signed-off-by: Abhishek Dubey <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Skip iov-generation for regions not having PROT_READ, since process_vm_readv syscall can't process them during "read" pre-dump. Handle random order of "read" & "splice" pre-dumps. Signed-off-by: Abhishek Dubey <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

"read" mode pre-dump may fail even after adding PROT_READ flag. Adding PROT_READ works when dumping statically. See added comment for details. Signed-off-by: Abhishek Dubey <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

adding cnt_sub function (complement of cnt_add). cnt_sub is utilized to decrement stats counter according to skipped page count during "read" mode pre-dump. Signed-off-by: Abhishek Dubey <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Pre-dump using the process_vm_readv syscall. During frozen state, only iovecs will be generated and draining of memory happens after the task is unfrozen. Pre-dumping of shared memory remains unmodified. Signed-off-by: Abhishek Dubey <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

refactoring time macros as per read mode pre-dump design. Signed-off-by: Abhishek Dubey <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Andrei Vagin <[email protected]>

lib/c/criu.c:343:30: error: implicit conversion from enumeration type 'enum criu_pre_dump_mode' to different enumeration type 'CriuPreDumpMode' (aka 'enum _CriuPreDumpMode') [-Werror,-Wenum-conversion opts->rpc->pre_dump_mode = mode; ~ ^~~~ Signed-off-by: Andrei Vagin <[email protected]>

atomic_sub isn't defined for all platforms. Reported-by: Mr Jenkins Cc: Abhishek Dubey <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Conflict register for file "sk-opts.proto": READ is already defined in file "rpc.proto". Please fix the conflict by adding package name on the proto file, or use different name for the duplication. Note: enum values appear as siblings of the enum type instead of children of it. checkpoint-restore#815 Signed-off-by: Andrei Vagin <[email protected]>

The lock status string may be empty. This can happen when the owner of the lock is invisible from our PID namespace. This unfortunate behavior is fixed in kernels v4.19 and up (see commit 1cf8e5de40) Signed-off-by: Nicolas Viennot <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

When performing pre-dump we continuously increase the page-pipe size to fit the max amount memory pages in the pipe's buffer. However, we never actually set the pipe's buffer size to max. By doing so, we can reduce the number of pipe-s necessary for pre-dump and improve the performance as shown in the example below. For example, let's consider the following process: #include <stdio.h> #include <stdlib.h> #include <unistd.h> void main(void) { int i = 0; void *cache = calloc(1, 1024 * 1024 * 1024); while(1) { printf("%d\n", i++); sleep(1); } } stats-dump before this change: frozen_time: 123538 memdump_time: 95344 memwrite_time: 11980078 pages_scanned: 262721 pages_written: 262169 page_pipes: 513 page_pipe_bufs: 519 stats-dump after this change: frozen_time: 83287 memdump_time: 54587 memwrite_time: 12547466 pages_scanned: 262721 pages_written: 262169 page_pipes: 257 page_pipe_bufs: 263 Signed-off-by: Radostin Stoyanov <[email protected]>

Signed-off-by: Pavel Tikhomirov <[email protected]>

The support for per-pid images with locks has been dropped with commit d040219 ("locks: Drop support for per-pid images with locks") and CR_FD_FILE_LOCKS_PID is not used. Signed-off-by: Radostin Stoyanov <[email protected]>

RPC messages are have fairly small size and using space on the stack might be a better option. This change follows the pattern used with do_pb_read_one() and pb_write_one(). Signed-off-by: Radostin Stoyanov <[email protected]>

libcriu tests are currently broken. This patch fixes couple of issues to allow the building and running libcriu tests. 1. lib/c/criu.h got updated to include version.h which is present at "criu/include", but the command to compile libcriu tests is not specifying "criu/include" in the path to be searched for header files. This resulted in compilation error. This can be fixed by adding "-I ../../../../../criu/criu/include" however it causes more problems as "criu/include/fcntl.h" would now hide system defined fcntl.h Solution is to use "-iquote ../../../../../criu/criu/include" which applies only to the quote form of include directive. 2. Secondly, libcriu.so major version got updated to 2 but libcriu/run.sh still assumes verion 1. Instead of just updating the version in libcriu/run.sh to 2, this patch updates the libcriu/Makefile to use "CRIU_SO_VERSION_MAJOR" so that future changes to major version of libcriu won't cause same problem again. Signed-off-by: Ashutosh Mehra <[email protected]>

Updated scripts/travis/travis-tests to run libcriu test. Signed-off-by: Ashutosh Mehra <[email protected]>

PATH is pointing to incorrect location for `criu` executable causing libcriu tests to fail when running in travis. Also added statements to display log file contents on failure to help in debugging. Signed-off-by: Ashutosh Mehra <[email protected]>

I don't see many issues with early-log, so we probably don't need the warning when it was used. Note that after commit 74731d9 ("zdtm: make grep_errors also grep warnings") also warnings are grepped by zdtm.py (and I believe that was an improvement) which prints some bothering lines: > =[log]=> dump/zdtm/static/inotify00/38/1/dump.log > ------------------------ grep Error ------------------------ > (00.000000) Will allow link remaps on FS > (00.000034) Warn (criu/log.c:203): The early log isn't empty > ------------------------ ERROR OVER ------------------------ Instead of decreasing loglevel of the message, improve it by reporting a real issue. Cc: Adrian Reber <[email protected]> Cc: Pavel Tikhomirov <[email protected]> Cc: Radostin Stoyanov <[email protected]> Signed-off-by: Dmitry Safonov <[email protected]>

Signal masks propagate through execve, so we need to clear them before invoking the action scripts as it may want to handle SIGCHLD, or SIGSEGV. Signed-off-by: Nicolas Viennot <[email protected]>

With the newly introduced aarch64 at Travis it is possible for the CRIU test-cases to switch to aarch64. Travis uses unprivileged LXD containers on aarch64 which blocks many of the kernel interfaces CRIU needs. So for now this only tests building CRIU natively on aarch64 instead of using the Docker+QEMU combination. All tests based on Docker are not working on aarch64 is there currently seems to be a problem with Docker on aarch64. Maybe because of the nesting of Docker in LXD. Signed-off-by: Adrian Reber <[email protected]>

Update test to support both iptables and nft to create conntrack rules. Signed-off-by: Vitaly Ostrosablin <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Nidhi Gupta <[email protected]>

It seems pretty stable and hasn't add many false-positives during last months. While can reveal some issues for compatible C/R code. Signed-off-by: Dmitry Safonov <[email protected]>

The path: restore_root_task prepare_namespace_before_tasks mntns_maybe_create_roots is always called before the path below: retore_root_task fork_with_pid restore_task_with_children prepare_namespace prepare_mnt_ns populate_mnt_ns So (!!mnt_roots) == (root_ns_mask & CLONE_NEWNS) in populate_mnt_ns, but in prepare_mnt_ns we've already checked that it is true, so there is no need in these check - remove it. Signed-off-by: Pavel Tikhomirov <[email protected]>

It's never used outside of namespaces.c Signed-off-by: Pavel Tikhomirov <[email protected]>

Images for mount and net namespaces are empty if ns does not belong to us, thus we don't need to collect on restore. By adding these checks we will eliminate suspicious messages in logs about lack of images: ./test/zdtm.py run -k always -f h -t zdtm/static/env00 env00/54/2/restore.log:(00.000332) No mountpoints-5.img image env00/54/2/restore.log:(00.000342) No netns-2.img image Signed-off-by: Pavel Tikhomirov <[email protected]>

Build each mntns mount tree alone just after reading mounts for it from image. These additional step before merging everything to a single mount tree allows us to have pointers to each mntns root mount at hand, also it allows us to remove extra complication from mnt_build_tree. Teach collect_mnt_from_image return a tail pointer, so we can merge lists together later after building each tree. Add separate merge_mount_trees helper to create joint mount tree for all mntns'es and simplify mnt_build_ids_tree. I don't see any place where we use mntinfo_tree on restore, so save the real root of mntns mounts tree in it, instead of root_yard_mp, will need it in next patches for checking restore of these trees. v2: prepend children to the root_yard in merge_mount_trees so that the order in merged tree persists Signed-off-by: Pavel Tikhomirov <[email protected]>

Add mnt_subtree_next DFS-next search to remove recursion. v5: add these patch, remove recursion from sorting helpers v6: rip out butifull yet unused step-part of nfs-next algorithm Signed-off-by: Pavel Tikhomirov <[email protected]>

Signed-off-by: Nidhi Gupta <[email protected]>

In my previous commit I copied a line with a return into the main script body. bash can only return from functions. This changes return to exit. Signed-off-by: Adrian Reber <[email protected]>

Travis uses unprivileged containers for aarch64 in LXD. Docker with '--privileged' fails in such situation. This changes the travis setup to only start docker with '--privileged' if running on x86_64. Signed-off-by: Adrian Reber <[email protected]>

For CRIU's compile only tests for armv7hf on Travis we are using 'setarch linux32' which returns armv8l on Travis aarch64. This adds a path in the Makefile to treat armv8l just as armv7hf during compile. This enables us to run armv7hf compile tests on Travis aarch64 hardware. Much faster. Maybe not entirely correct, but probably good enough for compile testing in an armv7hf container. Signed-off-by: Adrian Reber <[email protected]>

This switches all arm related tests (32bit and 64bit) to the aarch64 systems Travis provides. For arm32 we are running in a armv7hf container on aarch64 with 'setarch linux32'. The main changes are that docker on Travis aarch64 cannot use '--privileged' as Travis is using unprivileged LXD containers to setup the testing environment. Signed-off-by: Adrian Reber <[email protected]>

Now that Travis also supports ppc64le and s390x we can remove all qemu based docker emulation from our test setup. This now runs ppc64le and s390x tests on real hardware (LXD containers). Signed-off-by: Adrian Reber <[email protected]>

Tests are successful even after removing 'group:' from .travis.yml. Apparently it is not necessary. Signed-off-by: Adrian Reber <[email protected]>

>From man ptrace: > On error, all requests return -1, and errno is set appropriately. > Since the value returned by a successful PTRACE_PEEK* request may be > -1, the caller must clear errno before the call, and then check > it afterward to determine whether or not an error occurred. FWIW: if ptrace_peek_area() is called with (errno != 0) it may false-fail if the data is (-1). Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Also, don't use the magic -2 => return errno on failure. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

As a preparation for __must_check on compel_syscall(), check it on close() too - maybe not as useful as with other syscalls, but why not. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

And remove __maybe_unused work-around. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

On daemon stop or threads dump failures it's still desired to remove parasite from the remote (if possible). Try best and keep hopeing. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Probably, not the worst that could happen, but still unexpected. Preparing the ground to make compel_cure*() functions __must_check. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Too late to stop restore: it's already printed that restore was successful. Oh, well warn aloud about infection. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Maybe expected, hopefully never happens - let's warn in any case. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Unknown state means that the task in the end may be not in wanted state. Return err code. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Currently if anything fails after network has been unlocked tasks aren't killed. Which doesn't work anyway: any stage sets `ret` and nothing later gets called. Which means the tasks aren't resumed properly. Furthermore, functions like catch_tasks() and compel_stop_on_syscall() return failure on the first error. Let's do the cleanup even when the network is unlocked. If we want to keep the mess and ignore failures - a cli option should be introduced for that (and existing code should be reworked with decisions what is critical and what can be ignored). Move "Restore finished successfully" message accordingly where everything is evidently good. While at here, any late failure will result not only in cleanup but in criu returning error code. Which in result makes tests to fail in such case: > ======================= Run zdtm/static/inotify04 in ns ======================== > Start test > ./inotify04 --pidfile=inotify04.pid --outfile=inotify04.out --dirname=inotify04.test > Run criu dump > =[log]=> dump/zdtm/static/inotify04/84/1/dump.log > ------------------------ grep Error ------------------------ > (00.119763) fsnotify: openable (inode match) as zdtm/static/inotify04.test/inotify-testfile > (00.119766) fsnotify: Dumping /zdtm/static/inotify04.test/inotify-testfile as path for handle > (00.119769) fsnotify: id 0x00000b flags 0x000800 > (00.119787) 88 fdinfo 5: pos: 0 flags: 4000/0 > (00.119796) Warn (criu/fsnotify.c:336): fsnotify: The 0x00000c inotify events will be dropped > ------------------------ ERROR OVER ------------------------ > Run criu restore > =[log]=> dump/zdtm/static/inotify04/84/1/restore.log > ------------------------ grep Error ------------------------ > (00.391582) 123 was stopped > (00.391667) 106 was trapped > (00.391674) 106 (native) is going to execute the syscall 11, required is 11 > (00.391697) 106 was stopped > (00.391720) Error (compel/src/lib/infect.c:1439): Task 123 is in unexpected state: b7f > (00.391736) Error (compel/src/lib/infect.c:1447): Task stopped with 11: Segmentation fault > ------------------------ ERROR OVER ------------------------ > 5: Old maps lost: set([]) > 5: New maps appeared: set([u'10000-1a000 rwxp', u'1a000-24000 rw-p']) > ############### Test zdtm/static/inotify04 FAIL at maps compare ################ > Send the 9 signal to 106 > Wait for zdtm/static/inotify04(106) to die for 0.100000 > ======================= Test zdtm/static/inotify04 PASS ======================== Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

All those compel functions can fail by various reasons. It may be status of the system, interruption by user or anything else. It's really desired to handle as many PIE related errors as possible otherwise it's hard to analyze statuses of parasite/restorer and the C/R process. At least warning for logs should be produced or even C/R stopped. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Before this change CRIU would checkpoint all controllers, even the ones not specified in --cgroup-dump-controller. That becomes a problem if there's a cgroup controller on the checkpointing machine that doesn't exist on the restoring machine even if CRIU is instructed not to dump that controller. After that change everything works as expected. Signed-off-by: Michał Cłapiński <[email protected]>

The amount of lazy-pages options keeps growing, let's put the common ones into a variable. Signed-off-by: Mike Rapoport <[email protected]>

Kernels 5.4 and higher will restrict availability of UFFD_EVENT_FORK only for users with SYS_CAP_PTRACE. This prevents running --lazy-pages tests with 'uns' flavor. Disable 'uns' for lazy pages testing in travis for newer kernels. Signed-off-by: Mike Rapoport <[email protected]>

Suggested-by: Andrei Vagin <[email protected]> Signed-off-by: Radostin Stoyanov <[email protected]>

When restoring a runc container with enabled user namespace CRIU fails to mount the specified root directory because the path is under /run/runc which is inaccessible to unprivileged users. Signed-off-by: Radostin Stoyanov <[email protected]>

When restoring a container with user namespace, CRIU fails to create a temporary directory for proc. The is because the unprivileged user that has been just restored does not have permissions to access the working directory used by CRIU. Resolves checkpoint-restore#828 Signed-off-by: Radostin Stoyanov <[email protected]>

These requirements have been described in https://github.com/opencontainers/runc/blob/b133feae/libcontainer/container_linux.go#L1265 Signed-off-by: Radostin Stoyanov <[email protected]>

Replacing the if->continue pattern with if->elif->else reduces the number of lines while preserving the logic. Signed-off-by: Radostin Stoyanov <[email protected]>

Update zdtm tests to verify that CRIU does not require the --root path to be accessible to the unprivileged user being restored when restoring user namespace. Signed-off-by: Radostin Stoyanov <[email protected]>

SRCARCH is always equal ARCH. There are no rules when to use one or another and architectures may forget to set one of them up. No need for a second variable meaning the same and confusing people. Remove it completely. Self-correction [after some debug]: SRCARCH was different in one place: zdtm Makefile by some unintentional mistake: > ifeq ($(ARCH),arm64) > ARCH ?= aarch64 > SRCARCH ?= aarch64 > endif That meant to be "ARCH := aarch64" because "?=" would never work inside that ifeq. Fix up this part of mess too. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

It's not used anywhere now. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

It's always equal ARCH and not very useful (so nothing actually uses it). Time for a change: SUBARCH now is meaningful and gives a way to detect what kind of ARCH flavor build is dealing with. Also, for cross-compiling sake don't set SUBARCH if the user supplied it. (and don't call useless uname during cross compilation) Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Instead of doing additional `uname -m` - use provided $(SUBARCH) to detect what architecture flavour the build should produce the result for. Fixes two things: - zdtm make now correctly supplies $(USERCFLAGS) - subtly fixes cross compilation by providing a way to specify $(SUBARCH) Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Maybe not that useful, but only little change needed. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

The very same hack to build aarch32 zdtm tests on armv8 Travis-CI as in the commit dfa0a1e ("Makefile hack for travis aarch64/armv8l") Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Helps to cross-compile zdtm tests in case somebody needs it. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Fixes: checkpoint-restore#455 Based-on-patch-by: Andrei Vagin <[email protected]> Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

The original/old guide probably doesn't work anymore: - the patch isn't accessible; - criu now depends on more libraries not only protobuf Still, keep it as it might be helpful for someone. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Fixes the following compile-error: > CC socket-tcp-fin-wait1.o > socket-tcp-fin-wait1.c:144:26: error: adding 'int' to a string does not append to the string [-Werror,-Wstring-plus-int] > if (write(fd, TEST_MSG + 2, sizeof(TEST_MSG) - 2) != sizeof(TEST_MSG) - 2) { > ~~~~~~~~~^~~ > socket-tcp-fin-wait1.c:144:26: note: use array indexing to silence this warning > if (write(fd, TEST_MSG + 2, sizeof(TEST_MSG) - 2) != sizeof(TEST_MSG) - 2) { > ^ > & [ ] > 1 error generated. Signed-off-by: Dmitry Safonov <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Nidhi Gupta <[email protected]>

until it will not be fixed. Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Andrei Vagin <[email protected]>

It doesn't require to create a temporary directory and mount the proc file system in it. Signed-off-by: Andrei Vagin <[email protected]>

We will continue accepting patches. Signed-off-by: Andrei Vagin <[email protected]>

…ombies We've seen ppoll interrupted with signal in VZ7 CT migration tests, that is because in the beggining of CR_STATE_RESTORE_SIGCHLD zombies and helpers die, and that can trigger SIGCHILDs sent to their parents. Adding additional debug (printing "Task..." for zombies and helpers) in sigchld_handler I see: (15.644339) pie: 1: Task 10718 exited, status= 0 (15.644349) pie: 1: Cleaning inotify events from 29 (15.644359) pie: 1: Cleaning inotify events from 19 (15.644367) pie: 1: Cleaning inotify events from 10 And previousely we had: (05.718449) pie: 104: Cleaning inotify events from 5 (05.718835) pie: 330: Cleaning inotify events from 3 (05.719046) pie: 1: Cleaning inotify events from 23 (05.719164) pie: 80: Cleaning inotify events from 7 (05.719185) pie: 1: Error (criu/pie/restorer.c:1287): Failed to poll from inotify fd: -4 (05.719202) pie: 95: Cleaning inotify events from 6 (05.719269) pie: 1: Error (criu/pie/restorer.c:1890): Restorer fail 1 So reordering cleanup and wait should fix it. Signed-off-by: Pavel Tikhomirov <[email protected]>

When we do clone threads in a later stage of restore procedure it may race with helpers which do call clone_noasan by self. Thus we need to walk over each clone_noasan call and figure out if calling it without last_pid lock is safe. - open_mountpoint: called by fusectl_dump, dump_empty_fs, binfmt_misc_dump, tmpfs_dump -- they all are processing dump stage, thus safe - call_helper_process: try_remount_writable -- called from various places in reg-files.c, in particular open_reg_by_id called in parallel with other threads, needs a lock remount_readonly_mounts -- called from sigreturn_restore, so in parallel, needs a lock - call_in_child_process: prepare_net_namespaces -- called from prepare_namespace which runs before we start forking, no need for lock Thus call_helper_process should use lock_last_pid and unlock_last_pid helpers and wait for subprocess to finish. Same time put a warning text into clone_noasan comment so next time we need to use it we would recall the pitfalls. v2: - fix unitialized ret variable v3: - use exit_code instead of ret Signed-off-by: Cyrill Gorcunov <[email protected]>

The /proc/sys/net/unix/max_dgram_qlen is a per-net variable and we already noticed that systemd inside a container may change its value (for example it sets it to 512 by now instead of kernel's default value 10), thus we need keep it inside image and restore then. Signed-off-by: Cyrill Gorcunov <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]>

Test checks that if the /proc/sys/net/unix/max_dgram_qlen value has been changed in process net namespace, then it is saved after c/r. Signed-off-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]>

Make xfeatures_mask check explicit. We were relying on our guess about hardware "backward compatibility" and used ">" check here for a long time. But it looks better to explicitly check that all xfeature bits available on the source are also available on the destination. For xsave_size we need to have smaller size on destination than on source, because xsave operation on small allocated buffer may corrupt the nearby data. So split up comments about xfeatures_mask and xsave_size, as having single comment for quiet a different cases is less understandable. v2: improve comments, remove extra else-ifs, remove extra typecast Signed-off-by: Pavel Tikhomirov <[email protected]>

Signed-off-by: Nicolas Viennot <[email protected]>

The py-future package has been renamed to py3-future [1] and py2 package for yaml has been dropped [2]. [1] https://git.alpinelinux.org/aports/commit/main?id=316d44abaed13964e97eb43c095cd1b64e3943ad [2] https://git.alpinelinux.org/aports/commit/main?id=e369c1fd7707a73f2c3e2b11b613198d9a4106de Signed-off-by: Radostin Stoyanov <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]>

TCP keepalive packets can be used to determine if a connection is still valid. When the SO_KEEPALIVE option is set, TCP packets are periodically sent to keep the connection alive. This patch implements checkpoint/restore support for SO_KEEPALIVE, TCP_KEEPIDLE, TCP_KEEPINTVL and TCP_KEEPCNT options. Signed-off-by: Radostin Stoyanov <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]>

It seems like Ubuntu introduced a overlayfs change which breaks CRIU: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1857257 This disables overlayfs (unfortunately) in most tests by switching to devicemapper or vfs. Upstream kernels do not seem to have this problem. This also adds the 'docker-test' for xenial which still has a working overlayfs from CRIU's point of view. Also adjust Podman Ubuntu package location Podman Ubuntu packages are now available via OBS and no longer via PPA. Signed-off-by: Adrian Reber <[email protected]>

Include warnings that the kernel uses during compilation: -Wdeclaration-after-statement: enforces having variables declared at the top of scopes Signed-off-by: Nicolas Viennot <[email protected]> [Generated a commit message from the pull request] Signed-off-by: Dmitry Safonov <[email protected]>

Include warnings that the kernel uses during compilation: -Wstrict-prototypes: enforces full declaration of functions. Previously, when declaring extern void func(), one can call func(123) and have no compilation error. This is dangerous. The correct declaration is extern void func(void). Signed-off-by: Nicolas Viennot <[email protected]> [Generated a commit message from the pull request] Signed-off-by: Dmitry Safonov <[email protected]>

After Centos-8 nft used instead of iptables. But we had never supported nft rules in CRIU, and after c/r all rules are flushed. Co-developed-by: Pavel Tikhomirov <[email protected]> Signed-off-by: Pavel Tikhomirov <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Dmitry Safonov <[email protected]>

Signed-off-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]> [Added test_author to zdtm test] Signed-off-by: Dmitry Safonov <[email protected]>

Signed-off-by: Nicolas Viennot <[email protected]>

The kernel artificially adds the O_CLOEXEC flag when reading from the /proc/fdinfo/fd interface if FD_CLOEXEC is set on the file descriptor used to access the file. This commit removes the O_CLOEXEC flag in our file flags. To restore the proper FD_CLOEXEC value in each of the file descriptors, CRIU uses fcntl(F_GETFD) to retrieve the FD_CLOEXEC status, and restore it later with fcntl(F_SETFD). This is necessary because multiple file descriptors may point to the same open file.

Now that Python 2 has officially reached its end of life also switch the Alpine based test to Python 3. Signed-off-by: Adrian Reber <[email protected]>

Signed-off-by: Nidhi Gupta <[email protected]>

Compiling 'criu-dev' on Fedora 31 gives two errors about wrong clobber lists: compel/include/uapi/compel/asm/sigframe.h:47:9: error: listing the stack pointer register ‘1’ in a clobber list is deprecated [-Werror=deprecated] criu/arch/ppc64/include/asm/restore.h:14:2: error: listing the stack pointer register ‘1’ in a clobber list is deprecated [-Werror=deprecated] There was also a bug report from Debian that CRIU does not build because of this. Each of these errors comes with the following note: note: the value of the stack pointer after an ‘asm’ statement must be the same as it was before the statement As far as I understand it this should not be a problem in this cases as the code never returns anyway. Running zdtm very seldom fails during 'zdtm/static/cgroup_ifpriomap' with a double free or corruption. This happens not very often and I cannot verify if it happens without this patch. As CRIU does not build without the patch. Signed-off-by: Adrian Reber <[email protected]>

To ensure consistency of runtime environment processes within a container need to see same start time values over suspend/resume cycles. We introduce new field to the core image structure to store start time of a dumped process. Later same value would be restored to a newly created task. In future the feature is likely to be pulled here, so we reserve field id in protobuf descriptor. Signed-off-by: Valeriy Vdovin <[email protected]>

We are running each podman test loop 50 times. This takes more than 20 minutes in Travis. Reduce both test loops to only run 20 times. Signed-off-by: Adrian Reber <[email protected]>

Linux kernel 5.4 extends clone3() with set_tid to allow processes to specify the PID of a newly created process. This introduces detection of the clone3() syscall and if set_tid is supported. This first implementation is X86_64 only. Signed-off-by: Adrian Reber <[email protected]>

To create a new process/thread with a certain PID based on clone3() a new assembler wrapper is necessary as there is not glibc wrapper (yet). Signed-off-by: Adrian Reber <[email protected]>

With the in Linux Kernel 5.4 introduced clone3() with set_tid it is no longer necessary to write to to /proc/../ns_last_pid to influence the next PID number. clone3() can directly select a PID for the newly created process/thread. After checking for the availability of clone3() with set_tid and adding the assembler wrapper for clone3() in previous patches, this extends criu/pie/restorer.c and criu/clone-noasan.c to use the newly added assembler clone3() wrapper to create processes with a certain PID. This is a RFC and WIP, but I wanted to share it and run it through CI for feedback. As the CI will probably not use a 5.4 based kernel it should just keep on working as before. Signed-off-by: Adrian Reber <[email protected]>

Just like on all other supported architectures gcc complains about the stack pointer register being part of the clobber list: error: listing the stack pointer register ‘15’ in a clobber list is deprecated [-Werror=deprecated] This removes the stack pointer from the clobber list. 'zdtm.py run -a' still runs without any errors after this change. Signed-off-by: Adrian Reber <[email protected]>

This adds the parasite clone3() with set_tid wrapper for s390x. In contrast to the x86_64 implementation the thread start address and arguments are not put on the thread stack but passed via r4 and r5. As those registers are caller-saved they still contain the correct value (thread start address and arguments) after returning from the syscall. Tested on 5.5.0-rc6. Signed-off-by: Adrian Reber <[email protected]>

Just like on all other supported architectures gcc complains about the stack pointer register being part of the clobber list. This removes the stack pointer from the clobber list. Signed-off-by: Adrian Reber <[email protected]>

This adds the parasite clone3() with set_tid wrapper for ppc64le. Signed-off-by: Adrian Reber <[email protected]>

clone3() explicitly blocks setting an exit_signal if CLONE_PARENT is specified. With clone() it also did not work, but there was no error message. The exit signal from the thread group leader is taken. Signed-off-by: Adrian Reber <[email protected]>

This adds the parasite clone3() with set_tid wrapper for aarch64. Tested on Fedora 31 with 5.5.0-rc6. Signed-off-by: Adrian Reber <[email protected]>

This is the last architecture specific change to make CRIU use clone3() with set_tid if available. Just as on all other architectures this adds a clone3() based assembler wrapper to be used in the restorer code. Tested on Fedora 31 with the same 5.5.0-rc6 kernel as on the other architectures. Signed-off-by: Adrian Reber <[email protected]>

This fixes the validation errors from Travis: Build config validation root: deprecated key sudo (The key `sudo` has no effect anymore.) root: missing os, using the default linux root: key matrix is an alias for jobs, using jobs Signed-off-by: Adrian Reber <[email protected]>

…mset This patch fixes the problem with SSE (xmm) registers corruption on amd64 architecture. The problem was that gcc generates parasite blob that uses xmm registers, but we don't preserve this registers in CRIU when injecting parasite. Also, gcc, even with -nostdlib option uses builtin memcpy, memset functions that optimized for amd64 and involves SSE registers. It seems, that optimal solution is to use -ffreestanding gcc option to compile parasite. This option implies -fno-builtin and also it designed for OS kernels compilation/another code that suited to work on non-hosted environments and could prevent future sumilar bugs. To check that you amd64 CRIU build affected by this problem you could simply objdump -dS criu/pie/parasite.o | grep xmm Output should be empty. Reported-by: Diyu Zhou <zhoudiyupku at gmail.com> Signed-off-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]>

In case asciidoc is installed and xmlto is not, make returns an error but there's no diagnostics shown, since "xmlto: command not found" goes to /dev/null. Remove the redirect. Signed-off-by: Kir Kolyshkin <[email protected]>

1. Add a/the articles where I see them missing 2. s/Forbid/disable/ 3. s/crit/crit(1)/ as we're referring to a man page 4. Simplify some descriptions Signed-off-by: Kir Kolyshkin <[email protected]>

Commit 0493724 added support for using asciidoctor (instead of asciidoc + xmlto) to generate man pages. For some reason, asciidoctor does not deal well with some complex formatting that we use for options such as --external, leading to literal ’ and ' appearing in the man page instead of italic formatting. For example: > --inherit-fd fd[’N']:’resource' (here both N and resource should be in italic). Asciidoctor documentation (asciidoctor --help syntax) tells: > == Text Formatting > > .Constrained (applied at word boundaries) > *strong importance* (aka bold) > _stress emphasis_ (aka italic) > `monospaced` (aka typewriter text) > "`double`" and '`single`' typographic quotes > +passthrough text+ (substitutions disabled) > `+literal text+` (monospaced with substitutions disabled) > > .Unconstrained (applied anywhere) > **C**reate+**R**ead+**U**pdate+**D**elete > fan__freakin__tastic > ``mono``culture so I had to carefully replace *bold* with **bold** and 'italic' with __italic__ to make it all work. Tested with both terminal and postscript output, with both asciidoctor and asciidoc+xmlto. TODO: figure out how to fix examples (literal multi-line text), since asciidoctor does not display it in monospaced font (this is only true for postscript/pdf output so low priority). Signed-off-by: Kir Kolyshkin <[email protected]>

On gcc-10 (and gcc-9 -fno-common) build fails as: ``` ld: criu/arch/x86/crtools.o:criu/include/cr_options.h:159: multiple definition of `rpc_cfg_file'; criu/arch/x86/cpu.o:criu/include/cr_options.h:159: first defined here make[2]: *** [scripts/nmk/scripts/build.mk:164: criu/arch/x86/crtools.built-in.o] Error 1 ``` gcc-10 will change the default from -fcommon to fno-common: https://gcc.gnu.org/PR85678. The error also happens if CFLAGS=-fno-common passed explicitly. Reported-by: Toralf Förster Bug: https://bugs.gentoo.org/707942 Signed-off-by: Sergei Trofimovich <[email protected]>

Signed-off-by: Andrei Vagin <[email protected]>

In the current version, the offsets of remapping vvar and vdso regions are mixed up. If vdso is before vvar, vvar has to be mapped with the vdso_size offset. if vvar is before vdso, vdso has to be mapped with the vvar_size offset. Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Nicolas Viennot <[email protected]>

See "man memfd_create" for more information of what memfd is. This adds support for memfd open files, that are not not memory mapped. * We add a new kind of file: MEMFD. * We add two image types MEMFD_FILE, and MEMFD_INODE. MEMFD_FILE contains usual file information (e.g., position). MEMFD_INODE contains the memfd name, and a shmid identifier referring to the content. * We reuse the shmem facilities for dumping memfd content as it would be easier to support incremental checkpoints in the future. Signed-off-by: Nicolas Viennot <[email protected]>

Prepare memfd to use inherited_fd(), needing long path names support. Signed-off-by: Nicolas Viennot <[email protected]>

Upon file restore, inherited_fd() is called to check for a user-defined inerit-fd override. Note that the MEMFD_INODE image is read at each invocation (memfd name is not cached). Signed-off-by: Nicolas Viennot <[email protected]>

* During checkpoint, we add a vma flags: VMA_AREA_MEMFD to denote memfd regions. * Even though memfd is backed by the shmem device, we use the file semantics of memfd (via /proc/map_files/<vma>) which we already have support for. Signed-off-by: Nicolas Viennot <[email protected]>

See "man fcntl" for more information about seals. memfd are the only files that can be sealed, currently. For this reason, we dump the seal values in the MEMFD_INODE image. Restoring seals must be done carefully as the seal F_SEAL_FUTURE_WRITE prevents future write access. This means that any memory mapping with write access must be restored before restoring the seals. Signed-off-by: Nicolas Viennot <[email protected]>

File pairs naturally block on read() until the write() happen (or the writer is closed). This is not the case for regular files, so we take extra precaution for these. Also cleaned-up an extra my_file.close() Signed-off-by: Nicolas Viennot <[email protected]>

Testing for all the memfd features, namely support for CR of: * the same fd shared by multiple processes * the same file shared by multiple processes * the memfd content * file flags and fd flags * mmaps, MAP_SHARED and MAP_PRIVATE * seals, excluding F_SEAL_FUTURE_WRITE because this feature only exists in recent kernels (5.1 and up) * inherited fd Signed-off-by: Nicolas Viennot <[email protected]>

gcc8 in Fedora Rawhide has a new useful warning: > criu/img-remote.c: In function 'push_snapshot_id': > criu/img-remote.c:1099:2: error: 'strncpy' specified bound 4096 equals destination size [-Werror=stringop-truncation] > 1099 | strncpy(rn.snapshot_id, snapshot_id, PATH_MAX); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From man 3 strncpy: > Warning: If there is no null byte among the first n bytes of src, > the string placed in dest will not be null-terminated. Signed-off-by: Dmitry Safonov <[email protected]>

strncpy() with n == strlen(src) won't put NULL-terminator in dst. Signed-off-by: Dmitry Safonov <[email protected]>

Allows to override them in every test, optionally. Signed-off-by: Dmitry Safonov <[email protected]>

Use it in kerndat to check if the kernel provides vDSO. Signed-off-by: Dmitry Safonov <[email protected]>

Apparently, C/R is broken when CONFIG_VDSO is not set. Probably, I've broken it while adding arm vdso support. Or maybe some commits after. Repair it by adding checks into vdso_init_dump(), vdso_init_restore(). Also, don't try handling vDSO in restorer if it wasn't present in parent. And prevent summing VDSO_BAD_SIZE to {vdso,vvar}_rt_size. Reported-by: Adrian Reber <[email protected]> Signed-off-by: Dmitry Safonov <[email protected]>

It's always page-aligned (as any VMA). Signed-off-by: Dmitry Safonov <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]>

Podman changed the output of 'podman ps'. For the test only running containers are interesting. Adding the filter '-f status=running' only returns running containers as previously. Signed-off-by: Adrian Reber <[email protected]>

This allows to run inhfd tests with many iterations of C/R. Signed-off-by: Andrei Vagin <[email protected]>

202 Additional newline in a group of imports. I100 Import statements are in the wrong order. Signed-off-by: Andrei Vagin <[email protected]>

Signed-off-by: Pavel Tikhomirov <[email protected]>

In the recent kernels the userfaultfd support for FORK events is limited to CAP_SYS_PTRACE. That causes the followong error when the ioctl(UFFDIO_API) is executed from non-privilieged userns: Error (criu/uffd.c:273): uffd: Failed to get uffd API: Operation not permitted Wrapping the call to ioctl(UFFDIO_API) in userns_call() resolves the issue. Fixes: checkpoint-restore#964 Signed-off-by: Mike Rapoport <[email protected]>

This option was introduced with: checkpoint-restore@e2c3824 v2: (comment from Pavel Tikhomirov) --enable-fs does not fit with --external dev[]:, see try_resolve_ext_mount, external dev mounts only determined for FSTYPE__UNSUPPORTED. Signed-off-by: Radostin Stoyanov <[email protected]>

Signed-off-by: Radostin Stoyanov <[email protected]>

The cgroupv2 freezer does not return the same strings as v1. Instead of THAWED and FROZEN v2 returns 0 and 1 (strings). This prepares the seize code to use 0 and 1 everywhere and THAWED and FROZEN only for v1 specific code paths. Signed-off-by: Adrian Reber <[email protected]>

More preparations for cgroupv2 freezer. Factor our the freezer state opening and writing to have one location where to handle v1 and v2 differences. Signed-off-by: Adrian Reber <[email protected]>

This adds support to checkpoint processes using the cgroup v2 freezer. Signed-off-by: Adrian Reber <[email protected]>

The runc test cases are (sometimes) mounting a cgroup inside of the container. For these tests to succeed, let CRIU know that cgroup2 exists and how to restore such a mount. This does not fix any specific cgroup2 settings, it just enables CRIU to mount cgroup2 in the restored container. Signed-off-by: Adrian Reber <[email protected]>

Processes can run in a mount namespace without /proc. Reported-by: Mr Jenkins Signed-off-by: Andrei Vagin <[email protected]>

Right now, criu uses a dumped fd to dump content of a memfd "file". Here are two reasons why we should not do this: * a state of a dumped fd doesn't have to be changed, but now criu calls lseek on it. This can be workarounded by using pread. * a dumped descriptor can be write-only. Reported-by: Mr Jenkins Cc: Nicolas Viennot <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>

python 2.7 doesn't call the read system call if it's read file to the end once. The next seek allows to workaround this problem. inhfd/memfd.py hangs due to this issue. Reported-by: Mr Jenkins Signed-off-by: Andrei Vagin <[email protected]>

O_PATH opened files are special: they have empty file operations in kernel space, so there not that much we can do with them, even setting position is not allowed. Same applies to a signal number for owner settings. Signed-off-by: Cyrill Gorcunov <[email protected]> Co-developed-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) <[email protected]>

On these test without the patch ("fown: Don't fail on dumping files opened wit O_PATH") we trigger these errors: Error (criu/pie/parasite.c:340): fcntl(4, F_GETOWN_EX) -> -9 Error (criu/files.c:403): Can't get owner signum on 18: Bad file descriptor Error (criu/files-reg.c:1887): Can't restore file pos: Bad file descriptor Signed-off-by: Pavel Tikhomirov <[email protected]> Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) <[email protected]>

To really open symlink file and not the regular file below it, one needs to do open with O_PATH|O_NOFOLLOW flags. Looks like systemd started to open /etc/localtime symlink this way sometimes, and before that nobody actually used this and thus we never supported this in CRIU. Error (criu/files-ext.c:96): Can't dump file 11 of that type [120777] (unknown /etc/localtime) Looks like it is quiet easy to support, as c/r of symlink file is almost the same as c/r of regular one. We need to only make fstatat not following links in check_path_remap. Also we need to take into account support of ghost symlinks. Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) <[email protected]> Co-developed-by: Pavel Tikhomirov <[email protected]> Signed-off-by: Pavel Tikhomirov <[email protected]>

Signed-off-by: Pavel Tikhomirov <[email protected]> Co-Developed-by: Vitaly Ostrosablin <[email protected]> Signed-off-by: Vitaly Ostrosablin <[email protected]> Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) <[email protected]>

... otherwise write() can block. Reported-by: Mr Jenkins Signed-off-by: Andrei Vagin <[email protected]>

The plan is to remove "compel.h". That file only includes other headers (which may be not needed). If we aim for one-include-for-compel, we could instead paste all subheaders into "compel.h". Rather, I think it's worth to migrate to more fine-grained compel headers than follow the strategy 'one header to rule them all'. Further, the header creates problems for cross-compilation: it's included in files, those are used by host-compel. Which rightfully confuses compiler/linker as host's definitions for fpu regs/other platform details get drained into host's compel. As a first step - stop including "compel.h" in criu. Signed-off-by: Dmitry Safonov <[email protected]>

The file only includes other headers (which may be not needed). If we aim for one-include-for-compel, we could instead paste all subheaders into "compel.h". Rather, I think it's worth to migrate to more fine-grained compel headers than follow the strategy 'one header to rule them all'. Further, the header creates problems for cross-compilation: it's included in files, those are used by host-compel. Which rightfully confuses compiler/linker as host's definitions for fpu regs/other platform details get drained into host's compel. Signed-off-by: Dmitry Safonov <[email protected]>

Fixes: checkpoint-restore#924 Signed-off-by: Dmitry Safonov <[email protected]>

Jessie is called 'oldoldstable', migrate to Buster. Suggested-by: Adrian Reber <[email protected]> Signed-off-by: Dmitry Safonov <[email protected]>

We should ignore (not parse) images that has non-crtool format, that images has no magic number (RAW_IMAGE_MAGIC equals 0). nftables images has format compatible with `nft -f /proc/self/fd/0` input format. Reported-by: Mr Jenkins Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) <[email protected]>

Signed-off-by: Mike Rapoport <[email protected]>

Signed-off-by: Andrei Vagin <[email protected]>

The config.h detection scripts should use the provided CFLAGS/LDFLAGS as it tries to link libnl, libnet, and others. Signed-off-by: Nicolas Viennot <[email protected]>

Any shared memroy mapping can be opened via /proc/self/maps_files/. Such file descriptors look like memfd file descriptors, so they can be dumped by the same way. Signed-off-by: Andrei Vagin <[email protected]>

Here is a fast path when two consequent vma-s share the same file. But one of these vma-s can map a file with MAP_SHARED, but another one can map it with MAP_PRIVATE and we need to take this into account.

Any shared memory region can be openned via /proc/self/map_files. Signed-off-by: Andrei Vagin <[email protected]>

…rmissions Any filesystem syscall, that needs to navigate to inode by it's absolute path performs successive lookup operations for each part of the path. Lookup operation includes access rights check. Usually but not always zdtm tests processes fall under 'other' access category. Also, usually directories don't have 'x' bit set for other. In case when bit 'x' is not set and user-ID and group-ID of a process relate it to 'other', test's will not succeed in performing these syscalls which are most of filesystem api, that has const char *path as part of it arguments (open, openat, mkdir, bind, etc). The observable behavior of that is that zdtm tests fail at file creation ops on one system and pass on the other. The above is not immediately clear to the developer by just looking at failed test's logs. Investigation of that is also not quick for a developer due to the complex structure of zdtm runtime where nested clones with NAMESPACE flags take place alongside with bind-mounts. As an additional note: 'get_current_dir_name' is documented as returning EACCESS in case when some part of the path lacks read/list permissions. But in fact it's not always so. Practice shows, that test processes can get false success on this operation only to fail on later call to something like mkdir/mknod/bind with a given path in arguments. 'get_cwd_check_perm' is a wrapper around 'get_current_dir_name'. It also checks for permissions on the given filepath and logs the error. This directs the developer towards the right investigation path or even eliminates the need for investigation completely. Signed-off-by: Valeriy Vdovin <[email protected]>

…ucture The helper function removes code duplication from tests that want to initialize unix socket address to an absolute file path, derived from current working directory of the test + relative filename of a resulting socket. Because the former code used cwd = get_current_dir_name() as part of absolute filename generation, the resulting filepath could later cause failure of bind systcall due to unchecked permissions and introduce confusing permission errors. Signed-off-by: Valeriy Vdovin <[email protected]>

Without this patch, pipe size is restored only if a pipe has queued data. Reported-by: Mr Jenkins Signed-off-by: Andrei Vagin <[email protected]>

Create two pipes with and without queued data. Signed-off-by: Andrei Vagin <[email protected]>

This test doesn't expect that the write operation will block. Signed-off-by: Andrei Vagin <[email protected]>

It is known that for unix sockets we need to setup qlen first before restoring queue itself, otherwise there might be not enough place for data. Thus move the snippet into separate helper. Most likely we will merge it back again once it's been proved that for any kind of sockets it is safe to setup all options before queue data itself. https://jira.sw.ru/browse/PSBM-63762 Signed-off-by: Cyrill Gorcunov <[email protected]>

The size of qlen may be bigger than one provided by kernel as default, so pushing data may lead to Error (criu/sk-queue.c:500): Failed to send packet: Resource temporarily unavailable ie to -EAGAIN. Signed-off-by: Cyrill Gorcunov <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]>

This values are part of abi, so must be set in image file. Signed-off-by: Cyrill Gorcunov <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]> Signed-off-by: Alexander Mikhalitsyn <[email protected]>

Mount points might be beindmount to some resources (say unix binded sockets) thus when times come to do real bind mount call we need to prepare appropriate resource first. On dump procedure we walk over all bind-mounts and check if the mountpoint is a unix socket saving the mnt_id into the image then. To distinguish such sockets from others we use UNIX_UFLAGS__BINDMOUNT flag. Note at moment we support only DGRAM closed sockets. Signed-off-by: Cyrill Gorcunov <[email protected]>

Because we need to gather unix sockets earlier than we start creating mount tree. Thus we will be able to handle bindmounted sockets. Signed-off-by: Cyrill Gorcunov <[email protected]>

So when we will be examinating mount points and sockets we won't waste time on non-bindmounted. Signed-off-by: Cyrill Gorcunov <[email protected]>

We will need to take mutex when bind() bindmounted sockets. Strictly speaking we won't support bindmounted and deleted sockets for now but better prepare this scaffolds early. Signed-off-by: Cyrill Gorcunov <[email protected]>

Some unix sockets might be bindmounted (say /dev/log bound to another place). So to handle it we need to change the logic we open such sockets especially because we create mount tree earlier than we start to restore files. Thus here what we do: - on dump mark such sockets with UNIX_UFLAGS__BINDMOUNT flag so we would distinguish them on restore; - collect unix sockets before creating mount tree; note that at this moment we able to simply gather this sockets into own @unix_mnt_sockets list and nothing more because setting up the peers and such happens later in that named post action procedures; - when we need to create a bindmount point we enter into unix engine and figure out if there a socket to bindmount over; if found we pre-allocate the socketpair, bind it and save inside fdstore engine; using socketpair is important because later we need both peers to restore queued data; - finally when we start restoring files we simply fetch the socket from the fdstore and use it directly. All this scheme is working simply because we support dgram standalone sockets only, adding support for streamed sockets requires a way more engine rework and hopefully we won't need it in near future. Signed-off-by: Cyrill Gorcunov <[email protected]>

To test a case where unix socket is bind mounted to somewhere so restore may fail if socket has not been created. Signed-off-by: Cyrill Gorcunov <[email protected]>

Otherwise the mount call may fail since bindmounted unix socket wont be created. Signed-off-by: Cyrill Gorcunov <[email protected]>

To elimitane compilation warnings with gcc-8. Signed-off-by: Cyrill Gorcunov <[email protected]>

Signed-off-by: Cyrill Gorcunov <[email protected]>

For better debug Signed-off-by: Cyrill Gorcunov <[email protected]>

Signed-off-by: Cyrill Gorcunov <[email protected]>

No need to check for pair argument, it is rather confusing. Signed-off-by: Cyrill Gorcunov <[email protected]>

Currently bind_unix_sk is used in two contexts: to bind freshly created socket pairs and to bind name for sockets which are to be queued into fdstore (binmounted sockets). For first case we should notify the waiting side immediately but in turn bindmount sockets are created early and there might be the case where peers are not yet even opened and notification may simply lost or even cause sigsegv since file list is yet empty. In turn we should defer it until we do a real bindmount socket opening right after we fetched it from the fdstore. https://jira.sw.ru/browse/PSBM-88274 Signed-off-by: Cyrill Gorcunov <[email protected]>

The unlink procedure is rather a cleanup before we start creating new sockets, but bindmounted sockets are pre-created early so we should not touch them. Signed-off-by: Cyrill Gorcunov <[email protected]>

Once socket is bounded we should allow to connect to us via relative name. https://jira.sw.ru/browse/PSBM-88274 Signed-off-by: Cyrill Gorcunov <[email protected]>

Also full rework to make sure we can test the situation where client is not yet opened when we're restoring bindmount. Signed-off-by: Cyrill Gorcunov <[email protected]>

Signed-off-by: Cyrill Gorcunov <[email protected]>

Previously we always created sockets in root mount namespace but in 019ebec we've tried to resolve this problem setting up proper mount namespace. This is not always possible though: the root task from which we trying to switch ns might already have a number of children with CLONE_FS|CLONE_FILES set and kernel doesn't allow to do that. Lets disable this ability until we find a proper solution. https://jira.sw.ru/browse/PSBM-89126 Signed-off-by: Cyrill Gorcunov <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[draft] support of bindmounted unix dgram sockets #1001

[draft] support of bindmounted unix dgram sockets #1001

Commits on Jan 24, 2020

Commits on Jan 26, 2020

Commits on Jan 29, 2020

Commits on Jan 30, 2020

Commits on Feb 4, 2020

Commits on Feb 5, 2020

Commits on Feb 6, 2020

Commits on Feb 10, 2020

Commits on Feb 16, 2020

Commits on Feb 21, 2020

Commits on Feb 29, 2020

Commits on Mar 3, 2020

Commits on Mar 5, 2020

Commits on Mar 9, 2020

Commits on Mar 14, 2020

Commits on Mar 15, 2020

Commits on Mar 21, 2020

Commits on Mar 23, 2020

Commits on Mar 24, 2020

[draft] support of bindmounted unix dgram sockets #1001

Are you sure you want to change the base?

[draft] support of bindmounted unix dgram sockets #1001

Commits on Jan 24, 2020

Commits on Jan 26, 2020

Commits on Jan 29, 2020

Commits on Jan 30, 2020

Commits on Feb 4, 2020

Commits on Feb 5, 2020

Commits on Feb 6, 2020

Commits on Feb 10, 2020

Commits on Feb 16, 2020

Commits on Feb 21, 2020

Commits on Feb 29, 2020

Commits on Mar 3, 2020

Commits on Mar 5, 2020

Commits on Mar 9, 2020

Commits on Mar 14, 2020

Commits on Mar 15, 2020

Commits on Mar 21, 2020

Commits on Mar 23, 2020

Commits on Mar 24, 2020