pty_open_unpaired_slave must be called for
pty peers only and it caused no problems so
far but better to be sure it's pty peer we're
restoring (there gonna be more drivers with
time).
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
This is not pty specific routine but rather a common one.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
- rename @t to @type and use protobuf constants here instead
- for special features use @subtype just like kernel does
- get rid of TTY_TYPE_ constants, we don't need them
- drop @flags, we don't need it anymore
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
We have them in vma_entry_is_private() already.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
We have two helpers for VMA type testing: privately_dump_vma() and vma_priv(). They
work with different types but basically do the same: check if we should dump VMA into
the image and restore it back then.
Lets unify they both into common vma_entry_is_private() helper and vma_area_is_private()
for working with vma_area type.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
For some undiagnosed reason, this test hangs for me when using SIGINT. Since we
always want to kill this process anyway, use SIGKILL instead.
Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
*can* reads that it might fail but won't. Instead, this comment means that any
code added below it should not fail, because the network is unlocked and so the
restored process is exposed to the world.
Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
This is pure theoretical, especially in this particular case when we
actually want to (likely) free the unused memory. Still the code which
ignores potential error doesn't look good.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
1. parse_mountinfo_ent() mixes "return -1" and "goto err" on failure,
this looks confusing and inconsistent.
2. And buggy. It forgets to free(opt) if parse_mnt_flags() fails.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
The caller will do this on failure too. So this is unnecessary and wrong
because we do not nullify ->mountpoint.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
1. parse_mountinfo() forgets to free(fst) if parse_mountinfo_ent()
succeeds.
2. The usage of fst/r_fstype is ovecomplicated for no reason.
Just change the parse_mountinfo() paths to populate/use/free this
fsname unconditionally, and move the ownership to the caller. There
is no reason to check FSTYPE__UNSUPPORTED and/or fallback to ->name.
Better yet, we could even turn fsname into the local "char []" and
avoid %ms and free(), but then we would need to pass the length of
this buffer to parse_mountinfo_ent().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Preparation to simplify the review. parse_mountinfo() assumes that:
1. The "err:" block does all the necessary cleanups on failure.
This is wrong, see the next patch.
2. We can never skip the mountpoint.
This is true, but we are going to change this.
s/goto err/goto end/ in the main loop, add the "end:" label which inserts
the new mount_info into the list and then checks ret != 0 to figure out
whether we need to abort.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
This is required to use criu swrk in libcontainer.
v2: remove useless function declaration
allow to set inherit_fd only for swrk
v3: check swrk out of loop
Cc: Saied Kazemi <saied@google.com>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Andrew Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Use the format specifier PRIx64 instead of %lx to print uint64.
integer.
Reported-by: Mr Travis CI
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
This is logging routine, lets it not affect the global @errno.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Acked-by: Andrew Vagin <avagin@parallels.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
The __print_on_level routine may modify global @errno
variable which is inacceptable: this is logging routine
which must be transparent to the rest of the program code.
Thus save @errno in local @__errno variable and restore
it on return.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Acked-by: Andrew Vagin <avagin@parallels.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Here is a real example how it's used.
Cc: Tycho Andersen <tycho.andersen@canonical.com>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Tested-by: Tycho Andersen <tycho.andersen@canonical.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
validate_mounts() prints ->mnt_id in hex when it reports the failure.
This complicates the understanding because this ->mnt_id is printed as
decimal elsewhere, including /proc/$pid/mountinfo.
parse_mountinfo() adds "0x" at least and this is just pr_info(), but
lets change it too.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Andrew Vagin <avagin@openvz.org>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
parse_smaps() is too big for easy reading. In addition, we are
creating a new interface to get information about processes, which is
called taskdiag, so parse_smaps() will do only what it should do
accoding with the name. All other should be moved in a separate
functions which will be reused to work with task_diag.
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
parse_smaps() is too big for easy reading. In addition, we are
creating a new interface to get information about processes, which is
called taskdiag, so parse_smaps() will do only what it should do
accoding with the name. All other should be moved in a separate
functions which will be reused to work with task_diag.
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
For some reason it returns ENOENT sometime
when run in namespace. Need to figure out
why. Disable it for a while to continue
testing the rest.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
inotify_irmap creates files in /etc so it should be able to do
this from userns.
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
1. The -ERESTART_RESTARTBLOCK case in get_task_regs() depends on kernel
internals too much, and for no reason. We shouldn't rely on fact that
a) we are going to do sigreturn() and b) restore_sigcontext() always
sets restart_block->fn = do_no_restart_syscall which returns -EINTR.
Just change this code to enforce -EINTR after restore, this is what
we actually want until we teach criu to handle ERESTART_RESTARTBLOCK.
2. Add pr_warn() to make the potential bug-reports more understandable,
a sane application should handle -EINTR correctly but this is not
always the case.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
Acked-by: Andrew Vagin <avagin@parallels.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
/proc/pid/pagemap is protected by the global CAP_SYS_ADMIN, so
it can't be opened in a non-root userns.
00:34:48.260 Execute ns/user/static/cow01
...
00:34:48.260 12:37:03.951: 4: ERR: cow01.c:87: Unable to open child pagemap file /proc/5/pagemap (errno = 1 (Operation not permitted))
Reported-by: Mr Jenkins
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
The idea is simply make sure that timeour has not been
screwed during c/r cycle.
v2:
- Drop unneeded early poll() call.
- Make poll loop in cycle until caller passes c/r cycle.
v3:
- Use test_go helper
- Do poll in cycle
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Acked-by: Andrew Vagin <avagin@parallels.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
== is a bashism, we should use = for /bin/sh; this prevents output like:
/home/ubuntu/criu/test/post-dump.sh: 3: [: post-dump: unexpected operator
/home/ubuntu/criu/test/post-dump.sh: 3: [: network-unlock: unexpected operator
Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
We need to finalize the cg yard both on successful cgroup restore and on a
failed restore. Further, we should restore the cgroup properties before
allowing the task to continue in all modes (previously properties were only
restored correctly in --restore-detached mode).
CC: Saied Kazemi <saied@google.com>
Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
# Build criu in a docker container
$ make direct-build
# Execute tests in a docker container
$ make direct-test
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Vladimir said that a page can be unmapped from one process, but can be
mmaped in another one. In this case we can't understand whether both
processes link to the same page or they don't.
So we can try read both pages to take them from swap.
Cc: Vladimir Davydov <vdavydov@parallels.com>
Reported-by: Parallels' QA
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
cow01 uses /proc/pid/pagemap which has been protected by
CAP_SYS_ADMIN.
"""
commit ab676b7d6fbf4b294bf198fb27ade5b0e865c7ce
pagemap: do not leak physical addresses to non-privileged userspace
As pointed by recent post[1] on exploiting DRAM physical imperfection,
/proc/PID/pagemap exposes sensitive information which can be used to do
attacks.
"""
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Before the recent "x86_64,signal: Fix SS handling for signals delivered
to 64-bit programs" kernel patch, sigreturn paths forgot to restore ->ss
after return from the signal handler.
Now that the kernel was fixed, restore_gpregs() has to initialize ->ss
too, it is no longer ignored.
Note: this is the minimal fix. In the long term we probably should not
dump/restore the segment registers at all. We can use sigcontext filled
by the target kernel and modify the general-purpose regs.
Reported-and-tested-by: Andrey Wagin <avagin@gmail.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
Acked-by: Andrew Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
This is common for log files, when a number of processes
are writing something into one file. Currently, if someone
wrote something to log since dump happened, on restore criu
will complain about "File has bad size" and refuse to restore.
If file is opened with O_APPEND|O_WRONLY flags it is going to
jump to the EOF anyway.
v2, use O_ACCMODE and separate helper
Signed-off-by: Ruslan Kuprieiev <rkuprieiev@cloudlinux.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Print all cap values in case of test is failed.
Also check for capset() and capsget() return values.
Signed-off-by: Kirill Tkhai <ktkhai@parallels.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
If the --restore-detached command line option is not specified during
restore, CRIU should unmount and remove the temporary cgyard directory
tree before waiting for the restored process to exit. Otherwise, all
the temporary cgyard mount points will remain mounted and visible.
Signed-off-by: Saied Kazemi <saied@google.com>
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Currently on dump we generate too many image files, effectively
all the stuff from the GLOB set is created. The thing is that
sometimes some of created images can be empty (just contain the
magic number at the head). Thos images are useless and just
waste the space.
When applied after the "empty images" set, this introduces the
lazy images -- when we call open_image() the actual file is
only created (and the magic number is written into it) when the
very first object goes into it.
For example for the simplest test we have, then static/env00
one, the created image files are
core-7290.img
creds-7290.img
fdinfo-2.img
fs-7290.img
ids-7290.img
inventory.img
mm-7290.img
pagemap-7290.img
pages-1.img
pstree.img
reg-files.img
sigacts-7290.img
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
When an image of a certian type is not found, CRIU sometimes
fails, sometimes ignores this fact. I propose to ignore this
fact always and treat absent images and those containing no
objects inside (i.e. -- empty). If the latter code flow will
_need_ objects, then criu will fail later.
Why object will be explicitly required? For example, due to
restoring code reading the image with pb_read_one, w/o the
_eof suffix thus required the object to be in the image.
Another example is objects dependencies. E.g. fdinfo objects
require various files objects. So missing image files will
result in non-resolved searches later.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
When page-read fails to open the pagemap image it reports error.
One place (stacked page-reads) need to handle the absent images
case gracefully, so fix the return codes to make this check
work.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Current code doesn't make any difference between OPT and no-OPT
except for the message is printed or not in the open_image().
So this particular change changes nothing but the availability of
this message.
In the next patches I wil introduce "empty images" to deal with
the ENOENT situation in a more graceful manner.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
