[RFC 0/3] ABI spec - verification

[RFC 0/3] ABI spec - verification

[alexander.levin]

As discussed at plumbers, having a standard spec for the kernel's ABI has
quite a few uses and enough people wanted it to get the ball rolling.

We agreed that it's desirable to have something that can be used from code
rather than just a spec on paper both for validation and allowing other users
(like fuzzers, userspace libraries, and various userspace tools) to build
on that.

What we ended up deciding on at plumbers is:

 - I'll do a few kernel bits do demonstrate how we can validate the spec from
the kernel.
 - Dmitry Vyukov will provide a way to translate syzkaller's syscall
documentation into something that can be easily used in the kernel and
userspace.
 - Various projects will attempt to integrate it to make sure that the
framework works for them.

Once those bits are done we can focus on getting the spec right, and we'll
have a good way to validate our work both in userspace and in the kernel.

This patchset is a basic draft of said kernel bits. I mostly want to make
sure that Dmitry and myself are on the same page as to how integration will
look like, but also to open it to criticism and suggestions (bikeshedding).


Sasha Levin (3):
  abi_spec: basic definitions of constraints, args and syscalls
  abi_spec: hooks into syscall to allow pre and post checking
  abi_spec: example spec for open(), placeholder for rest of syscalls

 include/linux/syscalls.h      |   7 +-
 include/uapi/linux/abi_spec.h |  58 ++++++
 kernel/Makefile               |   2 +
 kernel/abi_spec.c             | 456 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 522 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/abi_spec.h
 create mode 100644 kernel/abi_spec.c

-- 
2.7.4

[RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[alexander.levin]

This is a very simple definition of the syscall ABI we can build on. The idea
is to have a generic description of syscalls, their arguments and return
values we can use to audit the kernel's implementation vs the specs.

Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 include/uapi/linux/abi_spec.h | 58 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 include/uapi/linux/abi_spec.h

diff --git a/include/uapi/linux/abi_spec.h b/include/uapi/linux/abi_spec.h
new file mode 100644
index 0000000..ad1a992
--- /dev/null
+++ b/include/uapi/linux/abi_spec.h
@@ -0,0 +1,58 @@
+#ifndef ABI_SPEC_H_
+#define ABI_SPEC_H_
+
+#include <linux/fcntl.h>
+#include <linux/stat.h>
+#define MAX_CONSTRAINTS 10
+#define MAX_ARGS 10
+
+#define	TYPE_FD		1
+#define TYPE_INT	2
+#define TYPE_PTR	3
+#define TYPE_STRING	4
+/* ... */
+
+#define CONSTRAINT_NON_NULL	(1<<0)
+#define CONSTRAINT_RANGE	(1<<1)
+#define CONSTRAINT_ADDRESS_TYPE	(1<<2)
+#define CONSTRAINT_FD_TYPE	(1<<3)
+#define CONSTRAINT_ERRNO	(1<<4)
+#define CONSTRAINT_BITMASK	(1<<5)
+#define CONSTRAINT_PATH		(1<<6)
+/* ... */
+/* A generic constraint on an argument or a return value */
+struct constraint {
+	unsigned int flags;		/* bitmask of applied constraints */
+	union {
+		struct {		/* int range */
+			int int_min;
+			int int_max;
+		};
+		unsigned long bitmask;	/* allowed flags bitmask */
+		unsigned long address_flags;	/* Type of allowed addr */
+		unsigned long fd_flags;	/* Type of allowed fd */
+	};
+};
+
+/* A generic argument (or return value) */
+struct argument {
+	const char *name;
+	int type;			/* should be a nicer way to do this */
+
+	unsigned int nconstraints;	/* can there be more than 1-2? */
+	struct constraint constraints[MAX_CONSTRAINTS];
+};
+
+/* A generic syscall */
+struct syscall_spec {
+	const char *name;
+	struct argument retval;
+
+	unsigned int nargs;
+	struct argument args[MAX_ARGS];
+};
+
+void abispec_check_pre(const struct syscall_spec *s, ...);
+void abispec_check_post(const struct syscall_spec *s, long retval, ...);
+
+#endif
-- 
2.7.4

[RFC 2/3] abi_spec: hooks into syscall to allow pre and post checking

[alexander.levin]

This is a simple way to be able to verify syscall parameters before the
call to the actual syscall, and also verify the return value after the
call.

Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 include/linux/syscalls.h |  7 ++++++-
 kernel/Makefile          |  2 ++
 kernel/abi_spec.c        | 15 +++++++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 kernel/abi_spec.c

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 91a740f..6aa3228 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -79,6 +79,7 @@ union bpf_attr;
 #include <linux/quota.h>
 #include <linux/key.h>
 #include <trace/syscall.h>
+#include <linux/abi_spec.h>
 
 /*
  * __MAP - apply a macro to syscall arguments
@@ -192,13 +193,17 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 
 #define __PROTECT(...) asmlinkage_protect(__VA_ARGS__)
 #define __SYSCALL_DEFINEx(x, name, ...)					\
+	extern const struct syscall_spec syscall_spec##name;		\
 	asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))	\
 		__attribute__((alias(__stringify(SyS##name))));		\
 	static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
 	asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
 	asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{								\
-		long ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));	\
+		long ret;						\
+		abispec_check_pre(&syscall_spec##name, __MAP(x,__SC_CAST,__VA_ARGS__)); \
+		ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));	\
+		abispec_check_post(&syscall_spec##name, ret, __MAP(x,__SC_CAST,__VA_ARGS__)); \
 		__MAP(x,__SC_TEST,__VA_ARGS__);				\
 		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));	\
 		return ret;						\
diff --git a/kernel/Makefile b/kernel/Makefile
index eb26e12c..d94a1f9 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -113,6 +113,8 @@ obj-$(CONFIG_MEMBARRIER) += membarrier.o
 
 obj-$(CONFIG_HAS_IOMEM) += memremap.o
 
+obj-y += abi_spec.o
+
 $(obj)/configs.o: $(obj)/config_data.h
 
 # config_data.h contains the same information as ikconfig.h but gzipped.
diff --git a/kernel/abi_spec.c b/kernel/abi_spec.c
new file mode 100644
index 0000000..7933c45
--- /dev/null
+++ b/kernel/abi_spec.c
@@ -0,0 +1,15 @@
+#include <linux/kernel.h>
+#include <linux/abi_spec.h>
+#include <linux/limits.h>
+#include <linux/uaccess.h>
+
+void abispec_check_pre(const struct syscall_spec *s, ...)
+{
+}
+EXPORT_SYMBOL_GPL(abispec_check_pre);
+
+void abispec_check_post(const struct syscall_spec *s, long retval, ...)
+{
+}
+EXPORT_SYMBOL_GPL(abispec_check_post);
+
-- 
2.7.4

[RFC 3/3] abi_spec: example spec for open(), placeholder for rest of syscalls

[alexander.levin]

Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 kernel/abi_spec.c | 441 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 441 insertions(+)

diff --git a/kernel/abi_spec.c b/kernel/abi_spec.c
index 7933c45..9378a533 100644
--- a/kernel/abi_spec.c
+++ b/kernel/abi_spec.c
@@ -3,8 +3,52 @@
 #include <linux/limits.h>
 #include <linux/uaccess.h>
 
+static void handle_fd(const struct syscall_spec *s, unsigned int i, int fd)
+{
+}
+
+static void handle_int(const struct syscall_spec *s, unsigned int i, int fd)
+{
+}
+
+static void handle_ptr(const struct syscall_spec *s, unsigned int i, const void *p)
+{
+}
+
+static void handle_str(const struct syscall_spec *s, unsigned int i, const char __user *str)
+{
+	if (s->args[i].nconstraints && s->args[i].constraints[0].flags && CONSTRAINT_PATH) {
+		static char kpath[PATH_MAX]; /* checking its readable for now */
+
+		WARN_ON(strncpy_from_user(kpath, str, PATH_MAX) < 0);
+	}
+}
+
 void abispec_check_pre(const struct syscall_spec *s, ...)
 {
+	va_list ap;
+	unsigned int i;
+
+	va_start(ap, s);
+
+	for (i = 0; i < s->nargs; i++) {
+		switch (s->args[i].type) {
+		case TYPE_FD:
+			handle_fd(s, i, va_arg(ap, int));
+			break;
+		case TYPE_INT:
+			handle_int(s, i, va_arg(ap, int));
+			break;
+		case TYPE_PTR:
+			handle_ptr(s, i, va_arg(ap, void *));
+			break;
+		case TYPE_STRING:
+			handle_str(s, i, va_arg(ap, char *));
+			break;
+		}
+	}
+
+	va_end(ap);
 }
 EXPORT_SYMBOL_GPL(abispec_check_pre);
 
@@ -13,3 +57,400 @@ void abispec_check_post(const struct syscall_spec *s, long retval, ...)
 }
 EXPORT_SYMBOL_GPL(abispec_check_post);
 
