From patchwork Wed Jan 4 04:52:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Levin, Alexander \(Sasha Levin\)" X-Patchwork-Id: 89771 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp8331590qgi; Tue, 3 Jan 2017 21:05:46 -0800 (PST) X-Received: by 10.99.110.10 with SMTP id j10mr119232794pgc.134.1483506346909; Tue, 03 Jan 2017 21:05:46 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id u74si71126551pgc.310.2017.01.03.21.05.46; Tue, 03 Jan 2017 21:05:46 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@verizon.com; dkim=pass header.i=@verizon.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE dis=NONE) header.from=verizon.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751469AbdADFFm (ORCPT + 25 others); Wed, 4 Jan 2017 00:05:42 -0500 Received: from omzsmtpe02.verizonbusiness.com ([199.249.25.209]:5121 "EHLO omzsmtpe02.verizonbusiness.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750853AbdADFFg (ORCPT ); Wed, 4 Jan 2017 00:05:36 -0500 X-Greylist: delayed 724 seconds by postgrey-1.27 at vger.kernel.org; Wed, 04 Jan 2017 00:05:35 EST DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=verizon.com; i=@verizon.com; q=dns/txt; s=corp; t=1483506335; x=1515042335; h=from:to:cc:subject:date:message-id:references: in-reply-to:mime-version; bh=+HoNzBDyYQz4nDGs9sJEfQCz0lJsMX+kawuBrqw2iZI=; b=ZkTE9iJkTJoPnAEzfYqJSwfVzwnoG+m97kxM1x8nNlQDoEyCW7tU7iyw kkuta+KFYvcuiL/f3xPX3kJEKeeUAkoTyivOinmLg8ld5ERM7cB8Onwof k2kXjxHNZdUVVkjlMP6NqYo/fVyV8R8nRHqbTuVd42MpKO0nAyqUfMYwU E=; X-IronPort-Anti-Spam-Filtered: false Received: from unknown (HELO fldsmtpi03.verizon.com) ([166.68.71.145]) by omzsmtpe02.verizonbusiness.com with ESMTP; 04 Jan 2017 04:53:29 +0000 X-IronPort-AV: E=Sophos;i="5.33,458,1477958400"; d="scan'208";a="260619145" Received: from rogue-10-255-0-101.rogue.vzwcorp.com (HELO Gemini.verizonwireless.com) ([10.255.0.101]) by fldsmtpi03.verizon.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 04 Jan 2017 04:52:58 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=verizon.com; i=@verizon.com; q=dns/txt; s=corp; t=1483505579; x=1515041579; h=from:to:cc:subject:date:message-id:references: in-reply-to:mime-version; bh=+HoNzBDyYQz4nDGs9sJEfQCz0lJsMX+kawuBrqw2iZI=; b=cPR3Ju70Ut893gvpYbYw15tSu9hkF3AJ2E6ULZmJK1JecblTyvJmB48G SSxFfUhNlJ/2/p8n+6hUzXQJ8vyB8UBAbyG2OU2dc42FRyypDairG6roG JubbA7b2oHlvF717aEwUFfR2cEZ9KTxydXaoUNSP5FLy0i7oJWD4D6te9 I=; Received: from ranger.odc.vzwcorp.com (HELO mercury.verizonwireless.com) ([10.255.240.27]) by Gemini.verizonwireless.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 03 Jan 2017 20:52:57 -0800 From: alexander.levin@verizon.com X-Host: ranger.odc.vzwcorp.com Received: from nyora1hub004.uswin.ad.vzwcorp.com ([10.170.34.219]) by mercury.verizonwireless.com with ESMTP/TLS/AES256-SHA; 04 Jan 2017 04:52:56 +0000 Received: from OMZP1LUMXCA15.uswin.ad.vzwcorp.com (144.8.22.190) by NYORA1HUB004.uswin.ad.vzwcorp.com (10.170.34.219) with Microsoft SMTP Server (TLS) id 8.3.406.0; Tue, 3 Jan 2017 23:52:56 -0500 Received: from OMZP1LUMXCA17.uswin.ad.vzwcorp.com (144.8.22.195) by OMZP1LUMXCA15.uswin.ad.vzwcorp.com (144.8.22.190) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Tue, 3 Jan 2017 22:52:55 -0600 Received: from OMZP1LUMXCA17.uswin.ad.vzwcorp.com ([144.8.22.195]) by OMZP1LUMXCA17.uswin.ad.vzwcorp.com ([144.8.22.195]) with mapi id 15.00.1210.000; Tue, 3 Jan 2017 22:52:54 -0600 To: Dmitry Vyukov CC: "tglx@linutronix.de" , "scientist@fb.com" , "glider@google.com" , "andreyknvl@google.com" , "rostedt@goodmis.org" , "arnd@arndb.de" , "mathieu.desnoyers@efficios.com" , "daniel.vetter@ffwll.ch" , "linux-kernel@vger.kernel.org" Subject: Re: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls Thread-Topic: [RFC 1/3] abi_spec: basic definitions of constraints, args and syscalls Thread-Index: AQHSZkZp9G8M9GFv4U6Yjir8BXr3iQ== Date: Wed, 4 Jan 2017 04:52:54 +0000 Message-ID: <20170104044830.l4tlvp6732nuef2a@sasha-lappy> References: <1479317803-17220-1-git-send-email-alexander.levin@verizon.com> <1479317803-17220-2-git-send-email-alexander.levin@verizon.com> <20161123150001.GD3218@sasha-lappy> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: user-agent: Mutt/1.6.2-neo (2016-08-21) x-ms-exchange-messagesentrepresentingtype: 1 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.144.60.250] MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --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 #include #include +#include -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 $