diff mbox

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

Message ID 20170104044830.l4tlvp6732nuef2a@sasha-lappy
State New
Headers show

Commit Message

Levin, Alexander (Sasha Levin) Jan. 4, 2017, 4:52 a.m. UTC 
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
diff mbox

Patch

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 $