+struct syscall_spec syscall_spec_open = {
+	.name = "open",
+	.retval = {	.name = "retval", .type = TYPE_INT,
+			.nconstraints = 1,
+			.constraints[0] = { .flags = CONSTRAINT_FD_TYPE | CONSTRAINT_ERRNO, }, },
+
+	.nargs = 3,
+	.args[0] = {	.name = "pathname", .type = TYPE_STRING,
+			.nconstraints = 1,
+			.constraints[0] = { .flags = CONSTRAINT_PATH | CONSTRAINT_NON_NULL, }, },
+	.args[1] = {	.name = "flags", .type = TYPE_INT,
+			.nconstraints = 1,
+			.constraints[0] = { .flags = CONSTRAINT_BITMASK,
+			.bitmask = O_APPEND | O_CREAT | O_CLOEXEC /* ... */, }, },
+	.args[2] = {	.name = "mode",	.type = TYPE_INT,
+			.constraints[0] = { .flags = CONSTRAINT_BITMASK,
+			.bitmask = S_IRWXU | S_IRUSR /* .... */, }, },
+};
+
+struct syscall_spec syscall_spec_read = { .name = "read" };
+struct syscall_spec syscall_spec_write = { .name = "write" };
+struct syscall_spec syscall_spec_close = { .name = "close" };
+struct syscall_spec syscall_spec_stat = { .name = "stat" };
+struct syscall_spec syscall_spec_fstat = { .name = "fstat" };
+struct syscall_spec syscall_spec_lstat = { .name = "lstat" };
+struct syscall_spec syscall_spec_poll = { .name = "poll" };
+struct syscall_spec syscall_spec_lseek = { .name = "lseek" };
+struct syscall_spec syscall_spec_mmap = { .name = "mmap" };
+struct syscall_spec syscall_spec_mprotect = { .name = "mprotect" };
+struct syscall_spec syscall_spec_munmap = { .name = "munmap" };
+struct syscall_spec syscall_spec_brk = { .name = "brk" };
+struct syscall_spec syscall_spec_rt_sigaction = { .name = "rt_sigaction" };
+struct syscall_spec syscall_spec_rt_sigprocmask = { .name = "rt_sigprocmask" };
+struct syscall_spec syscall_spec_rt_sigreturn = { .name = "rt_sigreturn" };
+struct syscall_spec syscall_spec_ioctl = { .name = "ioctl" };
+struct syscall_spec syscall_spec_pread64 = { .name = "pread64" };
+struct syscall_spec syscall_spec_pwrite64 = { .name = "pwrite64" };
+struct syscall_spec syscall_spec_readv = { .name = "readv" };
+struct syscall_spec syscall_spec_writev = { .name = "writev" };
+struct syscall_spec syscall_spec_access = { .name = "access" };
+struct syscall_spec syscall_spec_pipe = { .name = "pipe" };
+struct syscall_spec syscall_spec_select = { .name = "select" };
+struct syscall_spec syscall_spec_sched_yield = { .name = "sched_yield" };
+struct syscall_spec syscall_spec_mremap = { .name = "mremap" };
+struct syscall_spec syscall_spec_msync = { .name = "msync" };
+struct syscall_spec syscall_spec_mincore = { .name = "mincore" };
+struct syscall_spec syscall_spec_madvise = { .name = "madvise" };
+struct syscall_spec syscall_spec_shmget = { .name = "shmget" };
+struct syscall_spec syscall_spec_shmat = { .name = "shmat" };
+struct syscall_spec syscall_spec_shmctl = { .name = "shmctl" };
+struct syscall_spec syscall_spec_dup = { .name = "dup" };
+struct syscall_spec syscall_spec_dup2 = { .name = "dup2" };
+struct syscall_spec syscall_spec_pause = { .name = "pause" };
+struct syscall_spec syscall_spec_nanosleep = { .name = "nanosleep" };
+struct syscall_spec syscall_spec_getitimer = { .name = "getitimer" };
+struct syscall_spec syscall_spec_alarm = { .name = "alarm" };
+struct syscall_spec syscall_spec_setitimer = { .name = "setitimer" };
+struct syscall_spec syscall_spec_getpid = { .name = "getpid" };
+struct syscall_spec syscall_spec_sendfile = { .name = "sendfile" };
+struct syscall_spec syscall_spec_socket = { .name = "socket" };
+struct syscall_spec syscall_spec_connect = { .name = "connect" };
+struct syscall_spec syscall_spec_accept = { .name = "accept" };
+struct syscall_spec syscall_spec_sendto = { .name = "sendto" };
+struct syscall_spec syscall_spec_recvfrom = { .name = "recvfrom" };
+struct syscall_spec syscall_spec_sendmsg = { .name = "sendmsg" };
+struct syscall_spec syscall_spec_recvmsg = { .name = "recvmsg" };
+struct syscall_spec syscall_spec_shutdown = { .name = "shutdown" };
+struct syscall_spec syscall_spec_bind = { .name = "bind" };
+struct syscall_spec syscall_spec_listen = { .name = "listen" };
+struct syscall_spec syscall_spec_getsockname = { .name = "getsockname" };
+struct syscall_spec syscall_spec_getpeername = { .name = "getpeername" };
+struct syscall_spec syscall_spec_socketpair = { .name = "socketpair" };
+struct syscall_spec syscall_spec_setsockopt = { .name = "setsockopt" };
+struct syscall_spec syscall_spec_getsockopt = { .name = "getsockopt" };
+struct syscall_spec syscall_spec_clone = { .name = "clone" };
+struct syscall_spec syscall_spec_fork = { .name = "fork" };
+struct syscall_spec syscall_spec_vfork = { .name = "vfork" };
+struct syscall_spec syscall_spec_execve = { .name = "execve" };
+struct syscall_spec syscall_spec_exit = { .name = "exit" };
+struct syscall_spec syscall_spec_wait4 = { .name = "wait4" };
+struct syscall_spec syscall_spec_kill = { .name = "kill" };
+struct syscall_spec syscall_spec_uname = { .name = "uname" };
+struct syscall_spec syscall_spec_semget = { .name = "semget" };
+struct syscall_spec syscall_spec_semop = { .name = "semop" };
+struct syscall_spec syscall_spec_semctl = { .name = "semctl" };
+struct syscall_spec syscall_spec_shmdt = { .name = "shmdt" };
+struct syscall_spec syscall_spec_msgget = { .name = "msgget" };
+struct syscall_spec syscall_spec_msgsnd = { .name = "msgsnd" };
+struct syscall_spec syscall_spec_msgrcv = { .name = "msgrcv" };
+struct syscall_spec syscall_spec_msgctl = { .name = "msgctl" };
+struct syscall_spec syscall_spec_fcntl = { .name = "fcntl" };
+struct syscall_spec syscall_spec_flock = { .name = "flock" };
+struct syscall_spec syscall_spec_fsync = { .name = "fsync" };
+struct syscall_spec syscall_spec_fdatasync = { .name = "fdatasync" };
+struct syscall_spec syscall_spec_truncate = { .name = "truncate" };
+struct syscall_spec syscall_spec_ftruncate = { .name = "ftruncate" };
+struct syscall_spec syscall_spec_getdents = { .name = "getdents" };
+struct syscall_spec syscall_spec_getcwd = { .name = "getcwd" };
+struct syscall_spec syscall_spec_chdir = { .name = "chdir" };
+struct syscall_spec syscall_spec_fchdir = { .name = "fchdir" };
+struct syscall_spec syscall_spec_rename = { .name = "rename" };
+struct syscall_spec syscall_spec_mkdir = { .name = "mkdir" };
+struct syscall_spec syscall_spec_rmdir = { .name = "rmdir" };
+struct syscall_spec syscall_spec_creat = { .name = "creat" };
+struct syscall_spec syscall_spec_link = { .name = "link" };
+struct syscall_spec syscall_spec_unlink = { .name = "unlink" };
+struct syscall_spec syscall_spec_symlink = { .name = "symlink" };
+struct syscall_spec syscall_spec_readlink = { .name = "readlink" };
+struct syscall_spec syscall_spec_chmod = { .name = "chmod" };
+struct syscall_spec syscall_spec_fchmod = { .name = "fchmod" };
+struct syscall_spec syscall_spec_chown = { .name = "chown" };
+struct syscall_spec syscall_spec_fchown = { .name = "fchown" };
+struct syscall_spec syscall_spec_lchown = { .name = "lchown" };
+struct syscall_spec syscall_spec_umask = { .name = "umask" };
+struct syscall_spec syscall_spec_gettimeofday = { .name = "gettimeofday" };
+struct syscall_spec syscall_spec_getrlimit = { .name = "getrlimit" };
+struct syscall_spec syscall_spec_getrusage = { .name = "getrusage" };
+struct syscall_spec syscall_spec_sysinfo = { .name = "sysinfo" };
+struct syscall_spec syscall_spec_times = { .name = "times" };
+struct syscall_spec syscall_spec_ptrace = { .name = "ptrace" };
+struct syscall_spec syscall_spec_getuid = { .name = "getuid" };
+struct syscall_spec syscall_spec_syslog = { .name = "syslog" };
+struct syscall_spec syscall_spec_getgid = { .name = "getgid" };
+struct syscall_spec syscall_spec_setuid = { .name = "setuid" };
+struct syscall_spec syscall_spec_setgid = { .name = "setgid" };
+struct syscall_spec syscall_spec_geteuid = { .name = "geteuid" };
+struct syscall_spec syscall_spec_getegid = { .name = "getegid" };
+struct syscall_spec syscall_spec_setpgid = { .name = "setpgid" };
+struct syscall_spec syscall_spec_getppid = { .name = "getppid" };
+struct syscall_spec syscall_spec_getpgrp = { .name = "getpgrp" };
+struct syscall_spec syscall_spec_setsid = { .name = "setsid" };
+struct syscall_spec syscall_spec_setreuid = { .name = "setreuid" };
+struct syscall_spec syscall_spec_setregid = { .name = "setregid" };
+struct syscall_spec syscall_spec_getgroups = { .name = "getgroups" };
+struct syscall_spec syscall_spec_setgroups = { .name = "setgroups" };
+struct syscall_spec syscall_spec_setresuid = { .name = "setresuid" };
+struct syscall_spec syscall_spec_getresuid = { .name = "getresuid" };
+struct syscall_spec syscall_spec_setresgid = { .name = "setresgid" };
+struct syscall_spec syscall_spec_getresgid = { .name = "getresgid" };
+struct syscall_spec syscall_spec_getpgid = { .name = "getpgid" };
+struct syscall_spec syscall_spec_setfsuid = { .name = "setfsuid" };
+struct syscall_spec syscall_spec_setfsgid = { .name = "setfsgid" };
+struct syscall_spec syscall_spec_getsid = { .name = "getsid" };
+struct syscall_spec syscall_spec_capget = { .name = "capget" };
+struct syscall_spec syscall_spec_capset = { .name = "capset" };
+struct syscall_spec syscall_spec_rt_sigpending = { .name = "rt_sigpending" };
+struct syscall_spec syscall_spec_rt_sigtimedwait = { .name = "rt_sigtimedwait" };
+struct syscall_spec syscall_spec_rt_sigqueueinfo = { .name = "rt_sigqueueinfo" };
+struct syscall_spec syscall_spec_rt_sigsuspend = { .name = "rt_sigsuspend" };
+struct syscall_spec syscall_spec_sigaltstack = { .name = "sigaltstack" };
+struct syscall_spec syscall_spec_utime = { .name = "utime" };
+struct syscall_spec syscall_spec_mknod = { .name = "mknod" };
+struct syscall_spec syscall_spec_uselib = { .name = "uselib" };
+struct syscall_spec syscall_spec_personality = { .name = "personality" };
+struct syscall_spec syscall_spec_ustat = { .name = "ustat" };
+struct syscall_spec syscall_spec_statfs = { .name = "statfs" };
+struct syscall_spec syscall_spec_fstatfs = { .name = "fstatfs" };
+struct syscall_spec syscall_spec_sysfs = { .name = "sysfs" };
+struct syscall_spec syscall_spec_getpriority = { .name = "getpriority" };
+struct syscall_spec syscall_spec_setpriority = { .name = "setpriority" };
+struct syscall_spec syscall_spec_sched_setparam = { .name = "sched_setparam" };
+struct syscall_spec syscall_spec_sched_getparam = { .name = "sched_getparam" };
+struct syscall_spec syscall_spec_sched_setscheduler = { .name = "sched_setscheduler" };
+struct syscall_spec syscall_spec_sched_getscheduler = { .name = "sched_getscheduler" };
+struct syscall_spec syscall_spec_sched_get_priority_max = { .name = "sched_get_priority_max" };
+struct syscall_spec syscall_spec_sched_get_priority_min = { .name = "sched_get_priority_min" };
+struct syscall_spec syscall_spec_sched_rr_get_interval = { .name = "sched_rr_get_interval" };
+struct syscall_spec syscall_spec_mlock = { .name = "mlock" };
+struct syscall_spec syscall_spec_munlock = { .name = "munlock" };
+struct syscall_spec syscall_spec_mlockall = { .name = "mlockall" };
+struct syscall_spec syscall_spec_munlockall = { .name = "munlockall" };
+struct syscall_spec syscall_spec_vhangup = { .name = "vhangup" };
+struct syscall_spec syscall_spec_modify_ldt = { .name = "modify_ldt" };
+struct syscall_spec syscall_spec_pivot_root = { .name = "pivot_root" };
+struct syscall_spec syscall_spec__sysctl = { .name = "_sysctl" };
+struct syscall_spec syscall_spec_prctl = { .name = "prctl" };
+struct syscall_spec syscall_spec_arch_prctl = { .name = "arch_prctl" };
+struct syscall_spec syscall_spec_adjtimex = { .name = "adjtimex" };
+struct syscall_spec syscall_spec_setrlimit = { .name = "setrlimit" };
+struct syscall_spec syscall_spec_chroot = { .name = "chroot" };
+struct syscall_spec syscall_spec_sync = { .name = "sync" };
+struct syscall_spec syscall_spec_acct = { .name = "acct" };
+struct syscall_spec syscall_spec_settimeofday = { .name = "settimeofday" };
+struct syscall_spec syscall_spec_mount = { .name = "mount" };
+struct syscall_spec syscall_spec_umount2 = { .name = "umount2" };
+struct syscall_spec syscall_spec_swapon = { .name = "swapon" };
+struct syscall_spec syscall_spec_swapoff = { .name = "swapoff" };
+struct syscall_spec syscall_spec_reboot = { .name = "reboot" };
+struct syscall_spec syscall_spec_sethostname = { .name = "sethostname" };
+struct syscall_spec syscall_spec_setdomainname = { .name = "setdomainname" };
+struct syscall_spec syscall_spec_iopl = { .name = "iopl" };
+struct syscall_spec syscall_spec_ioperm = { .name = "ioperm" };
+struct syscall_spec syscall_spec_create_module = { .name = "create_module" };
+struct syscall_spec syscall_spec_init_module = { .name = "init_module" };
+struct syscall_spec syscall_spec_delete_module = { .name = "delete_module" };
+struct syscall_spec syscall_spec_get_kernel_syms = { .name = "get_kernel_syms" };
+struct syscall_spec syscall_spec_query_module = { .name = "query_module" };
+struct syscall_spec syscall_spec_quotactl = { .name = "quotactl" };
+struct syscall_spec syscall_spec_nfsservctl = { .name = "nfsservctl" };
+struct syscall_spec syscall_spec_getpmsg = { .name = "getpmsg" };
+struct syscall_spec syscall_spec_putpmsg = { .name = "putpmsg" };
+struct syscall_spec syscall_spec_afs_syscall = { .name = "afs_syscall" };
+struct syscall_spec syscall_spec_tuxcall = { .name = "tuxcall" };
+struct syscall_spec syscall_spec_security = { .name = "security" };
+struct syscall_spec syscall_spec_gettid = { .name = "gettid" };
+struct syscall_spec syscall_spec_readahead = { .name = "readahead" };
+struct syscall_spec syscall_spec_setxattr = { .name = "setxattr" };
+struct syscall_spec syscall_spec_lsetxattr = { .name = "lsetxattr" };
+struct syscall_spec syscall_spec_fsetxattr = { .name = "fsetxattr" };
+struct syscall_spec syscall_spec_getxattr = { .name = "getxattr" };
+struct syscall_spec syscall_spec_lgetxattr = { .name = "lgetxattr" };
+struct syscall_spec syscall_spec_fgetxattr = { .name = "fgetxattr" };
+struct syscall_spec syscall_spec_listxattr = { .name = "listxattr" };
+struct syscall_spec syscall_spec_llistxattr = { .name = "llistxattr" };
+struct syscall_spec syscall_spec_flistxattr = { .name = "flistxattr" };
+struct syscall_spec syscall_spec_removexattr = { .name = "removexattr" };
+struct syscall_spec syscall_spec_lremovexattr = { .name = "lremovexattr" };
+struct syscall_spec syscall_spec_fremovexattr = { .name = "fremovexattr" };
+struct syscall_spec syscall_spec_tkill = { .name = "tkill" };
+struct syscall_spec syscall_spec_time = { .name = "time" };
+struct syscall_spec syscall_spec_futex = { .name = "futex" };
+struct syscall_spec syscall_spec_sched_setaffinity = { .name = "sched_setaffinity" };
+struct syscall_spec syscall_spec_sched_getaffinity = { .name = "sched_getaffinity" };
+struct syscall_spec syscall_spec_set_thread_area = { .name = "set_thread_area" };
+struct syscall_spec syscall_spec_io_setup = { .name = "io_setup" };
+struct syscall_spec syscall_spec_io_destroy = { .name = "io_destroy" };
+struct syscall_spec syscall_spec_io_getevents = { .name = "io_getevents" };
+struct syscall_spec syscall_spec_io_submit = { .name = "io_submit" };
+struct syscall_spec syscall_spec_io_cancel = { .name = "io_cancel" };
+struct syscall_spec syscall_spec_get_thread_area = { .name = "get_thread_area" };
+struct syscall_spec syscall_spec_lookup_dcookie = { .name = "lookup_dcookie" };
+struct syscall_spec syscall_spec_epoll_create = { .name = "epoll_create" };
+struct syscall_spec syscall_spec_epoll_ctl_old = { .name = "epoll_ctl_old" };
+struct syscall_spec syscall_spec_epoll_wait_old = { .name = "epoll_wait_old" };
+struct syscall_spec syscall_spec_remap_file_pages = { .name = "remap_file_pages" };
+struct syscall_spec syscall_spec_getdents64 = { .name = "getdents64" };
+struct syscall_spec syscall_spec_set_tid_address = { .name = "set_tid_address" };
+struct syscall_spec syscall_spec_restart_syscall = { .name = "restart_syscall" };
+struct syscall_spec syscall_spec_semtimedop = { .name = "semtimedop" };
+struct syscall_spec syscall_spec_fadvise64 = { .name = "fadvise64" };
+struct syscall_spec syscall_spec_timer_create = { .name = "timer_create" };
+struct syscall_spec syscall_spec_timer_settime = { .name = "timer_settime" };
+struct syscall_spec syscall_spec_timer_gettime = { .name = "timer_gettime" };
+struct syscall_spec syscall_spec_timer_getoverrun = { .name = "timer_getoverrun" };
+struct syscall_spec syscall_spec_timer_delete = { .name = "timer_delete" };
+struct syscall_spec syscall_spec_clock_settime = { .name = "clock_settime" };
+struct syscall_spec syscall_spec_clock_gettime = { .name = "clock_gettime" };
+struct syscall_spec syscall_spec_clock_getres = { .name = "clock_getres" };
+struct syscall_spec syscall_spec_clock_nanosleep = { .name = "clock_nanosleep" };
+struct syscall_spec syscall_spec_exit_group = { .name = "exit_group" };
+struct syscall_spec syscall_spec_epoll_wait = { .name = "epoll_wait" };
+struct syscall_spec syscall_spec_epoll_ctl = { .name = "epoll_ctl" };
+struct syscall_spec syscall_spec_tgkill = { .name = "tgkill" };
+struct syscall_spec syscall_spec_utimes = { .name = "utimes" };
+struct syscall_spec syscall_spec_vserver = { .name = "vserver" };
+struct syscall_spec syscall_spec_mbind = { .name = "mbind" };
+struct syscall_spec syscall_spec_set_mempolicy = { .name = "set_mempolicy" };
+struct syscall_spec syscall_spec_get_mempolicy = { .name = "get_mempolicy" };
+struct syscall_spec syscall_spec_mq_open = { .name = "mq_open" };
+struct syscall_spec syscall_spec_mq_unlink = { .name = "mq_unlink" };
+struct syscall_spec syscall_spec_mq_timedsend = { .name = "mq_timedsend" };
+struct syscall_spec syscall_spec_mq_timedreceive = { .name = "mq_timedreceive" };
+struct syscall_spec syscall_spec_mq_notify = { .name = "mq_notify" };
+struct syscall_spec syscall_spec_mq_getsetattr = { .name = "mq_getsetattr" };
+struct syscall_spec syscall_spec_kexec_load = { .name = "kexec_load" };
+struct syscall_spec syscall_spec_waitid = { .name = "waitid" };
+struct syscall_spec syscall_spec_add_key = { .name = "add_key" };
+struct syscall_spec syscall_spec_request_key = { .name = "request_key" };
+struct syscall_spec syscall_spec_keyctl = { .name = "keyctl" };
+struct syscall_spec syscall_spec_ioprio_set = { .name = "ioprio_set" };
+struct syscall_spec syscall_spec_ioprio_get = { .name = "ioprio_get" };
+struct syscall_spec syscall_spec_inotify_init = { .name = "inotify_init" };
+struct syscall_spec syscall_spec_inotify_add_watch = { .name = "inotify_add_watch" };
+struct syscall_spec syscall_spec_inotify_rm_watch = { .name = "inotify_rm_watch" };
+struct syscall_spec syscall_spec_migrate_pages = { .name = "migrate_pages" };
+struct syscall_spec syscall_spec_openat = { .name = "openat" };
+struct syscall_spec syscall_spec_mkdirat = { .name = "mkdirat" };
+struct syscall_spec syscall_spec_mknodat = { .name = "mknodat" };
+struct syscall_spec syscall_spec_fchownat = { .name = "fchownat" };
+struct syscall_spec syscall_spec_futimesat = { .name = "futimesat" };
+struct syscall_spec syscall_spec_newfstatat = { .name = "newfstatat" };
+struct syscall_spec syscall_spec_unlinkat = { .name = "unlinkat" };
+struct syscall_spec syscall_spec_renameat = { .name = "renameat" };
+struct syscall_spec syscall_spec_linkat = { .name = "linkat" };
+struct syscall_spec syscall_spec_symlinkat = { .name = "symlinkat" };
+struct syscall_spec syscall_spec_readlinkat = { .name = "readlinkat" };
+struct syscall_spec syscall_spec_fchmodat = { .name = "fchmodat" };
+struct syscall_spec syscall_spec_faccessat = { .name = "faccessat" };
+struct syscall_spec syscall_spec_pselect6 = { .name = "pselect6" };
+struct syscall_spec syscall_spec_ppoll = { .name = "ppoll" };
+struct syscall_spec syscall_spec_unshare = { .name = "unshare" };
+struct syscall_spec syscall_spec_set_robust_list = { .name = "set_robust_list" };
+struct syscall_spec syscall_spec_get_robust_list = { .name = "get_robust_list" };
+struct syscall_spec syscall_spec_splice = { .name = "splice" };
+struct syscall_spec syscall_spec_tee = { .name = "tee" };
+struct syscall_spec syscall_spec_sync_file_range = { .name = "sync_file_range" };
+struct syscall_spec syscall_spec_vmsplice = { .name = "vmsplice" };
+struct syscall_spec syscall_spec_move_pages = { .name = "move_pages" };
+struct syscall_spec syscall_spec_utimensat = { .name = "utimensat" };
+struct syscall_spec syscall_spec_epoll_pwait = { .name = "epoll_pwait" };
+struct syscall_spec syscall_spec_signalfd = { .name = "signalfd" };
+struct syscall_spec syscall_spec_timerfd_create = { .name = "timerfd_create" };
+struct syscall_spec syscall_spec_eventfd = { .name = "eventfd" };
+struct syscall_spec syscall_spec_fallocate = { .name = "fallocate" };
+struct syscall_spec syscall_spec_timerfd_settime = { .name = "timerfd_settime" };
+struct syscall_spec syscall_spec_timerfd_gettime = { .name = "timerfd_gettime" };
+struct syscall_spec syscall_spec_accept4 = { .name = "accept4" };
+struct syscall_spec syscall_spec_signalfd4 = { .name = "signalfd4" };
+struct syscall_spec syscall_spec_eventfd2 = { .name = "eventfd2" };
+struct syscall_spec syscall_spec_epoll_create1 = { .name = "epoll_create1" };
+struct syscall_spec syscall_spec_dup3 = { .name = "dup3" };
+struct syscall_spec syscall_spec_pipe2 = { .name = "pipe2" };
+struct syscall_spec syscall_spec_inotify_init1 = { .name = "inotify_init1" };
+struct syscall_spec syscall_spec_preadv = { .name = "preadv" };
+struct syscall_spec syscall_spec_pwritev = { .name = "pwritev" };
+struct syscall_spec syscall_spec_rt_tgsigqueueinfo = { .name = "rt_tgsigqueueinfo" };
+struct syscall_spec syscall_spec_perf_event_open = { .name = "perf_event_open" };
+struct syscall_spec syscall_spec_recvmmsg = { .name = "recvmmsg" };
+struct syscall_spec syscall_spec_fanotify_init = { .name = "fanotify_init" };
+struct syscall_spec syscall_spec_fanotify_mark = { .name = "fanotify_mark" };
+struct syscall_spec syscall_spec_prlimit64 = { .name = "prlimit64" };
+struct syscall_spec syscall_spec_name_to_handle_at = { .name = "name_to_handle_at" };
+struct syscall_spec syscall_spec_open_by_handle_at = { .name = "open_by_handle_at" };
+struct syscall_spec syscall_spec_clock_adjtime = { .name = "clock_adjtime" };
+struct syscall_spec syscall_spec_syncfs = { .name = "syncfs" };
+struct syscall_spec syscall_spec_sendmmsg = { .name = "sendmmsg" };
+struct syscall_spec syscall_spec_setns = { .name = "setns" };
+struct syscall_spec syscall_spec_getcpu = { .name = "getcpu" };
+struct syscall_spec syscall_spec_process_vm_readv = { .name = "process_vm_readv" };
+struct syscall_spec syscall_spec_process_vm_writev = { .name = "process_vm_writev" };
+struct syscall_spec syscall_spec_kcmp = { .name = "kcmp" };
+struct syscall_spec syscall_spec_finit_module = { .name = "finit_module" };
+struct syscall_spec syscall_spec_sched_setattr = { .name = "sched_setattr" };
+struct syscall_spec syscall_spec_sched_getattr = { .name = "sched_getattr" };
+struct syscall_spec syscall_spec_renameat2 = { .name = "renameat2" };
+struct syscall_spec syscall_spec_seccomp = { .name = "seccomp" };
+struct syscall_spec syscall_spec_getrandom = { .name = "getrandom" };
+struct syscall_spec syscall_spec_memfd_create = { .name = "memfd_create" };
+struct syscall_spec syscall_spec_kexec_file_load = { .name = "kexec_file_load" };
+struct syscall_spec syscall_spec_bpf = { .name = "bpf" };
+struct syscall_spec syscall_spec_execveat = { .name = "execveat" };
+struct syscall_spec syscall_spec_userfaultfd = { .name = "userfaultfd" };
+struct syscall_spec syscall_spec_membarrier = { .name = "membarrier" };
+struct syscall_spec syscall_spec_mlock2 = { .name = "mlock2" };
+struct syscall_spec syscall_spec_copy_file_range = { .name = "copy_file_range" };
+struct syscall_spec syscall_spec_preadv2 = { .name = "preadv2" };
+struct syscall_spec syscall_spec_pwritev2 = { .name = "pwritev2" };
+struct syscall_spec syscall_spec_pkey_mprotect = { .name = "pkey_mprotect" };
+struct syscall_spec syscall_spec_pkey_alloc = { .name = "pkey_alloc" };
+struct syscall_spec syscall_spec_pkey_free = { .name = "pkey_free" };
+struct syscall_spec syscall_spec_waitpid = { .name = "waitpid" };
+
+struct syscall_spec syscall_spec_sysctl = { .name = "sysctl" };
+struct syscall_spec syscall_spec_sendfile64 = { .name = "sendfile64" };
+struct syscall_spec syscall_spec_sigprocmask = { .name = "sigprocmask" };
+struct syscall_spec syscall_spec_signal = { .name = "signal" };
+struct syscall_spec syscall_spec_ssetmask = { .name = "ssetmask" };
+struct syscall_spec syscall_spec_olduname = { .name = "olduname" };
+struct syscall_spec syscall_spec_gethostname = { .name = "gethostname" };
+struct syscall_spec syscall_spec_old_getrlimit = { .name = "old_getrlimit" };
+struct syscall_spec syscall_spec_llseek = { .name = "llseek" };
+struct syscall_spec syscall_spec_sigpending = { .name = "sigpending" };
+struct syscall_spec syscall_spec_sigsuspend = { .name = "sigsuspend" };
+struct syscall_spec syscall_spec_newuname = { .name = "newuname" };
+struct syscall_spec syscall_spec_newstat = { .name = "newstat" };
+struct syscall_spec syscall_spec_newlstat = { .name = "newlstat" };
+struct syscall_spec syscall_spec_newfstat = { .name = "newfstat" };
+struct syscall_spec syscall_spec_mmap_pgoff = { .name = "mmap_pgoff" };
+struct syscall_spec syscall_spec_nice = { .name = "nice" };
+struct syscall_spec syscall_spec_fadvise64_64 = { .name = "fadvist64_64" };
+struct syscall_spec syscall_spec_old_readdir = { .name = "old_readdir" };
+struct syscall_spec syscall_spec_umount = { .name = "umount" };
+struct syscall_spec syscall_spec_oldumount = { .name = "oldumount" };
+struct syscall_spec syscall_spec_stime = { .name = "stime" };
+struct syscall_spec syscall_spec_send = { .name = "send" };
+struct syscall_spec syscall_spec_recv = { .name = "recv" };
+struct syscall_spec syscall_spec_socketcall = { .name = "socketcall" };
+struct syscall_spec syscall_spec_lchown16 = { .name = "lchown16" };
+struct syscall_spec syscall_spec_fchown16 = { .name = "fchown16" };
+struct syscall_spec syscall_spec_setregid16 = { .name = "setregid16" };
+struct syscall_spec syscall_spec_setgid16 = { .name = "setgid16" };
+struct syscall_spec syscall_spec_setreuid16 = { .name = "setreuid16" };
+struct syscall_spec syscall_spec_setuid16 = { .name = "setuid16" };
+struct syscall_spec syscall_spec_setresuid16 = { .name = "setresuid16" };
+struct syscall_spec syscall_spec_setsuid16 = { .name = "setsuid16" };
+struct syscall_spec syscall_spec_chown16 = { .name = "chown16" };
+struct syscall_spec syscall_spec_getresuid16 = { .name = "getresuid16" };
+struct syscall_spec syscall_spec_setresgid16 = { .name = "setresgid16" };
+struct syscall_spec syscall_spec_getresgid16 = { .name = "getresgid16" };
+struct syscall_spec syscall_spec_setfsuid16 = { .name = "setfsuid16" };
+struct syscall_spec syscall_spec_setfsgid16 = { .name = "setfsgid16" };
+struct syscall_spec syscall_spec_getgroups16 = { .name = "getgroups16" };
+struct syscall_spec syscall_spec_setgroups16 = { .name = "sethroups16" };
+struct syscall_spec syscall_spec_sync_file_range2 = { .name = "sync_file_range2" };
+struct syscall_spec syscall_spec_statfs64 = { .name = "statfs64" };
+struct syscall_spec syscall_spec_fstatfs64 = { .name = "fstatfs64" };
+struct syscall_spec syscall_spec_bdflush = { .name = "bdflush" };
-- 
2.7.4

