lkml.org 
[lkml]   [2012]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC c/r 2/4] [RFC] syscalls, x86: Add __NR_kcmp syscall v7
On 1/27/2012 12:53 PM, Cyrill Gorcunov wrote:
> While doing the checkpoint-restore in the userspace one need to determine
> whether various kernel objects (like mm_struct-s of file_struct-s) are shared
> between tasks and restore this state.
>
> The 2nd step can be solved by using appropriate CLONE_ flags and the unshare
> syscall, while there's currently no ways for solving the 1st one.
>
> One of the ways for checking whether two tasks share e.g. mm_struct is to
> provide some mm_struct ID of a task to its proc file, but showing such
> info considered to be not that good for security reasons.
>
> Thus after some debates we end up in conclusion that using that named
> 'comparision' syscall might be the best candidate. So here is it --
> __NR_kcmp.
>
> It takes up to 5 agruments - the pids of the two tasks (which
> characteristics should be compared), the comparision type and
> (in case of comparision of files) two file descriptors.
>
> Lookups for pids are done in the caller's PID namespace only.
>
> At moment only x86 is supported and tested.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> CC: "Eric W. Biederman" <ebiederm@xmission.com>
> CC: Pavel Emelyanov <xemul@parallels.com>
> CC: Andrey Vagin <avagin@openvz.org>
> CC: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
> CC: Ingo Molnar <mingo@elte.hu>
> CC: H. Peter Anvin <hpa@zytor.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Glauber Costa <glommer@parallels.com>
> CC: Andi Kleen <andi@firstfloor.org>
> CC: Tejun Heo <tj@kernel.org>
> CC: Matt Helsley <matthltc@us.ibm.com>
> CC: Pekka Enberg <penberg@kernel.org>
> CC: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Vasiliy Kulikov <segoon@openwall.com>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Alexey Dobriyan <adobriyan@gmail.com>
> CC: Valdis.Kletnieks@vt.edu
> ---
> arch/x86/syscalls/syscall_32.tbl | 1
> arch/x86/syscalls/syscall_64.tbl | 1
> include/linux/kcmp.h | 17 ++
> include/linux/syscalls.h | 2
> kernel/Makefile | 1
> kernel/kcmp.c | 181 +++++++++++++++++++++++++++++++
> tools/testing/selftests/kcmp/Makefile | 35 +++++
> tools/testing/selftests/kcmp/kcmp_test.c | 126 +++++++++++++++++++++
> tools/testing/selftests/run_tests | 2
> 9 files changed, 365 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.git/arch/x86/syscalls/syscall_32.tbl
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/syscalls/syscall_32.tbl
> +++ linux-2.6.git/arch/x86/syscalls/syscall_32.tbl
> @@ -355,3 +355,4 @@
> 346 i386 setns sys_setns
> 347 i386 process_vm_readv sys_process_vm_readv compat_sys_process_vm_readv
> 348 i386 process_vm_writev sys_process_vm_writev compat_sys_process_vm_writev
> +349 i386 kcmp sys_kcmp
> Index: linux-2.6.git/arch/x86/syscalls/syscall_64.tbl
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/syscalls/syscall_64.tbl
> +++ linux-2.6.git/arch/x86/syscalls/syscall_64.tbl
> @@ -318,3 +318,4 @@
> 309 64 getcpu sys_getcpu
> 310 64 process_vm_readv sys_process_vm_readv
> 311 64 process_vm_writev sys_process_vm_writev
> +312 64 kcmp sys_kcmp
> Index: linux-2.6.git/include/linux/kcmp.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6.git/include/linux/kcmp.h
> @@ -0,0 +1,17 @@
> +#ifndef _LINUX_KCMP_H
> +#define _LINUX_KCMP_H
> +
> +/* Comparision type */
> +enum kcmp_type {
> + KCMP_FILE,
> + KCMP_VM,
> + KCMP_FILES,
> + KCMP_FS,
> + KCMP_SIGHAND,
> + KCMP_IO,
> + KCMP_SYSVSEM,
> +
> + KCMP_TYPES,
> +};
> +
> +#endif /* _LINUX_KCMP_H */
> Index: linux-2.6.git/include/linux/syscalls.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/syscalls.h
> +++ linux-2.6.git/include/linux/syscalls.h
> @@ -857,4 +857,6 @@ asmlinkage long sys_process_vm_writev(pi
> unsigned long riovcnt,
> unsigned long flags);
>
> +asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
> + unsigned long idx1, unsigned long idx2);
> #endif
> Index: linux-2.6.git/kernel/Makefile
> ===================================================================
> --- linux-2.6.git.orig/kernel/Makefile
> +++ linux-2.6.git/kernel/Makefile
> @@ -25,6 +25,7 @@ endif
> obj-y += sched/
> obj-y += power/
>
> +obj-$(CONFIG_X86) += kcmp.o
> obj-$(CONFIG_FREEZER) += freezer.o
> obj-$(CONFIG_PROFILING) += profile.o
> obj-$(CONFIG_SYSCTL_SYSCALL_CHECK) += sysctl_check.o
> Index: linux-2.6.git/kernel/kcmp.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6.git/kernel/kcmp.c
> @@ -0,0 +1,181 @@
> +#include <linux/kernel.h>
> +#include <linux/syscalls.h>
> +#include <linux/fdtable.h>
> +#include <linux/string.h>
> +#include <linux/random.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/cache.h>
> +#include <linux/bug.h>
> +#include <linux/err.h>
> +#include <linux/kcmp.h>
> +
> +#include <asm/unistd.h>
> +
> +/*
> + * We don't expose real in-memory order of objects for security
> + * reasons, still the comparision results should be suitable for
> + * sorting. Thus, we obfuscate kernel pointers values (using random
> + * cookies obtaned at early boot stage) and compare the production
> + * instead.
> + */
> +static unsigned long cookies[KCMP_TYPES][2] __read_mostly;
> +
> +static long kptr_obfuscate(long v, int type)
> +{
> + return (v ^ cookies[type][0]) * cookies[type][1];
> +}
> +
> +/*
> + * 0 - equal
> + * 1 - less than
> + * 2 - greater than
> + * 3 - not equal but ordering unavailable (reserved for future)
> + */
> +static int kcmp_ptr(long v1, long v2, enum kcmp_type type)
> +{
> + long ret;
> +
> + ret = kptr_obfuscate(v1, type) - kptr_obfuscate(v2, type);
> +
> + return (ret < 0) | ((ret > 0) << 1);
> +}
> +
> +/* The caller must have pinned the task */
> +static struct file *
> +get_file_raw_ptr(struct task_struct *task, unsigned int idx)
> +{
> + struct fdtable *fdt;
> + struct file *file;
> +
> + spin_lock(&task->files->file_lock);
> + fdt = files_fdtable(task->files);
> + if (idx < fdt->max_fds)
> + file = fdt->fd[idx];
> + else
> + file = NULL;
> + spin_unlock(&task->files->file_lock);
> +
> + return file;
> +}
> +
> +SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type,
> + unsigned long, idx1, unsigned long, idx2)
> +{
> + struct task_struct *task1, *task2;
> + int ret;
> +
> + rcu_read_lock();
> +
> + /*
> + * Tasks are looked up in caller's
> + * PID namespace only.
> + */
> +
> + task1 = find_task_by_vpid(pid1);
> + if (!task1) {
> + rcu_read_unlock();
> + return -ESRCH;
> + }
> +
> + task2 = find_task_by_vpid(pid2);
> + if (!task2) {
> + put_task_struct(task1);
> + rcu_read_unlock();
> + return -ESRCH;
> + }
> +
> + get_task_struct(task1);
> + get_task_struct(task2);
> +
> + rcu_read_unlock();
> +
> + /*
> + * One should have enough rights to inspect tasks details.
> + */
> + if (!ptrace_may_access(task1, PTRACE_MODE_READ) ||
> + !ptrace_may_access(task2, PTRACE_MODE_READ)) {
> + ret = -EACCES;
> + goto err;
> + }
> +
> + /*
> + * Note for all cases but the KCMP_FILE we
> + * don't take any locks in a sake of speed.
> + */
> +
> + switch (type) {
> + case KCMP_FILE: {
> + struct file *filp1, *filp2;
> +
> + filp1 = get_file_raw_ptr(task1, idx1);
> + filp2 = get_file_raw_ptr(task2, idx2);
> +
> + if (filp1 && filp2)
> + ret = kcmp_ptr((long)filp1, (long)filp2, KCMP_FILE);
> + else
> + ret = -ENOENT;

If my remember is correct, Andrew pointed out EINVAL is better than ENOENT.


> + break;
> + }
> + case KCMP_VM:
> + ret = kcmp_ptr((long)task1->mm,
> + (long)task2->mm,
> + KCMP_VM);
> + break;
> + case KCMP_FILES:
> + ret = kcmp_ptr((long)task1->files,
> + (long)task2->files,
> + KCMP_FILES);
> + break;
> + case KCMP_FS:
> + ret = kcmp_ptr((long)task1->fs,
> + (long)task2->fs,
> + KCMP_FS);
> + break;
> + case KCMP_SIGHAND:
> + ret = kcmp_ptr((long)task1->sighand,
> + (long)task2->sighand,
> + KCMP_SIGHAND);
> + break;
> + case KCMP_IO:
> + ret = kcmp_ptr((long)task1->io_context,
> + (long)task2->io_context,
> + KCMP_IO);
> + break;
> + case KCMP_SYSVSEM:
> +#ifdef CONFIG_SYSVIPC
> + ret = kcmp_ptr((long)task1->sysvsem.undo_list,
> + (long)task2->sysvsem.undo_list,
> + KCMP_SYSVSEM);
> +#else
> + ret = -EINVAL;

ENOTSUP is better, I think. because of, EINVAL implicitly mean _caller_ is wrong.
but in this case, it is not bad. only the kernel doesn't have enough feature.


> + goto err;

you don't need err label at all.


> +#endif
> + break;
> + default:
> + ret = -EINVAL;
> + goto err;
> + }
> +
> +err:
> + put_task_struct(task1);
> + put_task_struct(task2);
> +
> + return ret;
> +}
> +
> +static __init int kcmp_cookie_init(void)
> +{
> + int i, j;
> +
> + for (i = 0; i < KCMP_TYPES; i++) {
> + for (j = 0; j < 2; j++) {
> + get_random_bytes(&cookies[i][j],
> + sizeof(cookies[i][j]));
> + }
> + cookies[i][1] |= (~(~0UL >> 1) | 1);
> + }
> +
> + return 0;
> +}
> +late_initcall(kcmp_cookie_init);
> Index: linux-2.6.git/tools/testing/selftests/kcmp/Makefile
> ===================================================================
> --- /dev/null
> +++ linux-2.6.git/tools/testing/selftests/kcmp/Makefile
> @@ -0,0 +1,35 @@
> +ifeq ($(strip $(V)),)
> + E = @echo
> + Q = @
> +else
> + E = @\#
> + Q =
> +endif
> +export E Q
> +
> +uname_M := $(shell uname -m 2>/dev/null || echo not)
> +ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/)
> +ifeq ($(ARCH),i386)
> + ARCH := X86
> + CFLAGS := -DCONFIG_X86_32
> +endif
> +ifeq ($(ARCH),x86_64)
> + ARCH := X86
> + CFLAGS := -DCONFIG_X86_64
> +endif
> +
> +CFLAGS += -I../../../../arch/x86/include/generated/
> +CFLAGS += -I../../../../include/
> +
> +all:
> +ifeq ($(ARCH),X86)
> + $(E) " CC run_test"
> + $(Q) gcc $(CFLAGS) kcmp_test.c -o run_test
> +else
> + $(E) "Not an x86 target, can't build breakpoints selftests"
> +endif
> +
> +clean:
> + $(E) " CLEAN"
> + $(Q) rm -fr ./run_test
> + $(Q) rm -fr ./test-file
> Index: linux-2.6.git/tools/testing/selftests/kcmp/kcmp_test.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6.git/tools/testing/selftests/kcmp/kcmp_test.c
> @@ -0,0 +1,126 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <limits.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <fcntl.h>
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/wait.h>
> +
> +#ifdef CONFIG_X86_64
> +#include "asm/unistd_64.h"
> +#else
> +#include "asm/unistd_32.h"
> +#endif
> +
> +#include "linux/kcmp.h"
> +
> +#ifdef CONFIG_X86_64
> +static long syscall5(int nr, unsigned long arg0, unsigned long arg1,
> + unsigned long arg2, unsigned long arg3,
> + unsigned long arg4)
> +{
> + long ret;
> + asm volatile(
> + "movl %1, %%eax \n"
> + "movq %2, %%rdi \n"
> + "movq %3, %%rsi \n"
> + "movq %4, %%rdx \n"
> + "movq %5, %%r10 \n"
> + "movq %6, %%r8 \n"
> + "syscall \n"
> + "movq %%rax, %0 \n"
> + : "=r"(ret)
> + : "g" ((int)nr), "g" (arg0), "g" (arg1), "g" (arg2),
> + "g" (arg3), "g" (arg4)
> + : "rax", "rdi", "rsi", "rdx", "r10", "r8", "memory");
> + return ret;
> +}
> +#else
> +static long syscall5(int nr, unsigned long arg0, unsigned long arg1,
> + unsigned long arg2, unsigned long arg3,
> + unsigned long arg4)
> +{
> + long ret;
> + asm volatile(
> + "movl %1, %%eax \n"
> + "movl %2, %%ebx \n"
> + "movl %3, %%ecx \n"
> + "movl %4, %%edx \n"
> + "movl %5, %%esi \n"
> + "movl %6, %%edi \n"
> + "intl $0x80 \n"
> + "movq %%eax, %0 \n"
> + : "=r"(ret)
> + : "g" ((int)nr), "g" (arg0), "g" (arg1), "g" (arg2),
> + "g" (arg3), "g" (arg4)
> + : "eax", "ebx", "ecx", "edx", "esi", "edi", "memory");
> + return ret;
> +}
> +#endif
> +
> +static long sys_kcmp(int pid1, int pid2, int type, int fd1, int fd2)
> +{
> + return syscall5(__NR_kcmp, (long)pid1, (long)pid2,
> + (long)type, (long)fd1, (long)fd2);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int pid1, pid2;
> + int fd1, fd2;
> + int status;
> +
> + fd1 = open("test-file", O_RDWR | O_CREAT | O_TRUNC, 0644);
> + pid1 = getpid();
> +
> + if (fd1 < 0) {
> + perror("Can't create file");
> + exit(1);
> + }
> +
> + pid2 = fork();
> + if (pid2 < 0) {
> + perror("fork failed");
> + exit(1);
> + }
> +
> + if (!pid2) {
> + int pid2 = getpid();
> +
> + fd2 = open("test-file", O_RDWR, 0644);
> + if (fd2 < 0) {
> + perror("Can't open file");
> + exit(1);
> + }
> +
> + printf("pid1: %6d pid2: %6d FD: %2d FILES: %2d VM: %2d FS: %2d "
> + "SIGHAND: %2d IO: %2d SYSVSEM: %2d INV: %2d\n",
> + pid1, pid2,
> + sys_kcmp(pid1, pid2, KCMP_FILE, fd1, fd2),
> + sys_kcmp(pid1, pid2, KCMP_FILES, 0, 0),
> + sys_kcmp(pid1, pid2, KCMP_VM, 0, 0),
> + sys_kcmp(pid1, pid2, KCMP_FS, 0, 0),
> + sys_kcmp(pid1, pid2, KCMP_SIGHAND, 0, 0),
> + sys_kcmp(pid1, pid2, KCMP_IO, 0, 0),
> + sys_kcmp(pid1, pid2, KCMP_SYSVSEM, 0, 0),.

The best practice of auto test is

AssertFooBar(expected_value, actual_value);

and, just only print "correct or not". Only you know the correct value.



> +
> + /* This one should fail */
> + sys_kcmp(pid1, pid2, KCMP_TYPES + 1, 0, 0));
> +
> + /* This one should return same fd */
> + printf("pid1: %6d pid2: %6d SAME-FD: %2d\n",
> + pid1, pid2,
> + sys_kcmp(pid1, pid2, KCMP_FILE, fd1, fd1));
> +
> + exit(0);
> + }
> +
> + waitpid(pid2, &status, P_ALL);
> +
> + return 0;
> +}
> Index: linux-2.6.git/tools/testing/selftests/run_tests
> ===================================================================
> --- linux-2.6.git.orig/tools/testing/selftests/run_tests
> +++ linux-2.6.git/tools/testing/selftests/run_tests
> @@ -1,6 +1,6 @@
> #!/bin/bash
>
> -TARGETS=breakpoints
> +TARGETS="breakpoints kcmp"
>
> for TARGET in $TARGETS
> do
>
>



\
 
 \ /
  Last update: 2012-01-27 21:21    [W:0.130 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site