[CRIU] [PATCH] core: Implement xgetpid()

Kirill Tkhai ktkhai at virtuozzo.com
Wed Mar 15 10:35:52 PDT 2017


GLibc's getpid() caches pids, and it's unreliable.
For example, the following program returns different
results between getpid() and syscall(__NR_getpid):

parent: pid=32369
parent: fork pid=32370
1)child: pid=32370
2)child: pid=32369

Fix that by using raw xgetpid() everywhere.

***

define _GNU_SOURCE
#include <sched.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <fcntl.h>
#include <errno.h>
#include <sys/syscall.h>
#include <sys/mman.h>
#include <signal.h>

int child(void *a)
{
        printf("1)child: pid=%d\n", syscall(__NR_getpid));
        printf("2)child: pid=%d\n", getpid());
        return 0;
}

int main(void)
{
        int stack_size = 2 * 1024 * 1024;
        char *stack = mmap(NULL, stack_size, PROT_WRITE | PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
        pid_t pid;
        if (stack == MAP_FAILED) {
                perror("Can't allocate stack");
                exit(1);
        }

        printf("parent: pid=%d\n", getpid());
        pid = clone(child, stack + stack_size, CLONE_VM | CLONE_FILES | SIGCHLD, NULL);
        printf("parent: fork pid=%d\n", pid);
}

Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
---
 criu/action-scripts.c        |    3 ++-
 criu/autofs.c                |    9 +++++----
 criu/cgroup.c                |    3 ++-
 criu/cr-check.c              |    9 +++++----
 criu/cr-dump.c               |    3 ++-
 criu/cr-restore.c            |    9 +++++----
 criu/cr-service.c            |    3 ++-
 criu/files.c                 |    3 ++-
 criu/image.c                 |    3 ++-
 criu/include/libc-wrappers.h |   12 ++++++++++++
 criu/kerndat.c               |    3 ++-
 criu/log.c                   |    5 +++--
 criu/mount.c                 |    5 +++--
 criu/namespaces.c            |    7 ++++---
 criu/pstree.c                |    7 ++++---
 criu/shmem.c                 |    3 ++-
 criu/sk-unix.c               |    3 ++-
 criu/tty.c                   |    3 ++-
 criu/util.c                  |    9 +++++----
 criu/vdso.c                  |    2 +-
 20 files changed, 67 insertions(+), 37 deletions(-)
 create mode 100644 criu/include/libc-wrappers.h

diff --git a/criu/action-scripts.c b/criu/action-scripts.c
index 4fa8e2843..d5bab291a 100644
--- a/criu/action-scripts.c
+++ b/criu/action-scripts.c
@@ -3,6 +3,7 @@
 #include <limits.h>
 #include <stdlib.h>
 
+#include "libc-wrappers.h"
 #include "cr_options.h"
 #include "common/list.h"
 #include "xmalloc.h"
@@ -62,7 +63,7 @@ static int run_shell_scripts(const char *action)
 	}
 
 	if (!(env_set & ENV_IMGDIR)) {
-		sprintf(image_dir, "/proc/%ld/fd/%d", (long) getpid(), get_service_fd(IMG_FD_OFF));
+		sprintf(image_dir, "/proc/%ld/fd/%d", (long) xgetpid(), get_service_fd(IMG_FD_OFF));
 		if (setenv("CRTOOLS_IMAGE_DIR", image_dir, 1)) {
 			pr_perror("Can't set CRTOOLS_IMAGE_DIR=%s", image_dir);
 			return -1;
diff --git a/criu/autofs.c b/criu/autofs.c
index a436e9fa9..43349fb67 100644
--- a/criu/autofs.c
+++ b/criu/autofs.c
@@ -16,6 +16,7 @@
 #include "crtools.h"
 #include "util.h"
 
+#include "libc-wrappers.h"
 #include "images/autofs.pb-c.h"
 
 #define AUTOFS_OPT_UNKNOWN	INT_MIN
@@ -295,7 +296,7 @@ static int autofs_revisit_options(struct mount_info *pm)
 		return -ENOMEM;
 	}
 
-	f = fopen_proc(getpid(), "mountinfo");
+	f = fopen_proc(xgetpid(), "mountinfo");
 	if (!f)
 		goto free_str;
 
@@ -787,12 +788,12 @@ static int autofs_post_open(struct file_desc *d, int fd)
 
 	if (i->entry->has_read_fd) {
 		pr_info("%s: pid %d, closing write end %d\n", __func__,
-				getpid(), i->entry->fd);
+				xgetpid(), i->entry->fd);
 		close(i->entry->fd);
 	}
 
 	pr_info("%s: pid %d, closing artificial pipe end %d\n", __func__,
-					getpid(), fd);
+					xgetpid(), fd);
 	close(fd);
 	return 0;
 }