Re: [RFC 0/3] ABI spec - verification

[Thomas Gleixner]

On Wed, 16 Nov 2016, alexander.levin@verizon.com wrote:

Cc'ed Nicholas McGuire

> As discussed at plumbers, having a standard spec for the kernel's ABI has
> quite a few uses and enough people wanted it to get the ball rolling.
> 
> We agreed that it's desirable to have something that can be used from code
> rather than just a spec on paper both for validation and allowing other users
> (like fuzzers, userspace libraries, and various userspace tools) to build
> on that.
> 
> What we ended up deciding on at plumbers is:
> 
>  - I'll do a few kernel bits do demonstrate how we can validate the spec from
> the kernel.
>  - Dmitry Vyukov will provide a way to translate syzkaller's syscall
> documentation into something that can be easily used in the kernel and
> userspace.
>  - Various projects will attempt to integrate it to make sure that the
> framework works for them.
> 
> Once those bits are done we can focus on getting the spec right, and we'll
> have a good way to validate our work both in userspace and in the kernel.
> 
> This patchset is a basic draft of said kernel bits. I mostly want to make
> sure that Dmitry and myself are on the same page as to how integration will
> look like, but also to open it to criticism and suggestions (bikeshedding).
> 
> 
> Sasha Levin (3):
>   abi_spec: basic definitions of constraints, args and syscalls
>   abi_spec: hooks into syscall to allow pre and post checking
>   abi_spec: example spec for open(), placeholder for rest of syscalls
> 
>  include/linux/syscalls.h      |   7 +-
>  include/uapi/linux/abi_spec.h |  58 ++++++
>  kernel/Makefile               |   2 +
>  kernel/abi_spec.c             | 456 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 522 insertions(+), 1 deletion(-)
>  create mode 100644 include/uapi/linux/abi_spec.h
>  create mode 100644 kernel/abi_spec.c
> 
> -- 
> 2.7.4
> 

Re: [RFC 0/3] ABI spec - verification

[Dmitry Vyukov]

On Wed, Nov 16, 2016 at 6:37 PM,  <alexander.levin@verizon.com> wrote:
> As discussed at plumbers, having a standard spec for the kernel's ABI has
> quite a few uses and enough people wanted it to get the ball rolling.
>
> We agreed that it's desirable to have something that can be used from code
> rather than just a spec on paper both for validation and allowing other users
> (like fuzzers, userspace libraries, and various userspace tools) to build
> on that.
>
> What we ended up deciding on at plumbers is:
>
>  - I'll do a few kernel bits do demonstrate how we can validate the spec from
> the kernel.
>  - Dmitry Vyukov will provide a way to translate syzkaller's syscall
> documentation into something that can be easily used in the kernel and
> userspace.
>  - Various projects will attempt to integrate it to make sure that the
> framework works for them.
>
> Once those bits are done we can focus on getting the spec right, and we'll
> have a good way to validate our work both in userspace and in the kernel.
>
> This patchset is a basic draft of said kernel bits. I mostly want to make
> sure that Dmitry and myself are on the same page as to how integration will
> look like, but also to open it to criticism and suggestions (bikeshedding).


Looks like a good starting point!

Do you have a git repo with this somewhere? I have problems applying
the patches, seems that my gmail messed them with some weird escaping.

Is the intention that these descriptions are written by hand, or
generated from some DSL?
I benefited from easier to write descriptions, also I changed several
times what code generator generates without changing descriptions.
However, an additional level of indirection in the form of code
generator introduces own pain to maintain. So I am not too strong
here.

I will post other comments inline in the patches.

Thanks!

> Sasha Levin (3):
>   abi_spec: basic definitions of constraints, args and syscalls
>   abi_spec: hooks into syscall to allow pre and post checking
>   abi_spec: example spec for open(), placeholder for rest of syscalls
>
>  include/linux/syscalls.h      |   7 +-
>  include/uapi/linux/abi_spec.h |  58 ++++++
>  kernel/Makefile               |   2 +
>  kernel/abi_spec.c             | 456 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 522 insertions(+), 1 deletion(-)
>  create mode 100644 include/uapi/linux/abi_spec.h
>  create mode 100644 kernel/abi_spec.c
>
> --
> 2.7.4

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[Dmitry Vyukov]

On Wed, Nov 16, 2016 at 6:37 PM,  <alexander.levin@verizon.com> wrote:
> This is a very simple definition of the syscall ABI we can build on. The idea
> is to have a generic description of syscalls, their arguments and return
> values we can use to audit the kernel's implementation vs the specs.
>
> Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
> ---
>  include/uapi/linux/abi_spec.h | 58 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 include/uapi/linux/abi_spec.h
>
> diff --git a/include/uapi/linux/abi_spec.h b/include/uapi/linux/abi_spec.h
> new file mode 100644
> index 0000000..ad1a992
> --- /dev/null
> +++ b/include/uapi/linux/abi_spec.h
> @@ -0,0 +1,58 @@
> +#ifndef ABI_SPEC_H_
> +#define ABI_SPEC_H_
> +
> +#include <linux/fcntl.h>
> +#include <linux/stat.h>
> +#define MAX_CONSTRAINTS 10
> +#define MAX_ARGS 10
> +
> +#define        TYPE_FD         1
> +#define TYPE_INT       2
> +#define TYPE_PTR       3
> +#define TYPE_STRING    4
> +/* ... */
> +
> +#define CONSTRAINT_NON_NULL    (1<<0)
> +#define CONSTRAINT_RANGE       (1<<1)
> +#define CONSTRAINT_ADDRESS_TYPE        (1<<2)
> +#define CONSTRAINT_FD_TYPE     (1<<3)
> +#define CONSTRAINT_ERRNO       (1<<4)
> +#define CONSTRAINT_BITMASK     (1<<5)
> +#define CONSTRAINT_PATH                (1<<6)
> +/* ... */
> +/* A generic constraint on an argument or a return value */
> +struct constraint {
> +       unsigned int flags;             /* bitmask of applied constraints */
> +       union {
> +               struct {                /* int range */
> +                       int int_min;
> +                       int int_max;
> +               };
> +               unsigned long bitmask;  /* allowed flags bitmask */
> +               unsigned long address_flags;    /* Type of allowed addr */
> +               unsigned long fd_flags; /* Type of allowed fd */
> +       };
> +};
> +
> +/* A generic argument (or return value) */
> +struct argument {
> +       const char *name;
> +       int type;                       /* should be a nicer way to do this */
> +
> +       unsigned int nconstraints;      /* can there be more than 1-2? */
> +       struct constraint constraints[MAX_CONSTRAINTS];
> +};


Several observations based on my experience with syzkaller descriptions:
 - there are 2 levels: physical and logical;
   on physical level there are int, pointer, array, struct, union;
   and that's pretty much it.
   on logical level there are flags, bitmasks, file paths, sctp socket fds,
   unix socket names, etc.
   These levels are almost completely orthogonal. It would be useful to
   clearly separate them on description level. E.g. now you have TYPE_PTR and
   TYPE_INT which is physical level; and then TYPE_FD which is also an int.

 - logical types won't fit into 64 bits, there are more of them

 - we need support for recursive types (yes, there are linked lists in
kernel APIs)

 - we need support for input/output data
   currently syzkaller does this only on pointer level, i.e. you
attach direction to pointer target
   but that's not enough, frequently there is a struct where one field
is input and another is output

 - we may need support for reusing types in several arguments
   e.g. you may have a pretty complex type, and you don't want to
write it out a dozen of times

 - we need some support for discriminated syscalls
   if we want to support strace usecase, the support needs to be more
extensive than what syzkaller has;
   i.e. syzkaller can't restore discrimination having actual argument
values (it can do it only in the other direction)

 - I would not create a special support for arguments;
   rather I would create support for structs and struct fields,
   and then pretend that a syscalls effectively accepts a struct by value


How would you like us to collaborate on this?
If you share your git repo, I could form it into something that would
be suitable for syzkaller and incorporate most of the above.


> +/* A generic syscall */
> +struct syscall_spec {
> +       const char *name;
> +       struct argument retval;
> +
> +       unsigned int nargs;
> +       struct argument args[MAX_ARGS];
> +};
> +
> +void abispec_check_pre(const struct syscall_spec *s, ...);
> +void abispec_check_post(const struct syscall_spec *s, long retval, ...);
> +
> +#endif
> --
> 2.7.4

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[Steven Rostedt]

On Wed, 16 Nov 2016 17:37:00 +0000
alexander.levin@verizon.com wrote:

> This is a very simple definition of the syscall ABI we can build on. The idea
> is to have a generic description of syscalls, their arguments and return
> values we can use to audit the kernel's implementation vs the specs.
> 
> Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
> ---
>  include/uapi/linux/abi_spec.h | 58 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 include/uapi/linux/abi_spec.h
> 
> diff --git a/include/uapi/linux/abi_spec.h b/include/uapi/linux/abi_spec.h
> new file mode 100644
> index 0000000..ad1a992
> --- /dev/null
> +++ b/include/uapi/linux/abi_spec.h
> @@ -0,0 +1,58 @@
> +#ifndef ABI_SPEC_H_
> +#define ABI_SPEC_H_
> +
> +#include <linux/fcntl.h>
> +#include <linux/stat.h>
> +#define MAX_CONSTRAINTS 10
> +#define MAX_ARGS 10
> +

I'm curious to where you picked the number 10 from? Actually, I was
doing some work with the syscall_get_arguments() and the max syscall
arguments is currently set to 6. Anything greater causes a bug.

-- Steve

Re: [RFC 2/3] abi_spec: hooks into syscall to allow pre and post checking

[Steven Rostedt]

On Wed, 16 Nov 2016 17:37:01 +0000
alexander.levin@verizon.com wrote:

> This is a simple way to be able to verify syscall parameters before the
> call to the actual syscall, and also verify the return value after the
> call.
> 
> Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
> ---
>  include/linux/syscalls.h |  7 ++++++-
>  kernel/Makefile          |  2 ++
>  kernel/abi_spec.c        | 15 +++++++++++++++
>  3 files changed, 23 insertions(+), 1 deletion(-)
>  create mode 100644 kernel/abi_spec.c
> 
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 91a740f..6aa3228 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -79,6 +79,7 @@ union bpf_attr;
>  #include <linux/quota.h>
>  #include <linux/key.h>
>  #include <trace/syscall.h>
> +#include <linux/abi_spec.h>
>  
>  /*
>   * __MAP - apply a macro to syscall arguments
> @@ -192,13 +193,17 @@ extern struct trace_event_functions exit_syscall_print_funcs;
>  
>  #define __PROTECT(...) asmlinkage_protect(__VA_ARGS__)
>  #define __SYSCALL_DEFINEx(x, name, ...)					\
> +	extern const struct syscall_spec syscall_spec##name;		\
>  	asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))	\
>  		__attribute__((alias(__stringify(SyS##name))));		\
>  	static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
>  	asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
>  	asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
>  	{								\
> -		long ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));	\
> +		long ret;						\
> +		abispec_check_pre(&syscall_spec##name, __MAP(x,__SC_CAST,__VA_ARGS__)); \
> +		ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));	\
> +		abispec_check_post(&syscall_spec##name, ret, __MAP(x,__SC_CAST,__VA_ARGS__)); \

Do you want this for DEFINE0() too? Or does this not care about system
calls with no arguments?

-- Steve

>  		__MAP(x,__SC_TEST,__VA_ARGS__);				\
>  		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));	\
>  		return ret;						\

Re: [RFC 2/3] abi_spec: hooks into syscall to allow pre and post checking

[Dmitry Vyukov]

On Mon, Nov 21, 2016 at 4:54 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 16 Nov 2016 17:37:01 +0000
> alexander.levin@verizon.com wrote:
>
>> This is a simple way to be able to verify syscall parameters before the
>> call to the actual syscall, and also verify the return value after the
>> call.
>>
>> Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
>> ---
>>  include/linux/syscalls.h |  7 ++++++-
>>  kernel/Makefile          |  2 ++
>>  kernel/abi_spec.c        | 15 +++++++++++++++
>>  3 files changed, 23 insertions(+), 1 deletion(-)
>>  create mode 100644 kernel/abi_spec.c
>>
>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index 91a740f..6aa3228 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -79,6 +79,7 @@ union bpf_attr;
>>  #include <linux/quota.h>
>>  #include <linux/key.h>
>>  #include <trace/syscall.h>
>> +#include <linux/abi_spec.h>
>>
>>  /*
>>   * __MAP - apply a macro to syscall arguments
>> @@ -192,13 +193,17 @@ extern struct trace_event_functions exit_syscall_print_funcs;
>>
>>  #define __PROTECT(...) asmlinkage_protect(__VA_ARGS__)
>>  #define __SYSCALL_DEFINEx(x, name, ...)                                      \
>> +     extern const struct syscall_spec syscall_spec##name;            \
>>       asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))       \
>>               __attribute__((alias(__stringify(SyS##name))));         \
>>       static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));  \
>>       asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));      \
>>       asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))       \
>>       {                                                               \
>> -             long ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));  \
>> +             long ret;                                               \
>> +             abispec_check_pre(&syscall_spec##name, __MAP(x,__SC_CAST,__VA_ARGS__)); \
>> +             ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));       \
>> +             abispec_check_post(&syscall_spec##name, ret, __MAP(x,__SC_CAST,__VA_ARGS__)); \
>
> Do you want this for DEFINE0() too? Or does this not care about system
> calls with no arguments?

This should care about syscalls without args:
 - we still may want to check return values
 - something like debug tracing would like to print them
 - there may also be some side effects (or absence of side effects)
that we may want to check



>>               __MAP(x,__SC_TEST,__VA_ARGS__);                         \
>>               __PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));       \
>>               return ret;                                             \
>

Re: [RFC 0/3] ABI spec - verification

[alexander.levin]

On Mon, Nov 21, 2016 at 03:25:05PM +0100, Dmitry Vyukov wrote:
> On Wed, Nov 16, 2016 at 6:37 PM,  <alexander.levin@verizon.com> wrote:
> > As discussed at plumbers, having a standard spec for the kernel's ABI has
> > quite a few uses and enough people wanted it to get the ball rolling.
> >
> > We agreed that it's desirable to have something that can be used from code
> > rather than just a spec on paper both for validation and allowing other users
> > (like fuzzers, userspace libraries, and various userspace tools) to build
> > on that.
> >
> > What we ended up deciding on at plumbers is:
> >
> >  - I'll do a few kernel bits do demonstrate how we can validate the spec from
> > the kernel.
> >  - Dmitry Vyukov will provide a way to translate syzkaller's syscall
> > documentation into something that can be easily used in the kernel and
> > userspace.
> >  - Various projects will attempt to integrate it to make sure that the
> > framework works for them.
> >
> > Once those bits are done we can focus on getting the spec right, and we'll
> > have a good way to validate our work both in userspace and in the kernel.
> >
> > This patchset is a basic draft of said kernel bits. I mostly want to make
> > sure that Dmitry and myself are on the same page as to how integration will
> > look like, but also to open it to criticism and suggestions (bikeshedding).
> 
> 
> Looks like a good starting point!
> 
> Do you have a git repo with this somewhere? I have problems applying
> the patches, seems that my gmail messed them with some weird escaping.

I've pushed it to https://git.kernel.org/cgit/linux/kernel/git/sashal/linux.git/log/?h=abi_spec ,
will try to keep it updated.

> Is the intention that these descriptions are written by hand, or
> generated from some DSL?
> I benefited from easier to write descriptions, also I changed several
> times what code generator generates without changing descriptions.
> However, an additional level of indirection in the form of code
> generator introduces own pain to maintain. So I am not too strong
> here.

I would really to have the descriptions written in just *one* place, either
by hand the way I did in that example, or in DSL. I understand your point about
another level of indirection, but I'm afraid that if we don't force a monolithic
spec we'll end up with way more than 2 different descriptions to maintain.

-- 

Thanks,
Sasha

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[alexander.levin]

On Mon, Nov 21, 2016 at 03:48:17PM +0100, Dmitry Vyukov wrote:
> Several observations based on my experience with syzkaller descriptions:
>  - there are 2 levels: physical and logical;
>    on physical level there are int, pointer, array, struct, union;
>    and that's pretty much it.
>    on logical level there are flags, bitmasks, file paths, sctp socket fds,
>    unix socket names, etc.
>    These levels are almost completely orthogonal. It would be useful to
>    clearly separate them on description level. E.g. now you have TYPE_PTR and
>    TYPE_INT which is physical level; and then TYPE_FD which is also an int.
> 
>  - logical types won't fit into 64 bits, there are more of them

I agree with your two points above.

As an example, let's look at:

	int epoll_ctl(int epfd, int op, int fd, struct epoll_event *event);

epfd would be a physical int, logical "epoll_fd", and constrainted with being
an open descriptor.

fd on the other hand is tricky: it's a physical int, logical fd, and
constrainted with being a file descriptor that supports poll, and being
open.

So while I think that logical types can be just a counter rather than a bitmask
I suspect that our constraints won't fit into 64 bits. Make 2 64 bit fields?
 
>  - we need support for recursive types (yes, there are linked lists in
> kernel APIs)

I imagine that this will be handled by specific logical type handlers we'll
need to implement. Can you give me an example and I'll try to code that?

>  - we need support for input/output data
>    currently syzkaller does this only on pointer level, i.e. you
> attach direction to pointer target
>    but that's not enough, frequently there is a struct where one field
> is input and another is output

Assuming it's "data", for intput we'll just need to check that the given
length is readable and for output that the length is writable, no?

We can do it with constraints right now.

>  - we may need support for reusing types in several arguments
>    e.g. you may have a pretty complex type, and you don't want to
> write it out a dozen of times

Yup, so if we go with the physical/logical split we can have handlers for
logical types.

>  - we need some support for discriminated syscalls
>    if we want to support strace usecase, the support needs to be more
> extensive than what syzkaller has;
>    i.e. syzkaller can't restore discrimination having actual argument
> values (it can do it only in the other direction)
> 
>  - I would not create a special support for arguments;
>    rather I would create support for structs and struct fields,
>    and then pretend that a syscalls effectively accepts a struct by value

But that means I need a custom handler for every syscall to parse the
struct fields rather than a generic code that goes through the args and calls
the right handler?

> How would you like us to collaborate on this?
> If you share your git repo, I could form it into something that would
> be suitable for syzkaller and incorporate most of the above.

I'd really like to have something that either generates these descriptions from
your DSL (it really doesn't have to be perfect (at first)) or something that
generates DSL from these C structs.

You probably have a better idea than me about the right direction to take there.

I've pushed these 3 patches to https://git.kernel.org/cgit/linux/kernel/git/sashal/linux.git/log/?h=abi_spec

-- 

Thanks,
Sasha

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[alexander.levin]

On Mon, Nov 21, 2016 at 10:41:02AM -0500, Steven Rostedt wrote:
> On Wed, 16 Nov 2016 17:37:00 +0000
> alexander.levin@verizon.com wrote:
> 
> > This is a very simple definition of the syscall ABI we can build on. The idea
> > is to have a generic description of syscalls, their arguments and return
> > values we can use to audit the kernel's implementation vs the specs.
> > 
> > Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
> > ---
> >  include/uapi/linux/abi_spec.h | 58 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >  create mode 100644 include/uapi/linux/abi_spec.h
> > 
> > diff --git a/include/uapi/linux/abi_spec.h b/include/uapi/linux/abi_spec.h
> > new file mode 100644
> > index 0000000..ad1a992
> > --- /dev/null
> > +++ b/include/uapi/linux/abi_spec.h
> > @@ -0,0 +1,58 @@
> > +#ifndef ABI_SPEC_H_
> > +#define ABI_SPEC_H_
> > +
> > +#include <linux/fcntl.h>
> > +#include <linux/stat.h>
> > +#define MAX_CONSTRAINTS 10
> > +#define MAX_ARGS 10
> > +
> 
> I'm curious to where you picked the number 10 from? Actually, I was
> doing some work with the syscall_get_arguments() and the max syscall
> arguments is currently set to 6. Anything greater causes a bug.

It's really just made up, but I wanted it to be higher than 6 because:

 - The "6" limit is only per-arch, so there might be something that wants
more than 6 args?
 - This should also work for ioctls in the future.

-- 

Thanks,
Sasha

Re: [RFC 2/3] abi_spec: hooks into syscall to allow pre and post checking

[alexander.levin]

On Mon, Nov 21, 2016 at 04:57:02PM +0100, Dmitry Vyukov wrote:
> On Mon, Nov 21, 2016 at 4:54 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Wed, 16 Nov 2016 17:37:01 +0000
> > alexander.levin@verizon.com wrote:
> >>  #define __SYSCALL_DEFINEx(x, name, ...)                                      \
> >> +     extern const struct syscall_spec syscall_spec##name;            \
> >>       asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))       \
> >>               __attribute__((alias(__stringify(SyS##name))));         \
> >>       static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));  \
> >>       asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));      \
> >>       asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))       \
> >>       {                                                               \
> >> -             long ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));  \
> >> +             long ret;                                               \
> >> +             abispec_check_pre(&syscall_spec##name, __MAP(x,__SC_CAST,__VA_ARGS__)); \
> >> +             ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));       \
> >> +             abispec_check_post(&syscall_spec##name, ret, __MAP(x,__SC_CAST,__VA_ARGS__)); \
> >
> > Do you want this for DEFINE0() too? Or does this not care about system
> > calls with no arguments?
> 
> This should care about syscalls without args:
>  - we still may want to check return values
>  - something like debug tracing would like to print them
>  - there may also be some side effects (or absence of side effects)
> that we may want to check

Yes, agreed, I just missed that detail in my implementation.

-- 

Thanks,
Sasha

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[Steven Rostedt]

On Wed, 23 Nov 2016 15:03:23 +0000
alexander.levin@verizon.com wrote:

> It's really just made up, but I wanted it to be higher than 6 because:
> 
>  - The "6" limit is only per-arch, so there might be something that wants
> more than 6 args?
>  - This should also work for ioctls in the future.

The generic code all has a max of 6. Look at the macros for the system
calls. All they define up to is SYSCALL_DEFINE6().

If any arch creates a syscall with 7 or more args, that will be a world
of pain. I say we simply don't support it. Also, that arch would
require keeping that system call totally within the arch itself, and
wont be able to call into generic code.

I'm working to clean up the system call tracepoints, and they currently
(and will continue to) have a hard max upper limit of 6 arguments.

-- Steve

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[Steven Rostedt]

On Wed, 23 Nov 2016 15:03:23 +0000
alexander.levin@verizon.com wrote:


>  - This should also work for ioctls in the future.

Any ioctl that requires more than 6 arguments should also be NACKed in
the future. If you need that many arguments, its best to simply create
a structure and pass that it.

Again, the function syscalls_get_arguments() will BUG() on most archs
if it expects more than 6 arguments.

-- Steve

Re: [RFC 0/3] ABI spec - verification

[Dmitry Vyukov]

On Wed, Nov 23, 2016 at 3:36 PM,  <alexander.levin@verizon.com> wrote:
> On Mon, Nov 21, 2016 at 03:25:05PM +0100, Dmitry Vyukov wrote:
>> On Wed, Nov 16, 2016 at 6:37 PM,  <alexander.levin@verizon.com> wrote:
>> > As discussed at plumbers, having a standard spec for the kernel's ABI has
>> > quite a few uses and enough people wanted it to get the ball rolling.
>> >
>> > We agreed that it's desirable to have something that can be used from code
>> > rather than just a spec on paper both for validation and allowing other users
>> > (like fuzzers, userspace libraries, and various userspace tools) to build
>> > on that.
>> >
>> > What we ended up deciding on at plumbers is:
>> >
>> >  - I'll do a few kernel bits do demonstrate how we can validate the spec from
>> > the kernel.
>> >  - Dmitry Vyukov will provide a way to translate syzkaller's syscall
>> > documentation into something that can be easily used in the kernel and
>> > userspace.
>> >  - Various projects will attempt to integrate it to make sure that the
>> > framework works for them.
>> >
>> > Once those bits are done we can focus on getting the spec right, and we'll
>> > have a good way to validate our work both in userspace and in the kernel.
>> >
>> > This patchset is a basic draft of said kernel bits. I mostly want to make
>> > sure that Dmitry and myself are on the same page as to how integration will
>> > look like, but also to open it to criticism and suggestions (bikeshedding).
>>
>>
>> Looks like a good starting point!
>>
>> Do you have a git repo with this somewhere? I have problems applying
>> the patches, seems that my gmail messed them with some weird escaping.
>
> I've pushed it to https://git.kernel.org/cgit/linux/kernel/git/sashal/linux.git/log/?h=abi_spec ,
> will try to keep it updated.

Was paged out by personal matters. Now looking at your code and trying
to extend it to handle more cases.



>> Is the intention that these descriptions are written by hand, or
>> generated from some DSL?
>> I benefited from easier to write descriptions, also I changed several
>> times what code generator generates without changing descriptions.
>> However, an additional level of indirection in the form of code
>> generator introduces own pain to maintain. So I am not too strong
>> here.
>
> I would really to have the descriptions written in just *one* place, either
> by hand the way I did in that example, or in DSL. I understand your point about
> another level of indirection, but I'm afraid that if we don't force a monolithic
> spec we'll end up with way more than 2 different descriptions to maintain.


Yes, it absolutely must be written in just one place.
What I meant is that we write DSL that is also checked into kernel
tree. And then whenever the descriptions change, we also regenerate
code. So that C code is 100% auto generated and never changed by hand.

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[Dmitry Vyukov]

On Wed, Nov 23, 2016 at 3:59 PM,  <alexander.levin@verizon.com> wrote:
> On Mon, Nov 21, 2016 at 03:48:17PM +0100, Dmitry Vyukov wrote:
>> Several observations based on my experience with syzkaller descriptions:
>>  - there are 2 levels: physical and logical;
>>    on physical level there are int, pointer, array, struct, union;
>>    and that's pretty much it.
>>    on logical level there are flags, bitmasks, file paths, sctp socket fds,
>>    unix socket names, etc.
>>    These levels are almost completely orthogonal. It would be useful to
>>    clearly separate them on description level. E.g. now you have TYPE_PTR and
>>    TYPE_INT which is physical level; and then TYPE_FD which is also an int.
>>
>>  - logical types won't fit into 64 bits, there are more of them
>
> I agree with your two points above.
>
> As an example, let's look at:
>
>         int epoll_ctl(int epfd, int op, int fd, struct epoll_event *event);
>
> epfd would be a physical int, logical "epoll_fd", and constrainted with being
> an open descriptor.
>
> fd on the other hand is tricky: it's a physical int, logical fd, and
> constrainted with being a file descriptor that supports poll, and being
> open.
>
> So while I think that logical types can be just a counter rather than a bitmask
> I suspect that our constraints won't fit into 64 bits. Make 2 64 bit fields?

One observation is that there are just 5 physical types:
 - scalar
 - pointer
 - array
 - struct
 - union

The rest deals with what exactly "scalar" is in a particular case.

I don't yet have complete answer, as it somewhat intermixed with the
rest of questions.



>>  - we need support for recursive types (yes, there are linked lists in
>> kernel APIs)
>
> I imagine that this will be handled by specific logical type handlers we'll
> need to implement. Can you give me an example and I'll try to code that?

One example is te_oper_param here:
https://android.googlesource.com/kernel/tegra/+/android-tegra-3.10/security/tlk_driver/ote_protocol.h
next_ptr_user is a pointer to te_oper_param. Thus recursive definition.

Another example is snd_seq_ev_quote:
http://lxr.free-electrons.com/source/include/uapi/sound/asequencer.h#L194
it contains struct snd_seq_event *event and snd_seq_event recursively
contains snd_seq_ev_quote.

In all cases it is pointer recursion via structs.

Sometimes it wish that developers have to write formal descriptions in
a limited language upfront. That would probably eliminate lots of
weird one-off "see what I invented here" cases :)




>>  - we need support for input/output data
>>    currently syzkaller does this only on pointer level, i.e. you
>> attach direction to pointer target
>>    but that's not enough, frequently there is a struct where one field
>> is input and another is output
>
> Assuming it's "data", for intput we'll just need to check that the given
> length is readable and for output that the length is writable, no?

It also can be an fd in a struct field. If it's output (e.g. pipe),
then we must not check that it's valid on entry. But we may want to
check that it's valid on successful exit, or fuzzer will use these
output fd's as inputs to other calls.


> We can do it with constraints right now.
>
>>  - we may need support for reusing types in several arguments
>>    e.g. you may have a pretty complex type, and you don't want to
>> write it out a dozen of times
>
> Yup, so if we go with the physical/logical split we can have handlers for
> logical types.
>
>>  - we need some support for discriminated syscalls
>>    if we want to support strace usecase, the support needs to be more
>> extensive than what syzkaller has;
>>    i.e. syzkaller can't restore discrimination having actual argument
>> values (it can do it only in the other direction)
>>
>>  - I would not create a special support for arguments;
>>    rather I would create support for structs and struct fields,
>>    and then pretend that a syscalls effectively accepts a struct by value
>
> But that means I need a custom handler for every syscall to parse the
> struct fields rather than a generic code that goes through the args and calls
> the right handler?

No, you don't. We will need generic code that parses a piece of memory
as a struct and splits it into fields anyway.
We can just reuse this code to handle syscall arguments as follows.
Describe syscall arguments as a pseudo struct (array of fields). Then
syscall handling function accepts pointer to region of memory with
arguments and description of the struct, and invokes common struct
handling code.



>> How would you like us to collaborate on this?
>> If you share your git repo, I could form it into something that would
>> be suitable for syzkaller and incorporate most of the above.
>
> I'd really like to have something that either generates these descriptions from
> your DSL (it really doesn't have to be perfect (at first)) or something that
> generates DSL from these C structs.

Do you mean generating C from my DSL of a one-off or as a permanent solution?

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[Dmitry Vyukov]

On Mon, Dec 12, 2016 at 11:29 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Wed, Nov 23, 2016 at 3:59 PM,  <alexander.levin@verizon.com> wrote:
>> On Mon, Nov 21, 2016 at 03:48:17PM +0100, Dmitry Vyukov wrote:
>>> Several observations based on my experience with syzkaller descriptions:
>>>  - there are 2 levels: physical and logical;
>>>    on physical level there are int, pointer, array, struct, union;
>>>    and that's pretty much it.
>>>    on logical level there are flags, bitmasks, file paths, sctp socket fds,
>>>    unix socket names, etc.
>>>    These levels are almost completely orthogonal. It would be useful to
>>>    clearly separate them on description level. E.g. now you have TYPE_PTR and
>>>    TYPE_INT which is physical level; and then TYPE_FD which is also an int.
>>>
>>>  - logical types won't fit into 64 bits, there are more of them
>>
>> I agree with your two points above.
>>
>> As an example, let's look at:
>>
>>         int epoll_ctl(int epfd, int op, int fd, struct epoll_event *event);
>>
>> epfd would be a physical int, logical "epoll_fd", and constrainted with being
>> an open descriptor.
>>
>> fd on the other hand is tricky: it's a physical int, logical fd, and
>> constrainted with being a file descriptor that supports poll, and being
>> open.
>>
>> So while I think that logical types can be just a counter rather than a bitmask
>> I suspect that our constraints won't fit into 64 bits. Make 2 64 bit fields?
>
> One observation is that there are just 5 physical types:
>  - scalar
>  - pointer
>  - array
>  - struct
>  - union
>
> The rest deals with what exactly "scalar" is in a particular case.
>
> I don't yet have complete answer, as it somewhat intermixed with the
> rest of questions.
>
>
>
>>>  - we need support for recursive types (yes, there are linked lists in
>>> kernel APIs)
>>
>> I imagine that this will be handled by specific logical type handlers we'll
>> need to implement. Can you give me an example and I'll try to code that?
>
> One example is te_oper_param here:
> https://android.googlesource.com/kernel/tegra/+/android-tegra-3.10/security/tlk_driver/ote_protocol.h
> next_ptr_user is a pointer to te_oper_param. Thus recursive definition.
>
> Another example is snd_seq_ev_quote:
> http://lxr.free-electrons.com/source/include/uapi/sound/asequencer.h#L194
> it contains struct snd_seq_event *event and snd_seq_event recursively
> contains snd_seq_ev_quote.
>
> In all cases it is pointer recursion via structs.
>
> Sometimes it wish that developers have to write formal descriptions in
> a limited language upfront. That would probably eliminate lots of
> weird one-off "see what I invented here" cases :)
>
>
>
>
>>>  - we need support for input/output data
>>>    currently syzkaller does this only on pointer level, i.e. you
>>> attach direction to pointer target
>>>    but that's not enough, frequently there is a struct where one field
>>> is input and another is output
>>
>> Assuming it's "data", for intput we'll just need to check that the given
>> length is readable and for output that the length is writable, no?
>
> It also can be an fd in a struct field. If it's output (e.g. pipe),
> then we must not check that it's valid on entry. But we may want to
> check that it's valid on successful exit, or fuzzer will use these
> output fd's as inputs to other calls.
>
>
>> We can do it with constraints right now.
>>
>>>  - we may need support for reusing types in several arguments
>>>    e.g. you may have a pretty complex type, and you don't want to
>>> write it out a dozen of times
>>
>> Yup, so if we go with the physical/logical split we can have handlers for
>> logical types.
>>
>>>  - we need some support for discriminated syscalls
>>>    if we want to support strace usecase, the support needs to be more
>>> extensive than what syzkaller has;
>>>    i.e. syzkaller can't restore discrimination having actual argument
>>> values (it can do it only in the other direction)
>>>
>>>  - I would not create a special support for arguments;
>>>    rather I would create support for structs and struct fields,
>>>    and then pretend that a syscalls effectively accepts a struct by value
>>
>> But that means I need a custom handler for every syscall to parse the
>> struct fields rather than a generic code that goes through the args and calls
>> the right handler?
>
> No, you don't. We will need generic code that parses a piece of memory
> as a struct and splits it into fields anyway.
> We can just reuse this code to handle syscall arguments as follows.
> Describe syscall arguments as a pseudo struct (array of fields). Then
> syscall handling function accepts pointer to region of memory with
> arguments and description of the struct, and invokes common struct
> handling code.
>
>
>
>>> How would you like us to collaborate on this?
>>> If you share your git repo, I could form it into something that would
>>> be suitable for syzkaller and incorporate most of the above.
>>
>> I'd really like to have something that either generates these descriptions from
>> your DSL (it really doesn't have to be perfect (at first)) or something that
>> generates DSL from these C structs.
>
> Do you mean generating C from my DSL of a one-off or as a permanent solution?


Main problem I am trying to resolve now is how to make types reference
other types.
Say we have "pointer to pointer to int" (the same applies to structs
and arrays). This means that struct type must be able to reference
another instance of struct type. But we can't put type into type by
value. Namely, the following is not possible:

struct type {
  int kind;
  union {
    ...
    // for kind = KIND_PTR
    struct {
      struct type type;
    } ptr;
  };
};

One obvious solution would be to always reference _pointers_ to types. E.g.:

    // for kind = KIND_PTR
    struct {
      struct type* type;
    } ptr;

This works but leads to super-verbose descriptions (if we want to use
static tables), because we need to describe type of each syscall
argument and inner types for all pointers/arrays/structs/unions as
separate global variables:


static struct type open_arg0_inner {
  .kind = KIND_ARRAY,
  ...
};

static struct type open_arg0 {
  .kind = KIND_PTR,
  .ptr.type = &open_arg0_inner
  ...
};

static struct type open_arg1 {
  .kind = KIND_SCALAR,
  ...
};

static struct type open_arg2 {
  .kind = KIND_SCALAR,
  ...
};

static struct type open_ret {
  .kind = KIND_SCALAR,
  ...
};

struct syscall_spec syscall_spec_open = {
  .name = "open",
  .retval = {
    .name = "retval",
    .type = &open_ret,
   }
  .nargs = 3,
  .args[0] = {
    .name = "pathname",
    .type = &open_arg0,
  }
  .args[1] = {
    .name = "flags",
    .type = &open_arg1,
  }
  .args[2] = {
    .name = "mode",
    .type = &open_arg2,
  }
};


This looks way too verbose provided that we write these descriptions
by hand (just coming up with consistent unique names for all these
global vars is a problem).
If we generate that from DSL, it can work though.

Another option I am considering is to use helper construction
functions that return pointers to types, e.g. something along these
lines:

struct syscall_spec syscall_spec_open = {
  .name = "open",
  .retval = {
    .name = "retval",
    .type = type_scalar(FD | ERRNO),
   }
  .nargs = 3,
  .args[0] = {
    .name = "pathname",
    .type = type_ptr(DIR_IN, type_array(PATHNAME)),
  }
  .args[1] = {
    .name = "flags",
    .type = type_scalar_flags(O_RDONLY | O_WRONLY, ....),
  }
  .args[2] = {
    .name = "mode",
    .type = type_scalar_flags(S_IRWXU ....),
  }
};

Now these table can only be initialized dynamically. And we will
probably need lots of these helper functions. So I don't like it
either...

Any suggestions?

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[Dmitry Vyukov]

On Mon, Dec 12, 2016 at 11:45 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Mon, Dec 12, 2016 at 11:29 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Wed, Nov 23, 2016 at 3:59 PM,  <alexander.levin@verizon.com> wrote:
>>> On Mon, Nov 21, 2016 at 03:48:17PM +0100, Dmitry Vyukov wrote:
>>>> Several observations based on my experience with syzkaller descriptions:
>>>>  - there are 2 levels: physical and logical;
>>>>    on physical level there are int, pointer, array, struct, union;
>>>>    and that's pretty much it.
>>>>    on logical level there are flags, bitmasks, file paths, sctp socket fds,
>>>>    unix socket names, etc.
>>>>    These levels are almost completely orthogonal. It would be useful to
>>>>    clearly separate them on description level. E.g. now you have TYPE_PTR and
>>>>    TYPE_INT which is physical level; and then TYPE_FD which is also an int.
>>>>
>>>>  - logical types won't fit into 64 bits, there are more of them
>>>
>>> I agree with your two points above.
>>>
>>> As an example, let's look at:
>>>
>>>         int epoll_ctl(int epfd, int op, int fd, struct epoll_event *event);
>>>
>>> epfd would be a physical int, logical "epoll_fd", and constrainted with being
>>> an open descriptor.
>>>
>>> fd on the other hand is tricky: it's a physical int, logical fd, and
>>> constrainted with being a file descriptor that supports poll, and being
>>> open.
>>>
>>> So while I think that logical types can be just a counter rather than a bitmask
>>> I suspect that our constraints won't fit into 64 bits. Make 2 64 bit fields?
>>
>> One observation is that there are just 5 physical types:
>>  - scalar
>>  - pointer
>>  - array
>>  - struct
>>  - union
>>
>> The rest deals with what exactly "scalar" is in a particular case.
>>
>> I don't yet have complete answer, as it somewhat intermixed with the
>> rest of questions.
>>
>>
>>
>>>>  - we need support for recursive types (yes, there are linked lists in
>>>> kernel APIs)
>>>
>>> I imagine that this will be handled by specific logical type handlers we'll
>>> need to implement. Can you give me an example and I'll try to code that?
>>
>> One example is te_oper_param here:
>> https://android.googlesource.com/kernel/tegra/+/android-tegra-3.10/security/tlk_driver/ote_protocol.h
>> next_ptr_user is a pointer to te_oper_param. Thus recursive definition.
>>
>> Another example is snd_seq_ev_quote:
>> http://lxr.free-electrons.com/source/include/uapi/sound/asequencer.h#L194
>> it contains struct snd_seq_event *event and snd_seq_event recursively
>> contains snd_seq_ev_quote.
>>
>> In all cases it is pointer recursion via structs.
>>
>> Sometimes it wish that developers have to write formal descriptions in
>> a limited language upfront. That would probably eliminate lots of
>> weird one-off "see what I invented here" cases :)
>>
>>
>>
>>
>>>>  - we need support for input/output data
>>>>    currently syzkaller does this only on pointer level, i.e. you
>>>> attach direction to pointer target
>>>>    but that's not enough, frequently there is a struct where one field
>>>> is input and another is output
>>>
>>> Assuming it's "data", for intput we'll just need to check that the given
>>> length is readable and for output that the length is writable, no?
>>
>> It also can be an fd in a struct field. If it's output (e.g. pipe),
>> then we must not check that it's valid on entry. But we may want to
>> check that it's valid on successful exit, or fuzzer will use these
>> output fd's as inputs to other calls.
>>
>>
>>> We can do it with constraints right now.
>>>
>>>>  - we may need support for reusing types in several arguments
>>>>    e.g. you may have a pretty complex type, and you don't want to
>>>> write it out a dozen of times
>>>
>>> Yup, so if we go with the physical/logical split we can have handlers for
>>> logical types.
>>>
>>>>  - we need some support for discriminated syscalls
>>>>    if we want to support strace usecase, the support needs to be more
>>>> extensive than what syzkaller has;
>>>>    i.e. syzkaller can't restore discrimination having actual argument
>>>> values (it can do it only in the other direction)
>>>>
>>>>  - I would not create a special support for arguments;
>>>>    rather I would create support for structs and struct fields,
>>>>    and then pretend that a syscalls effectively accepts a struct by value
>>>
>>> But that means I need a custom handler for every syscall to parse the
>>> struct fields rather than a generic code that goes through the args and calls
>>> the right handler?
>>
>> No, you don't. We will need generic code that parses a piece of memory
>> as a struct and splits it into fields anyway.
>> We can just reuse this code to handle syscall arguments as follows.
>> Describe syscall arguments as a pseudo struct (array of fields). Then
>> syscall handling function accepts pointer to region of memory with
>> arguments and description of the struct, and invokes common struct
>> handling code.
>>
>>
>>
>>>> How would you like us to collaborate on this?
>>>> If you share your git repo, I could form it into something that would
>>>> be suitable for syzkaller and incorporate most of the above.
>>>
>>> I'd really like to have something that either generates these descriptions from
>>> your DSL (it really doesn't have to be perfect (at first)) or something that
>>> generates DSL from these C structs.
>>
>> Do you mean generating C from my DSL of a one-off or as a permanent solution?
>
>
> Main problem I am trying to resolve now is how to make types reference
> other types.
> Say we have "pointer to pointer to int" (the same applies to structs
> and arrays). This means that struct type must be able to reference
> another instance of struct type. But we can't put type into type by
> value. Namely, the following is not possible:
>
> struct type {
>   int kind;
>   union {
>     ...
>     // for kind = KIND_PTR
>     struct {
>       struct type type;
>     } ptr;
>   };
> };
>
> One obvious solution would be to always reference _pointers_ to types. E.g.:
>
>     // for kind = KIND_PTR
>     struct {
>       struct type* type;
>     } ptr;
>
> This works but leads to super-verbose descriptions (if we want to use
> static tables), because we need to describe type of each syscall
> argument and inner types for all pointers/arrays/structs/unions as
> separate global variables:
>
>
> static struct type open_arg0_inner {
>   .kind = KIND_ARRAY,
>   ...
> };
>
> static struct type open_arg0 {
>   .kind = KIND_PTR,
>   .ptr.type = &open_arg0_inner
>   ...
> };
>
> static struct type open_arg1 {
>   .kind = KIND_SCALAR,
>   ...
> };
>
> static struct type open_arg2 {
>   .kind = KIND_SCALAR,
>   ...
> };
>
> static struct type open_ret {
>   .kind = KIND_SCALAR,
>   ...
> };
>
> struct syscall_spec syscall_spec_open = {
>   .name = "open",
>   .retval = {
>     .name = "retval",
>     .type = &open_ret,
>    }
>   .nargs = 3,
>   .args[0] = {
>     .name = "pathname",
>     .type = &open_arg0,
>   }
>   .args[1] = {
>     .name = "flags",
>     .type = &open_arg1,
>   }
>   .args[2] = {
>     .name = "mode",
>     .type = &open_arg2,
>   }
> };
>
>
> This looks way too verbose provided that we write these descriptions
> by hand (just coming up with consistent unique names for all these
> global vars is a problem).
> If we generate that from DSL, it can work though.
>
> Another option I am considering is to use helper construction
> functions that return pointers to types, e.g. something along these
> lines:
>
> struct syscall_spec syscall_spec_open = {
>   .name = "open",
>   .retval = {
>     .name = "retval",
>     .type = type_scalar(FD | ERRNO),
>    }
>   .nargs = 3,
>   .args[0] = {
>     .name = "pathname",
>     .type = type_ptr(DIR_IN, type_array(PATHNAME)),
>   }
>   .args[1] = {
>     .name = "flags",
>     .type = type_scalar_flags(O_RDONLY | O_WRONLY, ....),
>   }
>   .args[2] = {
>     .name = "mode",
>     .type = type_scalar_flags(S_IRWXU ....),
>   }
> };
>
> Now these table can only be initialized dynamically. And we will
> probably need lots of these helper functions. So I don't like it
> either...
>
> Any suggestions?



Here is my current prototype:
https://github.com/dvyukov/linux/commit/6200a9333e78bef393f8ead41205813b94d340f3

For now it can trace arguments of 4 system calls:

[    4.055483] [pid 1258] open(&00007ffdefc023a0=[], 0x0, 0x1b6)
[    4.055991] [pid 1258] open(&00007ffdefc023a0=[], 0x0, 0x1b6) = 3
[    4.056486] [pid 1258] read(0x3, &00007ffdefc01320=[], 0x1000)
[    4.056977] [pid 1258] read(0x3, &00007ffdefc01320=[], 0x1000) = 1780
[    4.057485] [pid 1258] read(0x3, &00007f316a732000=[], 0x1000)
[    4.057991] [pid 1258] read(0x3, &00007f316a732000=[], 0x1000) = 0
[    4.058488] [pid 1258] close(0x0) = 0
[    4.058848] [pid 1258] write(0x1, &00007f316a732000=[], 0x5)
[    4.059304] [pid 1258] write(0x1, &00007f316a732000=[], 0x5) = 5
[    4.059905] [pid 1234] close(0x0) = 0
[    4.060239] [pid 1234] close(0x0) = 0


Main outstanding problems:
 - understanding length of arrays and buffers
 - understanding discriminators of unions and syscall variations

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[Dmitry Vyukov]

On Wed, Dec 14, 2016 at 8:46 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Mon, Dec 12, 2016 at 11:45 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Mon, Dec 12, 2016 at 11:29 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> On Wed, Nov 23, 2016 at 3:59 PM,  <alexander.levin@verizon.com> wrote:
>>>> On Mon, Nov 21, 2016 at 03:48:17PM +0100, Dmitry Vyukov wrote:
>>>>> Several observations based on my experience with syzkaller descriptions:
>>>>>  - there are 2 levels: physical and logical;
>>>>>    on physical level there are int, pointer, array, struct, union;
>>>>>    and that's pretty much it.
>>>>>    on logical level there are flags, bitmasks, file paths, sctp socket fds,
>>>>>    unix socket names, etc.
>>>>>    These levels are almost completely orthogonal. It would be useful to
>>>>>    clearly separate them on description level. E.g. now you have TYPE_PTR and
>>>>>    TYPE_INT which is physical level; and then TYPE_FD which is also an int.
>>>>>
>>>>>  - logical types won't fit into 64 bits, there are more of them
>>>>
>>>> I agree with your two points above.
>>>>
>>>> As an example, let's look at:
>>>>
>>>>         int epoll_ctl(int epfd, int op, int fd, struct epoll_event *event);
>>>>
>>>> epfd would be a physical int, logical "epoll_fd", and constrainted with being
>>>> an open descriptor.
>>>>
>>>> fd on the other hand is tricky: it's a physical int, logical fd, and
>>>> constrainted with being a file descriptor that supports poll, and being
>>>> open.
>>>>
>>>> So while I think that logical types can be just a counter rather than a bitmask
>>>> I suspect that our constraints won't fit into 64 bits. Make 2 64 bit fields?
>>>
>>> One observation is that there are just 5 physical types:
>>>  - scalar
>>>  - pointer
>>>  - array
>>>  - struct
>>>  - union
>>>
>>> The rest deals with what exactly "scalar" is in a particular case.
>>>
>>> I don't yet have complete answer, as it somewhat intermixed with the
>>> rest of questions.
>>>
>>>
>>>
>>>>>  - we need support for recursive types (yes, there are linked lists in
>>>>> kernel APIs)
>>>>
>>>> I imagine that this will be handled by specific logical type handlers we'll
>>>> need to implement. Can you give me an example and I'll try to code that?
>>>
>>> One example is te_oper_param here:
>>> https://android.googlesource.com/kernel/tegra/+/android-tegra-3.10/security/tlk_driver/ote_protocol.h
>>> next_ptr_user is a pointer to te_oper_param. Thus recursive definition.
>>>
>>> Another example is snd_seq_ev_quote:
>>> http://lxr.free-electrons.com/source/include/uapi/sound/asequencer.h#L194
>>> it contains struct snd_seq_event *event and snd_seq_event recursively
>>> contains snd_seq_ev_quote.
>>>
>>> In all cases it is pointer recursion via structs.
>>>
>>> Sometimes it wish that developers have to write formal descriptions in
>>> a limited language upfront. That would probably eliminate lots of
>>> weird one-off "see what I invented here" cases :)
>>>
>>>
>>>
>>>
>>>>>  - we need support for input/output data
>>>>>    currently syzkaller does this only on pointer level, i.e. you
>>>>> attach direction to pointer target
>>>>>    but that's not enough, frequently there is a struct where one field
>>>>> is input and another is output
>>>>
>>>> Assuming it's "data", for intput we'll just need to check that the given
>>>> length is readable and for output that the length is writable, no?
>>>
>>> It also can be an fd in a struct field. If it's output (e.g. pipe),
>>> then we must not check that it's valid on entry. But we may want to
>>> check that it's valid on successful exit, or fuzzer will use these
>>> output fd's as inputs to other calls.
>>>
>>>
>>>> We can do it with constraints right now.
>>>>
>>>>>  - we may need support for reusing types in several arguments
>>>>>    e.g. you may have a pretty complex type, and you don't want to
>>>>> write it out a dozen of times
>>>>
>>>> Yup, so if we go with the physical/logical split we can have handlers for
>>>> logical types.
>>>>
>>>>>  - we need some support for discriminated syscalls
>>>>>    if we want to support strace usecase, the support needs to be more
>>>>> extensive than what syzkaller has;
>>>>>    i.e. syzkaller can't restore discrimination having actual argument
>>>>> values (it can do it only in the other direction)
>>>>>
>>>>>  - I would not create a special support for arguments;
>>>>>    rather I would create support for structs and struct fields,
>>>>>    and then pretend that a syscalls effectively accepts a struct by value
>>>>
>>>> But that means I need a custom handler for every syscall to parse the
>>>> struct fields rather than a generic code that goes through the args and calls
>>>> the right handler?
>>>
>>> No, you don't. We will need generic code that parses a piece of memory
>>> as a struct and splits it into fields anyway.
>>> We can just reuse this code to handle syscall arguments as follows.
>>> Describe syscall arguments as a pseudo struct (array of fields). Then
>>> syscall handling function accepts pointer to region of memory with
>>> arguments and description of the struct, and invokes common struct
>>> handling code.
>>>
>>>
>>>
>>>>> How would you like us to collaborate on this?
>>>>> If you share your git repo, I could form it into something that would
>>>>> be suitable for syzkaller and incorporate most of the above.
>>>>
>>>> I'd really like to have something that either generates these descriptions from
>>>> your DSL (it really doesn't have to be perfect (at first)) or something that
>>>> generates DSL from these C structs.
>>>
>>> Do you mean generating C from my DSL of a one-off or as a permanent solution?
>>
>>
>> Main problem I am trying to resolve now is how to make types reference
>> other types.
>> Say we have "pointer to pointer to int" (the same applies to structs
>> and arrays). This means that struct type must be able to reference
>> another instance of struct type. But we can't put type into type by
>> value. Namely, the following is not possible:
>>
>> struct type {
>>   int kind;
>>   union {
>>     ...
>>     // for kind = KIND_PTR
>>     struct {
>>       struct type type;
>>     } ptr;
>>   };
>> };
>>
>> One obvious solution would be to always reference _pointers_ to types. E.g.:
>>
>>     // for kind = KIND_PTR
>>     struct {
>>       struct type* type;
>>     } ptr;
>>
>> This works but leads to super-verbose descriptions (if we want to use
>> static tables), because we need to describe type of each syscall
>> argument and inner types for all pointers/arrays/structs/unions as
>> separate global variables:
>>
>>
>> static struct type open_arg0_inner {
>>   .kind = KIND_ARRAY,
>>   ...
>> };
>>
>> static struct type open_arg0 {
>>   .kind = KIND_PTR,
>>   .ptr.type = &open_arg0_inner
>>   ...
>> };
>>
>> static struct type open_arg1 {
>>   .kind = KIND_SCALAR,
>>   ...
>> };
>>
>> static struct type open_arg2 {
>>   .kind = KIND_SCALAR,
>>   ...
>> };
>>
>> static struct type open_ret {
>>   .kind = KIND_SCALAR,
>>   ...
>> };
>>
>> struct syscall_spec syscall_spec_open = {
>>   .name = "open",
>>   .retval = {
>>     .name = "retval",
>>     .type = &open_ret,
>>    }
>>   .nargs = 3,
>>   .args[0] = {
>>     .name = "pathname",
>>     .type = &open_arg0,
>>   }
>>   .args[1] = {
>>     .name = "flags",
>>     .type = &open_arg1,
>>   }
>>   .args[2] = {
>>     .name = "mode",
>>     .type = &open_arg2,
>>   }
>> };
>>
>>
>> This looks way too verbose provided that we write these descriptions
>> by hand (just coming up with consistent unique names for all these
>> global vars is a problem).
>> If we generate that from DSL, it can work though.
>>
>> Another option I am considering is to use helper construction
>> functions that return pointers to types, e.g. something along these
>> lines:
>>
>> struct syscall_spec syscall_spec_open = {
>>   .name = "open",
>>   .retval = {
>>     .name = "retval",
>>     .type = type_scalar(FD | ERRNO),
>>    }
>>   .nargs = 3,
>>   .args[0] = {
>>     .name = "pathname",
>>     .type = type_ptr(DIR_IN, type_array(PATHNAME)),
>>   }
>>   .args[1] = {
>>     .name = "flags",
>>     .type = type_scalar_flags(O_RDONLY | O_WRONLY, ....),
>>   }
>>   .args[2] = {
>>     .name = "mode",
>>     .type = type_scalar_flags(S_IRWXU ....),
>>   }
>> };
>>
>> Now these table can only be initialized dynamically. And we will
>> probably need lots of these helper functions. So I don't like it
>> either...
>>
>> Any suggestions?
>
>
>
> Here is my current prototype:
> https://github.com/dvyukov/linux/commit/6200a9333e78bef393f8ead41205813b94d340f3
>
> For now it can trace arguments of 4 system calls:
>
> [    4.055483] [pid 1258] open(&00007ffdefc023a0=[], 0x0, 0x1b6)
> [    4.055991] [pid 1258] open(&00007ffdefc023a0=[], 0x0, 0x1b6) = 3
> [    4.056486] [pid 1258] read(0x3, &00007ffdefc01320=[], 0x1000)
> [    4.056977] [pid 1258] read(0x3, &00007ffdefc01320=[], 0x1000) = 1780
> [    4.057485] [pid 1258] read(0x3, &00007f316a732000=[], 0x1000)
> [    4.057991] [pid 1258] read(0x3, &00007f316a732000=[], 0x1000) = 0
> [    4.058488] [pid 1258] close(0x0) = 0
> [    4.058848] [pid 1258] write(0x1, &00007f316a732000=[], 0x5)
> [    4.059304] [pid 1258] write(0x1, &00007f316a732000=[], 0x5) = 5
> [    4.059905] [pid 1234] close(0x0) = 0
> [    4.060239] [pid 1234] close(0x0) = 0
>
>
> Main outstanding problems:
>  - understanding length of arrays and buffers
>  - understanding discriminators of unions and syscall variations



Here is open description:

struct syscall_spec syscall_spec_open = {
        .name = "open",
        .errno = {EACCES, EDQUOT, EEXIST, EFAULT, EFBIG, EINTR, EINVAL, EISDIR,
                ELOOP, EMFILE, ENAMETOOLONG, ENFILE, ENODEV, ENOENT, ENOMEM,
                ENOSPC, ENOTDIR, ENXIO, EOVERFLOW, EPERM, EROFS, ETXTBSY,
                EWOULDBLOCK},
        .ret = &type_fd,
        .args = {
                {"pathname", &type_ptr_pathname},
                {"flags", &type_open_flags},
                {"mode", &type_open_mode},
        },
};

Types are declares outline as:

static struct type type_open_mode = {
        .kind = KIND_SCALAR,
        .scalar.size = 4,
        .scalar.constraint = CONSTRAINT_BITMASK,
        .scalar.bitmask = {$(S_IRUSR), $(S_IWUSR), $(S_IXUSR),
                $(S_IRGRP), $(S_IWGRP), $(S_IXGRP),
                $(S_IROTH), $(S_IWOTH), $(S_IXOTH)},
};

static struct type type_fd = {
        .kind = KIND_RESOURCE,
        .res.res = RES_FD,
        .res.type = &type_i32,
};

static struct type type_array_i8 = {
        .kind = KIND_ARRAY,
        .array.type = &type_i8,
};

static struct type type_ptr_array_i8 = {
        .kind = KIND_PTR,
        .ptr.type = &type_array_i8,
};

etc

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[alexander.levin]

On Mon, Dec 12, 2016 at 11:29:35AM +0100, Dmitry Vyukov wrote:
> On Wed, Nov 23, 2016 at 3:59 PM,  <alexander.levin@verizon.com> wrote:
> > On Mon, Nov 21, 2016 at 03:48:17PM +0100, Dmitry Vyukov wrote:
> > I imagine that this will be handled by specific logical type handlers we'll
> > need to implement. Can you give me an example and I'll try to code that?
> 
> One example is te_oper_param here:
> https://android.googlesource.com/kernel/tegra/+/android-tegra-3.10/security/tlk_driver/ote_protocol.h
> next_ptr_user is a pointer to te_oper_param. Thus recursive definition.
> 
> Another example is snd_seq_ev_quote:
> http://lxr.free-electrons.com/source/include/uapi/sound/asequencer.h#L194
> it contains struct snd_seq_event *event and snd_seq_event recursively
> contains snd_seq_ev_quote.
> 
> In all cases it is pointer recursion via structs.
> 
> Sometimes it wish that developers have to write formal descriptions in
> a limited language upfront. That would probably eliminate lots of
> weird one-off "see what I invented here" cases :)

We'll need a special handler for each struct passed as a parameter anyway, no?

In this scenario we just call that handler recursively?
 
> > But that means I need a custom handler for every syscall to parse the
> > struct fields rather than a generic code that goes through the args and calls
> > the right handler?
> 
> No, you don't. We will need generic code that parses a piece of memory
> as a struct and splits it into fields anyway.
> We can just reuse this code to handle syscall arguments as follows.
> Describe syscall arguments as a pseudo struct (array of fields). Then
> syscall handling function accepts pointer to region of memory with
> arguments and description of the struct, and invokes common struct
> handling code.

Oh, I understand your point now - I missed the part about a generic fields
struct and thought you want a specialized struct for each syscall.

Yes, that makes sense.
 
> 
> >> How would you like us to collaborate on this?
> >> If you share your git repo, I could form it into something that would
> >> be suitable for syzkaller and incorporate most of the above.
> >
> > I'd really like to have something that either generates these descriptions from
> > your DSL (it really doesn't have to be perfect (at first)) or something that
> > generates DSL from these C structs.
> 
> Do you mean generating C from my DSL of a one-off or as a permanent solution?

I think it depends on what's easier to maintain - the spec in C or a parser
for DSL->C.

-- 

Thanks,
Sasha

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[Dmitry Vyukov]

On Tue, Dec 27, 2016 at 6:23 PM,  <alexander.levin@verizon.com> wrote:
> On Mon, Dec 12, 2016 at 11:29:35AM +0100, Dmitry Vyukov wrote:
>> On Wed, Nov 23, 2016 at 3:59 PM,  <alexander.levin@verizon.com> wrote:
>> > On Mon, Nov 21, 2016 at 03:48:17PM +0100, Dmitry Vyukov wrote:
>> > I imagine that this will be handled by specific logical type handlers we'll
>> > need to implement. Can you give me an example and I'll try to code that?
>>
>> One example is te_oper_param here:
>> https://android.googlesource.com/kernel/tegra/+/android-tegra-3.10/security/tlk_driver/ote_protocol.h
>> next_ptr_user is a pointer to te_oper_param. Thus recursive definition.
>>
>> Another example is snd_seq_ev_quote:
>> http://lxr.free-electrons.com/source/include/uapi/sound/asequencer.h#L194
>> it contains struct snd_seq_event *event and snd_seq_event recursively
>> contains snd_seq_ev_quote.
>>
>> In all cases it is pointer recursion via structs.
>>
>> Sometimes it wish that developers have to write formal descriptions in
>> a limited language upfront. That would probably eliminate lots of
>> weird one-off "see what I invented here" cases :)
>
> We'll need a special handler for each struct passed as a parameter anyway, no?
>
> In this scenario we just call that handler recursively?

Yes, but you can't describe it the way you started describing
arguments, because you can't put struct type into struct type by
value.


>> > But that means I need a custom handler for every syscall to parse the
>> > struct fields rather than a generic code that goes through the args and calls
>> > the right handler?
>>
>> No, you don't. We will need generic code that parses a piece of memory
>> as a struct and splits it into fields anyway.
>> We can just reuse this code to handle syscall arguments as follows.
>> Describe syscall arguments as a pseudo struct (array of fields). Then
>> syscall handling function accepts pointer to region of memory with
>> arguments and description of the struct, and invokes common struct
>> handling code.
>
> Oh, I understand your point now - I missed the part about a generic fields
> struct and thought you want a specialized struct for each syscall.
>
> Yes, that makes sense.
>
>>
>> >> How would you like us to collaborate on this?
>> >> If you share your git repo, I could form it into something that would
>> >> be suitable for syzkaller and incorporate most of the above.
>> >
>> > I'd really like to have something that either generates these descriptions from
>> > your DSL (it really doesn't have to be perfect (at first)) or something that
>> > generates DSL from these C structs.
>>
>> Do you mean generating C from my DSL of a one-off or as a permanent solution?
>
> I think it depends on what's easier to maintain - the spec in C or a parser
> for DSL->C.

Let's concentrate on the C part for now. Even with DSL we need to
understand how C should look like. We can decide on DSL later.

Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls

[alexander.levin]

[-- Attachment #1: Type: text/plain, Size: 3425 bytes --]

On Wed, Dec 14, 2016 at 08:46:25PM +0100, Dmitry Vyukov wrote:
> Here is my current prototype:
> https://github.com/dvyukov/linux/commit/6200a9333e78bef393f8ead41205813b94d340f3
> 
> For now it can trace arguments of 4 system calls:
> 
> [    4.055483] [pid 1258] open(&00007ffdefc023a0=[], 0x0, 0x1b6)
> [    4.055991] [pid 1258] open(&00007ffdefc023a0=[], 0x0, 0x1b6) = 3
> [    4.056486] [pid 1258] read(0x3, &00007ffdefc01320=[], 0x1000)
> [    4.056977] [pid 1258] read(0x3, &00007ffdefc01320=[], 0x1000) = 1780
> [    4.057485] [pid 1258] read(0x3, &00007f316a732000=[], 0x1000)
> [    4.057991] [pid 1258] read(0x3, &00007f316a732000=[], 0x1000) = 0
> [    4.058488] [pid 1258] close(0x0) = 0
> [    4.058848] [pid 1258] write(0x1, &00007f316a732000=[], 0x5)
> [    4.059304] [pid 1258] write(0x1, &00007f316a732000=[], 0x5) = 5
> [    4.059905] [pid 1234] close(0x0) = 0
> [    4.060239] [pid 1234] close(0x0) = 0
> 
> 
> Main outstanding problems:
>  - understanding length of arrays and buffers
>  - understanding discriminators of unions and syscall variations

Happy new year! I've been away for a bit myself, but now back working on this.

Attached a patch on top of your commit.

There are two things (very messy, I just want to go through the concept):

 - Reading the values into a generic fields struct, based on your suggestion.
There's no actual struct there, just the values - we can figure out how it'll
look like exactly, but something along this path makes sense?

tglx also raised a point that we want to read from userspace only once for
performance; it's a bit early to address performance at this stage, but it's
another advantage to pursuing this approach.

 - Array/string length. Since we read all values, we can point to the array's
length by using an offset from the currect arg. So for example, in read(), the
length of the buffer is at +1 offset from the buffer. This seems to be the case
for most syscalls.

The exception here is strings which we can just define as offset == 0.


With the patch, the trace is now able to work with strings:

[    1.234156] [pid 891] open(&00007fa7b35d4035=[ /etc/ld.so.cache ], 0x80000, 0x1)
[    1.235244] [pid 891] open(&00007fa7b35d4035=[ /etc/ld.so.cache ], 0x80000, 0x1) = -2
[    1.236101] [pid 891] open(&00007ffe57ca2a70=[ /lib/x86_64-linux-gnu/tls/x86_64/libc.so.6 ], 0x80000, 0xb37db168)
[    1.237361] [pid 891] open(&00007ffe57ca2a70=[ /lib/x86_64-linux-gnu/tls/x86_64/libc.so.6 ], 0x80000, 0xb37db168) = -2
[    1.238545] [pid 891] open(&00007ffe57ca2a70=[ /lib/x86_64-linux-gnu/tls/libc.so.6 ], 0x80000, 0xb37db168)
[    1.239600] [pid 891] open(&00007ffe57ca2a70=[ /lib/x86_64-linux-gnu/tls/libc.so.6 ], 0x80000, 0xb37db168) = -2
[    1.241033] [pid 891] open(&00007ffe57ca2a70=[ /lib/x86_64-linux-gnu/x86_64/libc.so.6 ], 0x80000, 0xb37db168)
[    1.242163] [pid 891] open(&00007ffe57ca2a70=[ /lib/x86_64-linux-gnu/x86_64/libc.so.6 ], 0x80000, 0xb37db168) = -2
[    1.243329] [pid 891] open(&00007ffe57ca2a70=[ /lib/x86_64-linux-gnu/libc.so.6 ], 0x80000, 0xb37db168)
[    1.244712] [pid 891] open(&00007ffe57ca2a70=[ /lib/x86_64-linux-gnu/libc.so.6 ], 0x80000, 0xb37db168) = 3
[    1.245633] [pid 891] read(0x3, &00007ffe57ca2c98=[ (null) ], 0x340)
[    1.246334] [pid 891] read(0x3, &00007ffe57ca2c98=[ (null) ], 0x340) = 832

Does the idea makes sense?

-- 

Thanks,
Sasha

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: diff.patch --]
[-- Type: text/x-diff; name="diff.patch", Size: 6635 bytes --]

diff --git a/include/uapi/linux/abi_spec.h b/include/uapi/linux/abi_spec.h
index dd8ddc3..21b31f9 100644
--- a/include/uapi/linux/abi_spec.h
+++ b/include/uapi/linux/abi_spec.h
@@ -80,6 +80,7 @@ struct type {
 		// KIND_ARRAY
 		struct {
 			struct type	*type;
+			u8		length_off;
 		} array;
 
 		// KIND_STRUCT
@@ -110,4 +111,15 @@ struct syscall_spec {
 	struct argument	args[ABI_MAX_ARGS];
 };
 
+struct syscall_values {
+	union {
+		u8 u8;
+		u16 u16;
+		u32 u32;
+		u64 u64;
+		void *ptr;
+		char *str;
+	};
+};
+
 #endif
diff --git a/kernel/abi_spec.c b/kernel/abi_spec.c
index fa249bd..dbafb98 100644
--- a/kernel/abi_spec.c
+++ b/kernel/abi_spec.c
@@ -7,10 +7,11 @@
 #include <linux/errno.h>
 #include <linux/fcntl.h>
 #include <linux/stat.h>
+#include <linux/vmalloc.h>
 
-typedef void (*cb_t)(void *ctx, struct type *t, int flags, const void __user *p, int post);
+typedef void (*cb_t)(void *ctx, struct type *t, int flags, const void __user *p, struct syscall_values *val, int post);
 
-static size_t handle_type(cb_t cb, void *ctx, struct type *t, int flags, const void __user *p);
+static size_t handle_type(cb_t cb, void *ctx, struct type *t, int flags, const void __user *p, struct syscall_values *val);
 
 static u64 read_val(struct type *t, int flags, const void __user *p)
 {
@@ -60,27 +61,34 @@ static u64 read_val(struct type *t, int flags, const void __user *p)
 	}
 }
 
-static size_t handle_type(cb_t cb, void *ctx, struct type *t, int flags, const void __user *p)
+static size_t handle_type(cb_t cb, void *ctx, struct type *t, int flags, const void __user *p, struct syscall_values *val)
 {
 	size_t off;
 
-	cb(ctx, t, flags, p, 0);
+	cb(ctx, t, flags, p, val, 0);
 	switch (t->kind) {
 	case KIND_SCALAR: {
 		off = t->scalar.size;
+		val->u64 = read_val(t, flags, p);
 		break;
 	}
 	case KIND_PTR: {
 		const void __user* p1;
 
 		p1 = (const void __user*)read_val(t, flags, p);
-		handle_type(cb, ctx, t->ptr.type, 0, p1);
+		handle_type(cb, ctx, t->ptr.type, 0, p1, val);
 		off = sizeof(void *);
 		break;
 	}
 	case KIND_ARRAY: {
-		// TODO: don't know the size...
-		// off = handle_array(cb, ctx, t, p);
+		// length_off == 0 => string
+		if (t->array.length_off == 0) {
+			u8 len = strnlen_user(p, 0x10000);
+			val->ptr = vmalloc(len);
+			if (WARN_ON(!val->ptr))
+				break;
+			strncpy_from_user(val->ptr, p, len);
+		} else {} // todo
 		break;
 	}
 	case KIND_STRUCT: {
@@ -95,8 +103,8 @@ static size_t handle_type(cb_t cb, void *ctx, struct type *t, int flags, const v
 			if (!f)
 				break;
 			if (i)
-				cb(ctx, NULL, 0, NULL, 0);
-			off += handle_type(cb, ctx, f, arg->flags, p + off);
+				cb(ctx, NULL, 0, NULL, val, 0);
+			off += handle_type(cb, ctx, f, arg->flags, p + off, val);
 		}
 		break;
 	}
@@ -109,13 +117,13 @@ static size_t handle_type(cb_t cb, void *ctx, struct type *t, int flags, const v
 		struct type *t1;
 
 		for (t1 = t; t1->kind == KIND_RESOURCE; t1 = t1->res.type) {}
-		off = handle_type(cb, ctx, t1, flags, p);
+		off = handle_type(cb, ctx, t1, flags, p, val);
 		break;
 	}
 	default:
 		BUG();
 	}
-	cb(ctx, t, flags, p, 1);
+	cb(ctx, t, flags, p, val, 1);
 	return off;
 }
 
@@ -125,6 +133,7 @@ static void handle_syscall(cb_t cb, void *ctx, struct syscall_spec *s, va_list *
 	struct argument *arg;
 	struct type *f;
 	long v;
+	struct syscall_values vals[ABI_MAX_ARGS] = { 0 };
 
 	for (i = 0; i < ABI_MAX_ARGS; i++) {
 		arg = &s->args[i];
@@ -132,9 +141,9 @@ static void handle_syscall(cb_t cb, void *ctx, struct syscall_spec *s, va_list *
 		if (!f)
 			break;
 		if (i)
-			cb(ctx, NULL, 0, NULL, 0);
+			cb(ctx, NULL, 0, NULL, &vals[i], 0);
 		v = va_arg(*ap, long);
-		handle_type(cb, ctx, f, arg->flags, &v);
+		handle_type(cb, ctx, f, arg->flags, &v, &vals[i]);
 	}
 }
 
@@ -148,7 +157,7 @@ static void check_retval(struct syscall_spec *s, long retval)
 		if (s->errno[i] == -retval)
 			return;
 	}
-	__WARN_printf("syscall %s returned unexpected error %ld",
+	WARN("syscall %s returned unexpected error %ld",
 		s->name, retval);
 }
 
@@ -169,7 +178,7 @@ void check_pre_printf(struct check_pre_ctx *ctx, const char *fmt, ...)
 	va_end(args);
 }
 
-void check_pre_cb(void *ctx, struct type *t, int flags, const void __user *p, int post)
+void check_pre_cb(void *ctx, struct type *t, int flags, const void __user *p, struct syscall_values *val, int post)
 {
 	if (!t) {
 		check_pre_printf(ctx, ", ");
@@ -188,7 +197,7 @@ void check_pre_cb(void *ctx, struct type *t, int flags, const void __user *p, in
 		check_pre_printf(ctx, "&%p=", (void*)read_val(t, flags, p));
 		break;
 	case KIND_ARRAY:
-		check_pre_printf(ctx, post ? "]" : "[");
+		check_pre_printf(ctx, post ? " %s ]" : "[", val->ptr);
 		break;
 	case KIND_STRUCT:
 		check_pre_printf(ctx, post ? "}" : "{");
@@ -287,9 +296,16 @@ static struct type type_iptr = {
 	.scalar.size = sizeof(void *),
 };
 
+static struct type type_string_i8 = {
+        .kind = KIND_ARRAY,
+        .array.type = &type_i8,
+        .array.length_off = 0,
+};
+
 static struct type type_array_i8 = {
 	.kind = KIND_ARRAY,
 	.array.type = &type_i8,
+	.array.length_off = 1,
 };
 
 static struct type type_ptr_array_i8 = {
@@ -300,7 +316,7 @@ static struct type type_ptr_array_i8 = {
 static struct type type_pathname = {
 	.kind = KIND_RESOURCE,
 	.res.res = RES_PATHNAME,
-	.res.type = &type_array_i8,
+	.res.type = &type_string_i8,
 };
 
 static struct type type_ptr_pathname = {
@@ -752,5 +768,15 @@ struct syscall_spec syscall_spec_sync_file_range2 = { .name = "sync_file_range2"
 struct syscall_spec syscall_spec_statfs64 = { .name = "statfs64" };
 struct syscall_spec syscall_spec_fstatfs64 = { .name = "fstatfs64" };
 struct syscall_spec syscall_spec_bdflush = { .name = "bdflush" };
+struct syscall_spec syscall_spec_sigaction = { .name = "sigaction" };
+struct syscall_spec syscall_spec_old_mmap = { .name = "old_mmap" };
+struct syscall_spec syscall_spec_truncate64 = { .name = "truncate64" };
+struct syscall_spec syscall_spec_ftruncate64 = { .name = "ftruncate64" };
+struct syscall_spec syscall_spec_stat64 = { .name = "stat64" };
+struct syscall_spec syscall_spec_lstat64 = { .name = "lstat64" };
+struct syscall_spec syscall_spec_fstat64 = { .name = "fstat64" };
+struct syscall_spec syscall_spec_fstatat64 = { .name = "fstatat64" };
+struct syscall_spec syscall_spec_fcntl64 = { .name = "fcntl64" };
+struct syscall_spec syscall_spec_old_select = { .name = "old_select" };
 
 #undef $