Parasite creation started to fail with GCC 12:
On x86_64 with:
./compel/compel-host hgen -f criu/pie/restorer.built-in.o -o criu/pie/restorer-blob.h
Error (compel/src/lib/handle-elf-host.c:337): Unexpected undefined symbol: `strlen'. External symbol in PIE?
On aarch64 with:
ld: criu/pie/restorer.o: in function `lsm_set_label':
/drone/src/criu/pie/restorer.c:174: undefined reference to `strlen'
Line 174 is: "for (len = 0; label[len]; len++)"
Adding '-ffreestanding' to parasite compilation fixes these errors
because, according to GCC developers:
"strlen is a standard C function, so I don't see any bug in that being used
unless you do a freestanding compilation (-nostdlib isn't that)."
Signed-off-by: Adrian Reber <areber@redhat.com>
This is the workaround for #1429.
The parasite code contains instructions that trigger SIGTRAP to stop at
certain points. In such cases, the kernel sends a force SIGTRAP that
can't be ignore and if it is blocked, the kernel resets its signal
handler to a default one and unblocks it. It means that if we want to
save the origin signal handle
Signed-off-by: Andrei Vagin <avagin@gmail.com>
My editor (vim) auto-removes whitespace at EOL for *.c and *.h files,
and I think it makes sense to have a separate commit for this, rather
than littering other commits with such changes.
To make sure this won't pile up again, add a line to Makefile under
the linter target to check for such things (so CI will fail).
This is all whitespace except an addition to Makefile.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This reverts commit c98af78c58.
Now FPU/SSE/MMX/etc can be used inside parasite.
Let's have compiler optimizations back.
Cc: Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
Arch-dependend way to restore extended registers set.
Use it straight-away to restore per-thread registers.
Signed-off-by: Dmitry Safonov <dima@arista.com>
Extended registers set for task is restored with rt_sigreturn() through
prepared sigframe. For threads it's currently lost.
Preserve it inside thread context to restore on thread curing.
Signed-off-by: Dmitry Safonov <dima@arista.com>
To minimize things done in parasite, PTRACE_GET_THREAD_AREA can be
used to get remote tls. That also removes an additional compat stack
(de)allocation in the parasite (also asm-coded syscall).
In order to use PTRACE_GET_THREAD_AREA, the dumpee should be stopped.
So, let's move this from criu to compel to non-seized state and put tls
into thread info on x86.
Signed-off-by: Dmitry Safonov <dima@arista.com>
We've seen addresses in parasite.built-in.o precalculated by linker but
in some unexpected manner:
readelf -WS criu/pie/parasite.built-in.o
Section Headers:
[Nr] Name Type Address Off Size ES Flg Lk Inf Al
[ 1] .text PROGBITS 0000000000000000 000040 00400a 00 AX 0 0 16
[87] .data PROGBITS 0000000000000000 005000 000068 00 WA 0 0 4096
[88] .rodata PROGBITS 0000000000000080 005080 001016 00 A 0 0 32
(Notes: All other sections does not have SHF_ALLOC or are of size 0, so I
skip them. Need to add "-specs=/usr/lib/rpm/redhat/redhat-annobin-cc1" to
CFLAGS to reproduce.)
Section 88 has address 0x80 in elf file but in our "consequent"
addresses precalculation algorithm it should be at 0x5080:
addr(.text) == 0x0
addr(.data) == 0x400a + (0x1000 - 0x400a % 0x1000) + 0x68 == 0x5068
addr(.rodata) == 0x5068 + (0x20 - 0x5068 % 0x20) == 0x5080
Probably the linker advises us to move 4096 aligned section to the
beginning to save some space, but it's just a guess.
So probably we should be ready to "non-consequent" alignments
precalculated and just override them.
https://github.com/checkpoint-restore/criu/issues/1301
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Using scan-build there is a warning about
infect.c:231:17: warning: The left operand of '!=' is a garbage value
if (ss->state != 'Z') {
which is a false positive as every process will have a 'Status' field,
but initializing the structure makes the clang analyzer silent.
Signed-off-by: Adrian Reber <areber@redhat.com>
Some kernels have W^X mitigation, which means they won't execute memory
blocks if that memory block is also writable or ever was writable. This
patch enables CRIU to run on such kernels.
1. Align .data section to a page.
2. mmap a memory block for parasite as RX.
3. mprotect everything after .text as RW.
Signed-off-by: Michał Cłapiński <mclapinski@google.com>
GNU ld precalculates this information but lld does not. With this
change, handle-elf.c calculates those addresses on its own.
When calculating addresses sections with SHF_ALLOC bit are put one after
another, respecting their alignment requirements. This matches the way
how the blob is constructed by copying section contents.
Signed-off-by: Wojciech Marczenko <marczenko@google.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Previously, the GOT table was using the same memory location as the
args region, leading to difficult to debug memory corruption bugs.
We allocate the GOT table between the parasite blob and the args region.
The reason this is a good placement is:
1) Putting it after the args region is possible but a bit combersome as
the args region has a variable size
2) The cr-restore.c code maps the parasite code without the args region,
as it does not do RPC.
Another option is to rely on the linker to generate a GOT section, but I
failed to do so despite my best attempts.
Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>
We don't need to push 0 on the stack. This seems to be a remnant of the
initial commit of 2011 that had a `pushq $0`.
The 16 bytes %rsp alignment was added with commit 2a0cea29 in 2012.
This is no longer necessary as we already guarantee that %rsp is 16
bytes aligned. A BUG_ON() is added to enforce this guarantee.
Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>
Previously, __export_parasite_cmd was located in parasite-head.S, and
__export_parasite_args located exactly at the end of the parasite blob.
This is not ideal for various reasons:
1) These two variables work together. It would be preferrable to have
them in the same location
2) This prevent us from allocating another section betweeen the parasite
blob and the args area. We'll need this to allocate a GOT table
This commit changes the allocation of these symbols from assembly/linker
script to a C file.
Moreover, the assembly entry points that invoke parasite_service()
prepares arguments with hand crafted assembly. This is unecessary.
This commit rewrite this logic with regular C code.
Note: if it wasn't for the x86 compat mode, we could remove all
parasite-head.S files and directly jump to parasite_service() via
ptrace. An int3 architecture specific equivalent could be called at the
end of parasite_service() with an inline asm statement.
Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>
It is unnecessary and potentially confusing for understanding the memory
layout requirement of the parasite blob.
Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>
It removes the potential confusion when it comes to virtual address vs
offsets. Further, doing so makes naming more consistent with the rest of
the parasite_blob_desc struct.
Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>
compel_relocs_apply() was taking arguments mostly from the struct
parasite_blob_desc. Instead of passing all the arguments, we pass a
pointer to the struct itself.
This makes the code safer, as cr-restore.c calls compel_relocs_apply().
It previously needed to poke into what can be considered private
variables of the restorer-pie.h file.
To allow the parasite_blob_desc struct to be populated without a
parasite_ctl struct, we expand the compel API to export a
parasite_setup_c_header_desc() in the generated pie.h.
Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>
COMMON symbols are emitted for global variable that are not initialized
in shared libraries.
They typically end up in the .bss section when linking an executable.
Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>
We should follow Linux Kernel Codding Style:
... the closing brace is empty on a line of its own, except in the cases
where it is followed by a continuation of the same statement, ie ... an
else in an if-statement ...
https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces
Automaticly fixing with:
:!git grep --files-with-matches "^\s*else[^{]*{" | xargs
:argadd <files>
:argdo :%s/}\s*\n\s*\(else[^{]*{\)/} \1/g | update
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
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 <dima@arista.com>
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 <alexander.mikhalitsyn@virtuozzo.com>
Signed-off-by: Alexander Mikhalitsyn <alexander@mihalicyn.com>
Unknown state means that the task in the end may be not in wanted state.
Return err code.
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
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 <dima@arista.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Also, don't use the magic -2 => return errno on failure.
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
>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 <dima@arista.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
It has a different alignment - rework ugly macro RT_SIGFRAME_UC_SIGMASK
into helpers.
Fixes: #666
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
for i in $(find . -name "*.[ch]" -type f); do
sed -i 's/\(pr_warn(".*[^n]\)\("[),]\)/\1\\n\2/' $i
done
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
We need to know what are stack pointers of every thread to ensure that the
current stack page will not be treated as lazy.
Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
Acked-by: Alice Frosi <alice@linux.vnet.ibm.com>
Reviewed-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
For architectures like aarch64/ppc64 it's needed to propagate the size
of page inside PIEs. For the parasite page size will be defined during
seizing, and for restorer during early initialization.
Afterward we can use PAGE_SIZE in PIEs like we did before.
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
get_task_regs() needs to know if it needs to use workaround
for a Skylake ptrace() bug. The next patch will introduce a
new flag for that.
I also thought about making 3 versions of get_task_regs() and
adding them to ictx->get_task_regs() depending on the flags..
But get_task_regs() is a private function and infect_ctx is
a uapi.. So, let's just pass context flags to get_task_regs().
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
Currently we use the "map_files/%p-%p" format, but actually it should
be "map_files/%lx-%lx".
The kernel could handle both formats, but recently Alexey Dobriyan fixed
the kernel and it accept only the second format.
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
Add handeling of R_X86_64_GOTPCRELX and R_X86_64_REX_GOTPCRELX.
They are not that old, so I provided ifdef-guards for them.
According to x86-64 ABI specification paper, they should be
generated instead of R_X86_64_GOTPCREL for cases when relaxation
is possible.
At this moment we can handle them the same way like R_X86_64_GOTPCREL.
[0] https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-r249.pdfFixes: #397
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Reported-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
This is what we have:
> compel/src/lib/infect.c:1145:38: error: taking address of packed member
> 'uc_sigmask' of class or structure 'ucontext_ia32' may result in an
> unaligned pointer value [-Werror,-Waddress-of-packed-member]
> blk_sigset = RT_SIGFRAME_UC_SIGMASK(f);
> ~~~~~~~~~~~~~~~~~~~~~~~^~
> compel/include/uapi/asm/sigframe.h:133:4: note: expanded from macro
> 'RT_SIGFRAME_UC_SIGMASK'
> (&rt_sigframe->compat.uc.uc_sigmask))
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
Indeed this results in an unaligned pointer, but as this is intended and
well known (see commit dd6736bd "compel/x86/compat: pack ucontext_ia32"),
we need to silence the warning here.
For more details, see https://reviews.llvm.org/D20561
Originally found by Travis on Alpine Linux, reproduced on Ubuntu 17.10.
[v2: fix for non-x86]
Reported-by: Andrei Vagin <avagin@virtuozzo.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
Add s390 parts to common code files.
Patch history
-------------
v2->v3:
* Add: s390: Consolidate -msoft-float into Makefile.compel
Reviewed-by: Alice Frosi <alice@linux.vnet.ibm.com>
Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
Reviewed-by: Dmitry Safonov <dsafonov@virtuozzo.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
In case of success, we want to be silent when on default log level.
This is a time-honored UNIX tradition, who we are to break it?
Signed-off-by: Kir Kolyshkin <kir@openvz.org>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
As we have more than 1 working plugin right now, let's implement
the TODO item for "compel plugins" command.
Signed-off-by: Kir Kolyshkin <kir@openvz.org>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
Some get_status() methods may allocate data, because
not all of the fields in /proc/[pid]/status file
have the fixed size. For example, NSpid, which
size may vary.
Introduce new method free_status() in counterweight
for such type get_status() methods. it will be called
in case of we go to try_again and need to free allocated
data.
Also, introduce data parameter for a use in the future.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
Unlike pr_perror(), pr_err() does not append a newline.
Signed-off-by: Kir Kolyshkin <kir@openvz.org>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
Since commit 93db22b ("compel: no -r for ARM ldflags") and
commit ce043d3 ("compel: make plugins .a archives") the generated
parasite/restorer blobs for 32-bit ARM are no longer relocatable,
and so their ELF header has e_type of ET_EXEC (rather than ET_REL).
As a result, we have "Unsupported header detected" message and
a failure from compel hgen.
This commit allows e_type == ET_EXEC in case NO_RELOCS is defined.
I am not sure if it's OK to still allow ET_REL -- maybe Dmitry
can chime in on that.
Cc: Dmitry Safonov <dsafonov@virtuozzo.com>
Signed-off-by: Kir Kolyshkin <kir@openvz.org>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
