Commit e7ba9095 authored by Filipe Brandenburger's avatar Filipe Brandenburger Committed by Pavel Emelyanov

cr-check: Inspect errno on syscall failures

After replacing sys_kcmp with syscall() and sys_prctl with prctl(), the
API is changed to return -1 and set errno on failure (instead of
returning the negative value of the error code directly on return.)

Commit 8ceab588 ("crtools: no more linked with builtin syscall")
replaced calls to sys_kcmp and sys_prctl, but did not update the checks
for ret to check for -1 and errno, so update them here.

Also make the comparisons for the return value explicit checks for a
negative value (expected -1) instead of just failing on a non-zero value
with the implicit comparison.

Add %m to report what errno was set to on failed syscalls, to make it
easier to troubleshoot a check failure.

Tested that now `criu check --ms` stopped reporting the misleading:
  prctl: One needs CAP_SYS_RESOURCE capability to perform testing.
Instead, now it reports:
  Error (cr-check.c:165): System call kcmp is not supported: No such process
  Warn  (cr-check.c:195): Skipping unssuported PR_SET_MM_MAP: Invalid argument
  prctl: PR_SET_MM is not supported: Invalid argument
And those messages include the error message that explains why the
syscall failed.
Reported-by: 's avatarSaied Kazemi <saied@google.com>
Fixes: 8ceab588 ("crtools: no more linked with builtin syscall")
Cc: Laurent Dufour <ldufour@linux.vnet.ibm.com>
Signed-off-by: 's avatarFilipe Brandenburger <filbranden@google.com>
Signed-off-by: 's avatarPavel Emelyanov <xemul@virtuozzo.com>
parent a1b94a3d
...@@ -161,12 +161,12 @@ static int check_kcmp(void) ...@@ -161,12 +161,12 @@ static int check_kcmp(void)
{ {
int ret = syscall(SYS_kcmp, getpid(), -1, -1, -1, -1); int ret = syscall(SYS_kcmp, getpid(), -1, -1, -1, -1);
if (ret != -ENOSYS) if (ret < 0 && errno != ENOSYS) {
return 0; pr_perror("System call kcmp is not supported");
return -1;
}
errno = -ret; return 0;
pr_perror("System call kcmp is not supported");
return -1;
} }
static int check_prctl(void) static int check_prctl(void)
...@@ -177,8 +177,8 @@ static int check_prctl(void) ...@@ -177,8 +177,8 @@ static int check_prctl(void)
int ret; int ret;
ret = prctl(PR_GET_TID_ADDRESS, (unsigned long)&tid_addr, 0, 0, 0); ret = prctl(PR_GET_TID_ADDRESS, (unsigned long)&tid_addr, 0, 0, 0);
if (ret) { if (ret < 0) {
pr_msg("prctl: PR_GET_TID_ADDRESS is not supported"); pr_msg("prctl: PR_GET_TID_ADDRESS is not supported: %m");
return -1; return -1;
} }
...@@ -186,32 +186,32 @@ static int check_prctl(void) ...@@ -186,32 +186,32 @@ static int check_prctl(void)
* Either new or old interface must be supported in the kernel. * Either new or old interface must be supported in the kernel.
*/ */
ret = prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, (unsigned long)&size, 0, 0); ret = prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, (unsigned long)&size, 0, 0);
if (ret) { if (ret < 0) {
if (!opts.check_ms_kernel) { if (!opts.check_ms_kernel) {
pr_msg("prctl: PR_SET_MM_MAP is not supported, which " pr_msg("prctl: PR_SET_MM_MAP is not supported, which "
"is required for restoring user namespaces\n"); "is required for restoring user namespaces: %m\n");
return -1; return -1;
} else } else
pr_warn("Skipping unssuported PR_SET_MM_MAP\n"); pr_warn("Skipping unssuported PR_SET_MM_MAP: %m\n");
ret = prctl(PR_SET_MM, PR_SET_MM_BRK, brk(0), 0, 0); ret = prctl(PR_SET_MM, PR_SET_MM_BRK, brk(0), 0, 0);
if (ret) { if (ret < 0) {
if (ret == -EPERM) if (errno == EPERM)
pr_msg("prctl: One needs CAP_SYS_RESOURCE capability to perform testing\n"); pr_msg("prctl: One needs CAP_SYS_RESOURCE capability to perform testing\n");
else else
pr_msg("prctl: PR_SET_MM is not supported\n"); pr_msg("prctl: PR_SET_MM is not supported: %m\n");
return -1; return -1;
} }
ret = prctl(PR_SET_MM, PR_SET_MM_EXE_FILE, -1, 0, 0); ret = prctl(PR_SET_MM, PR_SET_MM_EXE_FILE, -1, 0, 0);
if (ret != -EBADF) { if (ret < 0 && errno != EBADF) {
pr_msg("prctl: PR_SET_MM_EXE_FILE is not supported (%d)\n", ret); pr_msg("prctl: PR_SET_MM_EXE_FILE is not supported: %m\n");
return -1; return -1;
} }
ret = prctl(PR_SET_MM, PR_SET_MM_AUXV, (long)&user_auxv, sizeof(user_auxv), 0); ret = prctl(PR_SET_MM, PR_SET_MM_AUXV, (long)&user_auxv, sizeof(user_auxv), 0);
if (ret) { if (ret < 0) {
pr_msg("prctl: PR_SET_MM_AUXV is not supported\n"); pr_msg("prctl: PR_SET_MM_AUXV is not supported: %m\n");
return -1; return -1;
} }
} }
...@@ -523,7 +523,7 @@ static int check_sigqueuinfo() ...@@ -523,7 +523,7 @@ static int check_sigqueuinfo()
signal(SIGUSR1, SIG_IGN); signal(SIGUSR1, SIG_IGN);
if (syscall(SYS_rt_sigqueueinfo, getpid(), SIGUSR1, &info)) { if (syscall(SYS_rt_sigqueueinfo, getpid(), SIGUSR1, &info) < 0) {
pr_perror("Unable to send siginfo with positive si_code to itself"); pr_perror("Unable to send siginfo with positive si_code to itself");
return -1; return -1;
} }
...@@ -744,7 +744,7 @@ static int check_aio_remap(void) ...@@ -744,7 +744,7 @@ static int check_aio_remap(void)
int r; int r;
if (syscall(SYS_io_setup, 16, &ctx) < 0) { if (syscall(SYS_io_setup, 16, &ctx) < 0) {
pr_err("No AIO syscall\n"); pr_err("No AIO syscall: %m\n");
return -1; return -1;
} }
...@@ -767,10 +767,10 @@ static int check_aio_remap(void) ...@@ -767,10 +767,10 @@ static int check_aio_remap(void)
r = syscall(SYS_io_getevents, ctx, 0, 1, NULL, NULL); r = syscall(SYS_io_getevents, ctx, 0, 1, NULL, NULL);
if (r < 0) { if (r < 0) {
if (!opts.check_ms_kernel) { if (!opts.check_ms_kernel) {
pr_err("AIO remap doesn't work properly\n"); pr_err("AIO remap doesn't work properly: %m\n");
return -1; return -1;
} else } else
pr_warn("Skipping unsupported AIO remap\n"); pr_warn("Skipping unsupported AIO remap: %m\n");
} }
return 0; return 0;
...@@ -925,8 +925,7 @@ static int check_userns(void) ...@@ -925,8 +925,7 @@ static int check_userns(void)
} }
ret = prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, (unsigned long)&size, 0, 0); ret = prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, (unsigned long)&size, 0, 0);
if (ret) { if (ret < 0) {
errno = -ret;
pr_perror("No new prctl API"); pr_perror("No new prctl API");
return -1; return -1;
} }
......
...@@ -168,9 +168,8 @@ static int dump_sched_info(int pid, ThreadCoreEntry *tc) ...@@ -168,9 +168,8 @@ static int dump_sched_info(int pid, ThreadCoreEntry *tc)
* in kernel. Thus we have to take it with us in the image. * in kernel. Thus we have to take it with us in the image.
*/ */
errno = 0;
ret = getpriority(PRIO_PROCESS, pid); ret = getpriority(PRIO_PROCESS, pid);
if (errno) { if (ret < 0) {
pr_perror("Can't get nice for %d", pid); pr_perror("Can't get nice for %d", pid);
return -1; return -1;
} }
...@@ -522,7 +521,7 @@ static int get_task_futex_robust_list(pid_t pid, ThreadCoreEntry *info) ...@@ -522,7 +521,7 @@ static int get_task_futex_robust_list(pid_t pid, ThreadCoreEntry *info)
int ret; int ret;
ret = syscall(SYS_get_robust_list, pid, &head, &len); ret = syscall(SYS_get_robust_list, pid, &head, &len);
if (ret == -ENOSYS) { if (ret < 0 && errno == ENOSYS) {
/* /*
* If the kernel says get_robust_list is not implemented, then * If the kernel says get_robust_list is not implemented, then
* check whether set_robust_list is also not implemented, in * check whether set_robust_list is also not implemented, in
...@@ -534,7 +533,8 @@ static int get_task_futex_robust_list(pid_t pid, ThreadCoreEntry *info) ...@@ -534,7 +533,8 @@ static int get_task_futex_robust_list(pid_t pid, ThreadCoreEntry *info)
* implemented, in which case it will return -EINVAL because * implemented, in which case it will return -EINVAL because
* len should be greater than zero. * len should be greater than zero.
*/ */
if (syscall(SYS_set_robust_list, NULL, 0) != -ENOSYS) ret = syscall(SYS_set_robust_list, NULL, 0);
if (ret == 0 || (ret < 0 && errno != ENOSYS))
goto err; goto err;
head = NULL; head = NULL;
......
...@@ -747,7 +747,6 @@ static int prepare_sigactions(void) ...@@ -747,7 +747,6 @@ static int prepare_sigactions(void)
*/ */
ret = syscall(SYS_rt_sigaction, sig, &act, NULL, sizeof(k_rtsigset_t)); ret = syscall(SYS_rt_sigaction, sig, &act, NULL, sizeof(k_rtsigset_t));
if (ret < 0) { if (ret < 0) {
errno = -ret;
pr_perror("Can't restore sigaction"); pr_perror("Can't restore sigaction");
goto err; goto err;
} }
......
...@@ -209,7 +209,6 @@ static int open_handle(unsigned int s_dev, unsigned long i_ino, ...@@ -209,7 +209,6 @@ static int open_handle(unsigned int s_dev, unsigned long i_ino,
fd = userns_call(open_by_handle, UNS_FDOUT, &handle, sizeof(handle), mntfd); fd = userns_call(open_by_handle, UNS_FDOUT, &handle, sizeof(handle), mntfd);
if (fd < 0) { if (fd < 0) {
errno = -fd;
pr_perror("Can't open file handle for 0x%08x:0x%016lx", pr_perror("Can't open file handle for 0x%08x:0x%016lx",
s_dev, i_ino); s_dev, i_ino);
} }
...@@ -718,7 +717,6 @@ static int open_fanotify_fd(struct file_desc *d) ...@@ -718,7 +717,6 @@ static int open_fanotify_fd(struct file_desc *d)
ret = fanotify_init(flags, info->ffe->evflags); ret = fanotify_init(flags, info->ffe->evflags);
if (ret < 0) { if (ret < 0) {
errno = -ret;
pr_perror("Can't init fanotify mark (%d)", ret); pr_perror("Can't init fanotify mark (%d)", ret);
return -1; return -1;
} }
......
...@@ -61,7 +61,7 @@ static inline void tcp_repair_off(int fd) ...@@ -61,7 +61,7 @@ static inline void tcp_repair_off(int fd)
ret = setsockopt(fd, SOL_TCP, TCP_REPAIR, &aux, sizeof(aux)); ret = setsockopt(fd, SOL_TCP, TCP_REPAIR, &aux, sizeof(aux));
if (ret < 0) if (ret < 0)
pr_err("Failed to turn off repair mode on socket (%d)\n", ret); pr_err("Failed to turn off repair mode on socket: %m\n");
} }
extern void tcp_locked_conn_add(struct inet_sk_info *); extern void tcp_locked_conn_add(struct inet_sk_info *);
......
...@@ -394,7 +394,7 @@ static bool kerndat_has_memfd_create(void) ...@@ -394,7 +394,7 @@ static bool kerndat_has_memfd_create(void)
else if (ret == -1 && errno == EFAULT) else if (ret == -1 && errno == EFAULT)
kdat.has_memfd = true; kdat.has_memfd = true;
else { else {
pr_err("Unexpected error %d from memfd_create(NULL, 0)\n", ret); pr_err("Unexpected error from memfd_create(NULL, 0): %m\n");
return -1; return -1;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment