Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kerndat: Skip clone3(set_tid) when unprivileged. #2252

Open
wants to merge 192 commits into
base: criu-dev
Choose a base branch
from
Open

kerndat: Skip clone3(set_tid) when unprivileged. #2252

wants to merge 192 commits into from

Conversation

osctobe
Copy link
Contributor

@osctobe osctobe commented Aug 24, 2023 •
edited
Loading

A set of fixes for kerndat tests and a few debug logging improvements.

prakritigoyal19 and others added 30 commits June 11, 2023 23:30
@prakritigoyal19 @avagin
Add flog to CRIU
823db1b 
Change made through this commit:
- Include copy of flog as a seperate tree.
- Modify the makefile to add and compile flog code.

Signed-off-by: prakritigoyal19 <[email protected]>
@adrianreber @avagin
flog: Missing varargs init or cleanup (VARARGS)
1444b72 
CID 302713 (checkpoint-restore#1 of 1): Missing varargs init or cleanup (VARARGS)
 va_end was not called for argptr.

Signed-off-by: Adrian Reber <[email protected]>
@adrianreber @avagin
Run 'make indent' on 'flog/'
cc193dc 
Separate commit for easier criu-dev <-> master transfer.

Acked-by: Mike Rapoport <[email protected]>
Signed-off-by: Adrian Reber <[email protected]>
@kolyshkin @avagin
flog: typo: mmaped -> mmapped
1fa21a0 
It is mapped, not maped. Same applies for mmap I guess.

Found by codespell, except it wants to change it to mapped,
which will make it less specific.

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin @avagin
flog: fix some codespell warnings
7eaf7a3 
Brought to you by

    codespell -w

(using codespell v2.1.0).

[v2: use "make indent" on the result]

Signed-off-by: Kir Kolyshkin <[email protected]>
@Daz-3ux @avagin
limit the field width of 'scanf'
aed3f34 
Fixes: checkpoint-restore#2121

Signed-off-by: Pengda Yang <[email protected]>
@h0lyalg0rithm @avagin
sk-inet: Add IP TOS socket option
d6860d0 
The TOS(type of service) field in the ip header allows you specify the
priority of the socket data.

Signed-off-by: Suraj Shirvankar <[email protected]>
@h0lyalg0rithm @avagin
zdtm: Add tests for ip tos restore
0dd4668 
Signed-off-by: Suraj Shirvankar <[email protected]>
@hdzhoujie @avagin
dump: increase fcntl call failure judgment
13eb876 
The pipe_size type is unsigned int, when the fcntl call fails and
return -1, it will cause a negative rollover problem.

Signed-off-by: zhoujie <[email protected]>
@adrianreber @avagin
compel: support XSAVE on newer Intel CPUs
970c4ab 
Newer Intel CPUs (Sapphire Rapids) have a much larger xsave area than
before. Looking at older CPUs I see 2440 bytes.

    # cpuid -1 -l 0xd -s 0
    ...
        bytes required by XSAVE/XRSTOR area     = 0x00000988 (2440)

On newer CPUs (Sapphire Rapids) it grows to 11008 bytes.

    # cpuid -1 -l 0xd -s 0
    ...
        bytes required by XSAVE/XRSTOR area     = 0x00002b00 (11008)

This increase the xsave area from one page to four pages.

Without this patch the fpu03 test fails, with this patch it works again.

Signed-off-by: Adrian Reber <[email protected]>
@adrianreber @avagin
ci: fix new codespell errors
1d4c5ed 
Signed-off-by: Adrian Reber <[email protected]>
@adrianreber @avagin
scripts: make newer versions of shellcheck happy
d98c533 
Signed-off-by: Adrian Reber <[email protected]>
@Snorch @avagin
criu-ns: make --pidfile option show pid in caller pidns
8e0697d 
Using the fact that we know criu_pid and criu is a parent of restored
process we can create pidfile with pid on caller pidns level.

We need to move mount namespace creation to child so that criu-ns can
see caller pidns proc.

Signed-off-by: Pavel Tikhomirov <[email protected]>
@rst0git @avagin
docs: rename amdgpu_plugin.txt to criu-amdgpu-plugin.txt
806ee35 
By default, the file name 'amdgpu_plugin.txt' is used also as the name
for the corresponding man page (`man amdgpu_plugin`). However, when
this man page is installed system-wide it would be more appropriate
to have a prefix 'criu-' (e.g., `man criu-amdgpu-plugin`).

Signed-off-by: Radostin Stoyanov <[email protected]>
@adrianreber @avagin
lib/c: add empty_ns interfaces to libcriu
f57bda4 
crun wants to set empty_ns and this interface is missing from the
library. This adds it to libcriu.

Signed-off-by: Adrian Reber <[email protected]>
@warusadura @avagin
criu-ns: Add --criu-binary argument to run_criu()
f308272 
--criu-binary argument provides a way to supply the CRIU binary
location to run_criu().

Related to: checkpoint-restore#1909

Signed-off-by: Dhanuka Warusadura <[email protected]>
@warusadura @avagin
criu-ns: Add support for older Python version in CI
38db5e1 
These changes remove and update the changes introduced in
7177938 in favor of the
Python version in CI.

os.waitstatus_to_exitcode() function appeared in Python 3.9

Related to: checkpoint-restore#1909

Signed-off-by: Dhanuka Warusadura <[email protected]>
@warusadura @avagin
criu-ns: Add tests for criu-ns script
8094df8 
These changes add test implementations for criu-ns script.

Fixes: checkpoint-restore#1909

Signed-off-by: Dhanuka Warusadura <[email protected]>
@warusadura @avagin
criu-ns: Install Python pathlib module in CentOS 7
f0e9358 
These changes fix the `ImportError: No module named pathlib`
error when executing criu-ns tests located at criu/test/others/criu-ns

Signed-off-by: Dhanuka Warusadura <[email protected]>
@warusadura @avagin
criu-ns: Update shebang line to python
9130fef 
CentOS 7 CI environment uses Python 2. To execute criu-ns
script in CentOS 7 changing the current shebang line to
python is required.

This reverse the changes made in a15a63f

Signed-off-by: Dhanuka Warusadura <[email protected]>
@KAGA-KOKO @avagin
timers: improve and fix posix timer id sequence checks
358f09c 
This is a patch proposed by Thomas here:
https://lore.kernel.org/all/87ilczc7d9.ffs@tglx/

It removes (created id > desired id) "sanity" check and adds proper
checking that ids start at zero and increment by one each time when we
create/delete a posix timer.

First purpose of it is to fix infinite looping in create_posix_timers on
old pre 3.11 kernels.

Second purpose is to allow kernel interface of creating posix timers
with desired id change from iterating with predictable next id to just
setting next id directly. And at the same time removing predictable next
id so that criu with this patch would not get to infinite loop in
create_posix_timers if this happens.

Thanks a lot to Thomas!

Signed-off-by: Pavel Tikhomirov <[email protected]>
@rst0git @avagin
action-scripts: Add pre-stream hook
104a828 
This hook allows to start image streamer process from an action script.

Signed-off-by: Radostin Stoyanov <[email protected]>
@valeriyvdovin @avagin
cgroup/restore: split prepare_task_cgroup code into two separate func…
4d137b8 
…tions

This does cgroup namespace creation separately from joining task
cgroups. This makes the code more logical, because creating cgroup
namespace also involves joining cgroups but these cgroups can be
different to task's cgroups as they are cgroup namespace roots
(cgns_prefix), and mixing all of them together may lead to
misunderstanding.

Another positive thing is that we consolidate !item->parent checks in
one place in restore_task_with_children.

Signed-off-by: Valeriy Vdovin <[email protected]>
Signed-off-by: Pavel Tikhomirov <[email protected]>
@osctobe @avagin
Fix dumping hugetlb-based memfd on kernels < 4.16.
af0e413 
4.15-based kernels don't allow F_*SEAL for memfds created with MFD_HUGETLB.
Since seals are not possible in this case, fake F_GETSEALS result as if it
was queried for a non-sealing-enabled memfd.

Signed-off-by: Michał Mirosław <[email protected]>
@osctobe @avagin
Fix mount(cgroup2) for older kernels.
11288c9 
Linux 4.15 doesn't like empty string for cgroup2 mount options.
Pass NULL then to satisfy the kernel check. Log the options for
easier debugging.

Signed-off-by: Michał Mirosław <[email protected]>
@osctobe @avagin
Restore THP_DISABLE prctl.
9e6454f 
The original commit added saving THP_DISABLED flag value, but missed
restoring it. There is restoring code, but used only when --lazy_pages
mode is enabled. Restore the prctl flag always. While at it, rename the
`has_thp_enabled` -> `!thp_disabled` for consistency.

Fixes: bbbd597 (2017-06-28 "mem: add dump state of THP_DISABLED prctl")
Signed-off-by: Michał Mirosław <[email protected]>
@osctobe @avagin
Log if prctl(SET_THP_DISABLE) doesn't work as expected.
7ca6856 
If prctl(SET_THP_DISABLE) is not used due to bad semantics, log it
for easier debugging.

Signed-off-by: Michał Mirosław <[email protected]>
@osctobe @avagin
zdtm: thp_disable: Output a single failure message
d3a33ca 
While at it, don't carry over stale errno to the fail() message.

Signed-off-by: Michał Mirosław <[email protected]>
@osctobe @avagin
zdtm: thp_disable: Verify prctl(THP_DISABLE) migration
6006cb6 
Signed-off-by: Michał Mirosław <[email protected]>
@osctobe @avagin
zdtm: thp_disable: Verify MADV_NOHUGEPAGE before migration
c75c017 
Add a sanity check for THP_DISABLE. This discovered a broken commit
in Google's kernel tree.

Signed-off-by: Michał Mirosław <[email protected]>
osctobe and others added 3 commits October 13, 2023 07:42
@osctobe @avagin
zdtm: cgroup_ifpriomap: Improve skip check's robustness.
711775f 
cgroup_ifpriomap test needs net_prio cgroup, which might not be
available. Make the .checkskip script check it.

Signed-off-by: Michał Mirosław <[email protected]>
@rst0git @avagin
lib: use separate packages for pycriu and crit
df24fe8 
Newer versions of pip use an isolated virtual environment when building
Python projects. However, when the source code of CRIT is copied into
the isolated environment, the symlink for `../lib/py` (pycriu) becomes
invalid. As a workaround, we used the `--no-build-isolation` option for
`pip install`. However, this functionality has issues in some versions
of PIP [1, 2]. To fix this problem, this patch adds separate packages
for pycriu and crit, and each package is installed independently.

[1] pypa/pip#8221
[2] pypa/pip#8165 (comment)

Signed-off-by: Radostin Stoyanov <[email protected]>
@marcusfolkesson @avagin
Makefile: introduce ARCHCFLAGS for arch specific cflags
c474816 
Do not use $(USERCFLAGS) for anything other than what the user provide.

Signed-off-by: Marcus Folkesson <[email protected]>
@Snorch
Copy link
Member

Snorch commented Oct 24, 2023

Hello @avagin and @osctobe, I had one idea, which I like a lot, but guys in mainstream Linux faced it with total silence.

The idea was to allow clone3 syscall to alter owner user namespace of newly created namespaces (e.g. new pid namespace owner if CLONE_NEWPID is specified). https://lore.kernel.org/all/[email protected]/

This way using clone3 CRIU is able to create all restored processes in topmost user namespace available, while preserving namespace ownership topology. So at each clone3 call we would have all permissions needed by clone3_set_tid functionality. (Later we can switch to proper user namespace for each process to also preserve task's user namespaces.)

I believe my fix would help in this case too. @osctobe Can you, please, give it a try on your environment?

SallyKAN and others added 11 commits November 28, 2023 09:12
@SallyKAN @avagin
comple: correct the syscall number of bind on ARM64
d88dcef 
In the compel/arch/arm/plugins/std/syscalls/syscall.def, the syscall number of bind on ARM64 should be 200 instead of 235

Signed-off-by: Sally Kang <[email protected]>
@adrianreber
ci: fix rawhide netlink error
5e56756 
The rawhide netlink errors are fixed with a newer kernel than the
default 6.2 available in Fedora 38.

Signed-off-by: Adrian Reber <[email protected]>
@adrianreber
test: check for btrfs in the current directory
4213f16 
The old test was checking if '/' is btrfs but we should check if the
current directory is btrfs.

Signed-off-by: Adrian Reber <[email protected]>
@adrianreber
ci: switch to permissive selinux mode during test
9d3e71a 
Signed-off-by: Adrian Reber <[email protected]>
@avagin
ci: fix codespell errors
b17a73b 
Signed-off-by: Andrei Vagin <[email protected]>
@rst0git @avagin
docker-test: fix condition for max tries
95975e0 
Replace a recursive call with a loop.

Reported-by: Andrei Vagin <[email protected]>
Signed-off-by: Radostin Stoyanov <[email protected]>
@rst0git @avagin
docker-test: downgrade docker to v24.0.7
0da1ab2 
Checkpoint/restore with version 25.0.0-beta.1 fails
with the following error:

$ docker start --checkpoint=c1 cr
Error response from daemon: failed to create task for container: content digest fdb1054b00a8c07f08574ce52198c5501d1f552b6a5fb46105c688c70a9acb45: not found: unknown

Release notes:
moby/moby#46816

Signed-off-by: Radostin Stoyanov <[email protected]>
@iv-m @avagin
Makefile: Use common warnings settings for loongarch64
378da3b 
WARNINGS variable should be amended, not redefined.
We still need, e.g.,  `-Wno-dangling-pointer` to build
criu on loongarch64 with gcc13.

Signed-off-by: Ivan A. Melnikov <[email protected]>
@mihalicyn @avagin
tty: skip ioctl(TIOCSLCKTRMIOS) if possible
dc49eb4 
If ioctl(TIOCSLCKTRMIOS) fails with EPERM it means that a CRIU
process lacks of CAP_SYS_ADMIN capability. But we can use
ioctl(TIOCGLCKTRMIOS) to *read* current ->termios_locked
value from the kernel and if it's the same as we already have
we can skip failing ioctl(TIOCSLCKTRMIOS) safely.

Adrian has recently posted [1] a very good patch to allow ioctl(TIOCSLCKTRMIOS)
for processes that have CAP_CHECKPOINT_RESTORE (right now it requires CAP_SYS_ADMIN).

[1] https://lore.kernel.org/all/[email protected]/

Suggested-by: Andrei Vagin <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@adrianreber @avagin
ci: do not use 'tail' for skip-file-rwx-check test
1573064 
Newer versions of 'tail' rely on inotify and after a restore 'tail' is
unhappy with the state of inotify and just stops.

This replaces 'tail' with a minimal shell based test (thanks Andrei).

Signed-off-by: Adrian Reber <[email protected]>
@adrianreber @avagin
ci: fix centos-stream 9 ci errors
561f845 
The image has a too old version of nettle which does not work with gnutls.
Just upgrade to the latest to make the error go away.

Signed-off-by: Adrian Reber <[email protected]>
@github-actions GitHub Actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

adrianreber and others added 4 commits December 21, 2023 08:50
@adrianreber
ci: disable non-root in user namespace test in container
e7aca13 
Signed-off-by: Adrian Reber <[email protected]>
@rst0git @avagin
gitignore: remove historical left-over files
61224f2 
In commit [1] was introduced a mechanism to auto-generate the files:
sys-exec-tbl*.c, syscalls*.S, syscall-codes*.h, and syscall*.h.
This commit also updated the gitignore rules to ignore auto-generated
files. However, after commit [2], the path for these files has changed
and the patterns specified in gitignore are no longer needed.

[1] bbc2f13 (x86/build: generate syscalls-{64,32}.built-in.o)
[2] 19fadee (compel: plugins,std -- Implement syscalls in std plugin)

Reported-by: @felicitia
Signed-off-by: Radostin Stoyanov <[email protected]>
@avagin
make: fix compilation on alpine
50aa6da 
Starting with the musl v1.2.4~69, _GNU_SOURCE doesn't set _LARGEFILE64_SOURCE.

Fixes checkpoint-restore#2313
Signed-off-by: Andrei Vagin <[email protected]>
@rayrapetyan @avagin
irmap: hardcode some more interesting paths
cda1c5c 
Signed-off-by: robert <[email protected]>
@osctobe
kerndat: Skip clone3(set_tid) when unprivileged.
ff088a7 
clone3(set_tid) requires CAP_CHECKPOINT_RESTORE we might not have.
Assume that if it errored out with EPERM it's there and might be
usable from inside a user namespace.

Signed-off-by: Michał Mirosław <[email protected]>
@github-actions github-actions bot removed the stale-pr label Jan 10, 2024
@rst0git rst0git added the no-auto-close Don't auto-close as a stale issue label Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avagin avagin Awaiting requested review from avagin

Assignees
No one assigned
Labels
no-auto-close Don't auto-close as a stale issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.