@@ -940,7 +941,7 @@ static int autofs_add_mount_info(void *data)
 
 	if (entry->fd == -1)
 		/* Catatonic mounts have no owner. Keep them with init. */
-		master = pstree_item_by_virt(getpid());
+		master = pstree_item_by_virt(xgetpid());
 	else
 		master = pstree_item_by_virt(entry->pgrp);
 	BUG_ON(!master);
diff --git a/criu/cgroup.c b/criu/cgroup.c
index a1b3eb23f..36f0cbd4e 100644
--- a/criu/cgroup.c
+++ b/criu/cgroup.c
@@ -23,6 +23,7 @@
 #include "protobuf.h"
 #include "images/core.pb-c.h"
 #include "images/cgroup.pb-c.h"
+#include "libc-wrappers.h"
 
 /*
  * This structure describes set of controller groups
@@ -642,7 +643,7 @@ int dump_task_cgroup(const struct pstree_item *item, u32 *cg_id, struct parasite
 	if (item)
 		pid = item->pid->real;
 	else
-		pid = getpid();
+		pid = xgetpid();
 
 	pr_info("Dumping cgroups for %d\n", pid);
 	if (parse_task_cgroup(pid, args, &ctls, &n_ctls))
diff --git a/criu/cr-check.c b/criu/cr-check.c
index c8261255f..ca5e8de6d 100644
--- a/criu/cr-check.c
+++ b/criu/cr-check.c
@@ -49,6 +49,7 @@
 #include "cr_options.h"
 #include "libnetlink.h"
 #include "net.h"
+#include "libc-wrappers.h"
 #include "linux/userfaultfd.h"
 #include "restorer.h"
 
@@ -172,7 +173,7 @@ static int check_sock_peek_off(void)
 
 static int check_kcmp(void)
 {
-	int ret = syscall(SYS_kcmp, getpid(), -1, -1, -1, -1);
+	int ret = syscall(SYS_kcmp, xgetpid(), -1, -1, -1, -1);
 
 	if (ret < 0 && errno == ENOSYS) {
 		pr_perror("System call kcmp is not supported");
@@ -266,7 +267,7 @@ static int check_proc_stat(void)
 	struct proc_pid_stat stat;
 	int ret;
 
-	ret = parse_pid_stat(getpid(), &stat);
+	ret = parse_pid_stat(xgetpid(), &stat);
 	if (ret) {
 		pr_msg("procfs: stat extension is not supported\n");
 		return -1;
@@ -544,7 +545,7 @@ static int check_sigqueuinfo()
 
 	signal(SIGUSR1, SIG_IGN);
 
-	if (syscall(SYS_rt_sigqueueinfo, getpid(), SIGUSR1, &info) < 0) {
+	if (syscall(SYS_rt_sigqueueinfo, xgetpid(), SIGUSR1, &info) < 0) {
 		pr_perror("Unable to send siginfo with positive si_code to itself");
 		return -1;
 	}
@@ -1135,7 +1136,7 @@ int cr_check(void)
 	if (root_item == NULL)
 		return -1;
 
-	root_item->pid->real = getpid();
+	root_item->pid->real = xgetpid();
 
 	if (collect_pstree_ids())
 		return -1;
diff --git a/criu/cr-dump.c b/criu/cr-dump.c
index 5f1f668bb..be51b2095 100644
--- a/criu/cr-dump.c
+++ b/criu/cr-dump.c
@@ -32,6 +32,7 @@
 #include "images/file-lock.pb-c.h"
 #include "images/rlimit.pb-c.h"
 #include "images/siginfo.pb-c.h"
+#include "libc-wrappers.h"
 
 #include "common/list.h"
 #include "imgset.h"
@@ -786,7 +787,7 @@ static int collect_pstree_ids_predump(void)
 	 */
 
 	crt.i.pid->state = TASK_ALIVE;
-	crt.i.pid->real = getpid();
+	crt.i.pid->real = xgetpid();
 
 	if (predump_task_ns_ids(&crt.i))
 		return -1;
diff --git a/criu/cr-restore.c b/criu/cr-restore.c
index d0a970bc5..a9a4f3e4a 100644
--- a/criu/cr-restore.c
+++ b/criu/cr-restore.c
@@ -93,6 +93,7 @@
 #include "images/rlimit.pb-c.h"
 #include "images/pagemap.pb-c.h"
 #include "images/siginfo.pb-c.h"
+#include "libc-wrappers.h"
 
 #include "restore.h"
 
@@ -1200,7 +1201,7 @@ static void restore_sid(void)
 			exit(1);
 		}
 	} else {
-		sid = getsid(getpid());
+		sid = getsid(xgetpid());
 		if (sid != current->sid) {
 			/* Skip the root task if it's not init */
 			if (current == root_item && vpid(root_item) != INIT_PID)
@@ -1300,7 +1301,7 @@ static int create_children_and_session(void)
 		if (!restore_before_setsid(child))
 			continue;
 
-		BUG_ON(child->born_sid != -1 && getsid(getpid()) != child->born_sid);
+		BUG_ON(child->born_sid != -1 && getsid(xgetpid()) != child->born_sid);
 
 		ret = fork_with_pid(child);
 		if (ret < 0)
@@ -1348,7 +1349,7 @@ static int restore_task_with_children(void *_arg)
 			goto err;
 	}
 
-	pid = getpid();
+	pid = xgetpid();
 	if (vpid(current) != pid) {
 		pr_err("Pid %d do not match expected %d\n", pid, vpid(current));
 		set_task_cr_err(EEXIST);
@@ -1737,7 +1738,7 @@ static int prepare_userns_hook(void)
 	 * inside container due to permissions.
 	 * But you still can set this value if it was unset.
 	 */
-	saved_loginuid = parse_pid_loginuid(getpid(), &ret, false);
+	saved_loginuid = parse_pid_loginuid(xgetpid(), &ret, false);
 	if (ret < 0)
 		return -1;
 
diff --git a/criu/cr-service.c b/criu/cr-service.c
index 69e1a5557..5de91af0b 100644
--- a/criu/cr-service.c
+++ b/criu/cr-service.c
@@ -39,6 +39,7 @@
 #include <sys/un.h>
 #include <sys/socket.h>
 #include "common/scm.h"
+#include "libc-wrappers.h"
 
 #include "setproctitle.h"
 
@@ -1153,7 +1154,7 @@ int cr_service(bool daemon_mode)
 	}
 
 	if (opts.pidfile) {
-		if (write_pidfile(getpid()) == -1) {
+		if (write_pidfile(xgetpid()) == -1) {
 			pr_perror("Can't write pidfile");
 			goto err;
 		}
diff --git a/criu/files.c b/criu/files.c
index d68c7057b..8124db4c8 100644
--- a/criu/files.c
+++ b/criu/files.c
@@ -48,6 +48,7 @@
 #include "util.h"
 #include "images/fs.pb-c.h"
 #include "images/ext-file.pb-c.h"
+#include "libc-wrappers.h"
 
 #include "plugin.h"
 
@@ -638,7 +639,7 @@ int restore_fown(int fd, FownEntry *fown)
 {
 	struct f_owner_ex owner;
 	uid_t uids[3];
-	pid_t pid = getpid();
+	pid_t pid = xgetpid();
 
 	if (fown->signum) {
 		if (fcntl(fd, F_SETSIG, fown->signum)) {
diff --git a/criu/image.c b/criu/image.c
index 57d3c7b5f..615b1cc2a 100644
--- a/criu/image.c
+++ b/criu/image.c
@@ -17,6 +17,7 @@
 #include "images/inventory.pb-c.h"
 #include "images/pagemap.pb-c.h"
 #include "img-remote.h"
+#include "libc-wrappers.h"
 
 bool ns_per_id = false;
 bool img_common_magic = true;
@@ -135,7 +136,7 @@ int prepare_inventory(InventoryEntry *he)
 	}
 
 	crt.i.pid->state = TASK_ALIVE;
-	crt.i.pid->real = getpid();
+	crt.i.pid->real = xgetpid();
 	if (get_task_ids(&crt.i))
 		return -1;
 
diff --git a/criu/include/libc-wrappers.h b/criu/include/libc-wrappers.h
new file mode 100644
index 000000000..d38875433
--- /dev/null
+++ b/criu/include/libc-wrappers.h
@@ -0,0 +1,12 @@
+#ifndef __LIBC_WRAPPERS
+#define __LIBC_WRAPPERS
+
+#include <sys/syscall.h>
+#include <unistd.h>
+
+static inline int xgetpid()
+{
+	return (int)syscall(__NR_getpid);
+}
+
+#endif
diff --git a/criu/kerndat.c b/criu/kerndat.c
index 162ac28bd..205014bf3 100644
--- a/criu/kerndat.c
+++ b/criu/kerndat.c
@@ -31,6 +31,7 @@
 #include <compel/plugins/std/syscall-codes.h>
 #include <compel/compel.h>
 #include "linux/userfaultfd.h"
+#include "libc-wrappers.h"
 
 struct kerndat_s kdat = {
 };
@@ -289,7 +290,7 @@ int kerndat_get_dirty_track(void)
 	 * was at least once re-set. (this is to be removed in
 	 * a couple of kernel releases)
 	 */
-	ret = do_task_reset_dirty_track(getpid());
+	ret = do_task_reset_dirty_track(xgetpid());
 	if (ret < 0)
 		return ret;
 	if (ret == 1)
diff --git a/criu/log.c b/criu/log.c
index a2beabdb0..77abdb4f5 100644
--- a/criu/log.c
+++ b/criu/log.c
@@ -20,6 +20,7 @@
 #include "rst-malloc.h"
 #include "common/lock.h"
 #include "string.h"
+#include "libc-wrappers.h"
 
 #define DEFAULT_LOGFD		STDERR_FILENO
 /* Enable timestamps if verbosity is increased from default */
@@ -182,14 +183,14 @@ int log_init_by_pid(void)
 	reset_buf_off();
 
 	if (!opts.log_file_per_pid) {
-		buf_off += snprintf(buffer + buf_off, sizeof buffer - buf_off, "%6d: ", getpid());
+		buf_off += snprintf(buffer + buf_off, sizeof buffer - buf_off, "%6d: ", xgetpid());
 		return 0;
 	}
 
 	if (!opts.output)
 		return 0;
 
-	snprintf(path, PATH_MAX, "%s.%d", opts.output, getpid());
+	snprintf(path, PATH_MAX, "%s.%d", opts.output, xgetpid());
 
 	return log_init(path);
 }
diff --git a/criu/mount.c b/criu/mount.c
index 147001bcf..850d5bae3 100644
--- a/criu/mount.c
+++ b/criu/mount.c
@@ -28,6 +28,7 @@
 #include "external.h"
 
 #include "images/mnt.pb-c.h"
+#include "libc-wrappers.h"
 
 /*
  * Put a : in here since those are invalid on
@@ -1721,7 +1722,7 @@ static int apply_sb_flags(void *args, int fd, pid_t pid)
 
 	snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
 
-	if (pid != getpid() && switch_ns(pid, &mnt_ns_desc, &rst))
+	if (pid != xgetpid() && switch_ns(pid, &mnt_ns_desc, &rst))
 		return -1;
 
 	err = mount(NULL, path, NULL, MS_REMOUNT | flags, NULL);
@@ -2429,7 +2430,7 @@ static int rst_collect_local_mntns(enum ns_type typ)
 {
 	struct ns_id *nsid;
 
-	nsid = rst_new_ns_id(0, getpid(), &mnt_ns_desc, typ);
+	nsid = rst_new_ns_id(0, xgetpid(), &mnt_ns_desc, typ);
 	if (!nsid)
 		return -1;
 
diff --git a/criu/namespaces.c b/criu/namespaces.c
index a4c3063db..b89e70c4b 100644
--- a/criu/namespaces.c
+++ b/criu/namespaces.c
@@ -33,6 +33,7 @@
 #include "common/scm.h"
 #include "fdstore.h"
 #include "proc_parse.h"
+#include "libc-wrappers.h"
 
 static struct ns_desc *ns_desc_array[] = {
 	&net_ns_desc,
@@ -402,7 +403,7 @@ static unsigned int generate_ns_id(int pid, unsigned int kid, struct ns_desc *nd
 	if (nsid)
 		goto found;
 
-	if (pid != getpid()) {
+	if (pid != xgetpid()) {
 		type = NS_OTHER;
 		if (pid == root_item->pid->real) {
 			BUG_ON(root_ns_mask & nd->cflag);
@@ -1442,7 +1443,7 @@ static inline void unsc_msg_init(struct unsc_msg *m, uns_call_t *c,
 	ch->cmsg_type = SCM_CREDENTIALS;
 
 	ucred = (struct ucred *) CMSG_DATA(ch);
-	ucred->pid = getpid();
+	ucred->pid = xgetpid();
 	ucred->uid = getuid();
 	ucred->gid = getgid();
 
@@ -1564,7 +1565,7 @@ int __userns_call(const char *func_name, uns_call_t call, int flags,
 	}
 
 	if (!usernsd_pid)
-		return call(arg, fd, getpid());
+		return call(arg, fd, xgetpid());
 
 	sk = get_service_fd(USERNSD_SK);
 	pr_debug("uns: calling %s (%d, %x)\n", func_name, fd, flags);
diff --git a/criu/pstree.c b/criu/pstree.c
index f86b75a70..8d60d5349 100644
--- a/criu/pstree.c
+++ b/criu/pstree.c
@@ -22,6 +22,7 @@
 #include "protobuf.h"
 #include "images/pstree.pb-c.h"
 #include "crtools.h"
+#include "libc-wrappers.h"
 
 struct pstree_item *root_item;
 static struct rb_root pid_root_rb;
@@ -333,8 +334,8 @@ int dump_pstree(struct pstree_item *root_item)
 
 static int prepare_pstree_for_shell_job(void)
 {
-	pid_t current_sid = getsid(getpid());
-	pid_t current_gid = getpgid(getpid());
+	pid_t current_sid = getsid(xgetpid());
+	pid_t current_gid = getpgid(xgetpid());
 
 	struct pstree_item *pi;
 
@@ -651,7 +652,7 @@ static int prepare_pstree_ids(void)
 	struct pstree_item *item, *child, *helper, *tmp;
 	LIST_HEAD(helpers);
 
-	pid_t current_pgid = getpgid(getpid());
+	pid_t current_pgid = getpgid(xgetpid());
 
 	/*
 	 * Some task can be reparented to init. A helper task should be added
diff --git a/criu/shmem.c b/criu/shmem.c
index 21a9d8b9e..568279977 100644
--- a/criu/shmem.c
+++ b/criu/shmem.c
@@ -25,6 +25,7 @@
 #include "pstree.h"
 #include "protobuf.h"
 #include "images/pagemap.pb-c.h"
+#include "libc-wrappers.h"
 
 #ifndef SEEK_DATA
 #define SEEK_DATA	3
@@ -557,7 +558,7 @@ static int open_shmem(int pid, struct vma_area *vma)
 	}
 
 	if (f == -1) {
-		f = open_proc_rw(getpid(), "map_files/%lx-%lx",
+		f = open_proc_rw(xgetpid(), "map_files/%lx-%lx",
 				(unsigned long) addr,
 				(unsigned long) addr + si->size);
 		if (f < 0)
diff --git a/criu/sk-unix.c b/criu/sk-unix.c
index a44d9985c..d17c759c9 100644
--- a/criu/sk-unix.c
+++ b/criu/sk-unix.c
@@ -31,6 +31,7 @@
 
 #include "protobuf.h"
 #include "images/sk-unix.pb-c.h"
+#include "libc-wrappers.h"
 
 #undef	LOG_PREFIX
 #define LOG_PREFIX "sk unix: "
@@ -1005,7 +1006,7 @@ static int bind_unix_sk(int sk, struct unix_sk_info *ui)
 
 				pr_info("found duplicate unix socket bound at %s\n", addr.sun_path);
 
-				ret = snprintf(temp, sizeof(temp), "%s-%s-%d", addr.sun_path, "criu-temp", getpid());
+				ret = snprintf(temp, sizeof(temp), "%s-%s-%d", addr.sun_path, "criu-temp", xgetpid());
 				/* this shouldn't happen, since sun_addr is only 108 chars long */
 				if (ret < 0 || ret >= sizeof(temp)) {
 					pr_err("snprintf of %s failed?\n", addr.sun_path);
diff --git a/criu/tty.c b/criu/tty.c
index 0ff690c82..523491e0b 100644
--- a/criu/tty.c
+++ b/criu/tty.c
@@ -37,6 +37,7 @@
 
 #include "parasite-syscall.h"
 #include "parasite.h"
+#include "libc-wrappers.h"
 
 #include "pstree.h"
 #include "fdstore.h"
@@ -714,7 +715,7 @@ int tty_restore_ctl_terminal(struct file_desc *d, int fd)
 
 out:
 	pr_info("Restore session %d by %d tty (index %d)\n",
-		 info->tie->sid, (int)getpid(), index);
+		 info->tie->sid, (int)xgetpid(), index);
 
 	ret = tty_set_sid(slave);
 	if (!ret)
diff --git a/criu/util.c b/criu/util.c
index 9fd8ba285..0945f1929 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -50,6 +50,7 @@
 #include "cr_options.h"
 #include "servicefd.h"
 #include "cr-service.h"
+#include "libc-wrappers.h"
 #include "files.h"
 
 #include "cr-errno.h"
@@ -255,7 +256,7 @@ static inline void set_proc_self_fd(int fd)
 		close(open_proc_self_fd);
 
 	open_proc_self_fd = fd;
-	open_proc_self_pid = getpid();
+	open_proc_self_pid = xgetpid();
 }
 
 static inline int set_proc_pid_fd(int pid, int fd)
@@ -275,7 +276,7 @@ static inline int set_proc_pid_fd(int pid, int fd)
 static inline int get_proc_fd(int pid)
 {
 	if (pid == PROC_SELF) {
-		if (open_proc_self_fd != -1 && open_proc_self_pid != getpid()) {
+		if (open_proc_self_fd != -1 && open_proc_self_pid != xgetpid()) {
 			close(open_proc_self_fd);
 			open_proc_self_fd = -1;
 		}
@@ -399,7 +400,7 @@ int init_service_fd(void)
 	 * conflict with any 'real-life' ones
 	 */
 
-	if (syscall(__NR_prlimit64, getpid(), RLIMIT_NOFILE, NULL, &rlimit)) {
+	if (syscall(__NR_prlimit64, xgetpid(), RLIMIT_NOFILE, NULL, &rlimit)) {
 		pr_perror("Can't get rlimit");
 		return -1;
 	}
@@ -834,7 +835,7 @@ int vaddr_to_pfn(unsigned long vaddr, u64 *pfn)
 	off = (vaddr / page_size()) * sizeof(u64);
 	ret = pread(fd, pfn, sizeof(*pfn), off);
 	if (ret != sizeof(*pfn)) {
-		pr_perror("Can't read pme for pid %d", getpid());
+		pr_perror("Can't read pme for pid %d", xgetpid());
 		ret = -1;
 	} else {
 		*pfn &= PME_PFRAME_MASK;
diff --git a/criu/vdso.c b/criu/vdso.c
index 770853514..68d7be046 100644
--- a/criu/vdso.c
+++ b/criu/vdso.c
@@ -380,7 +380,7 @@ static void compat_vdso_helper(struct vdso_symtable *native, int pipe_fd,
 		exit_on(-1, err_fd, "Error: Compatible vdso's size is bigger than reserved buf\n");
 
 	/* Stop so CRIU could parse smaps to find 32-bit vdso's size */
-	ret = syscall(__NR_kill, syscall(__NR_getpid), SIGSTOP);
+	ret = syscall(__NR_kill, xgetpid(), SIGSTOP);
 	exit_on(ret, err_fd, "Error: Can't stop myself with SIGSTOP (having a good time)\n");
 
 	ret = syscall(__NR_read, pipe_fd, &vdso_addr, sizeof(void *));



More information about the CRIU mailing list