From patchwork Thu Aug 27 06:26:59 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexei Starovoitov X-Patchwork-Id: 511177 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48B651401F6 for ; Thu, 27 Aug 2015 16:27:31 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753253AbbH0G1J (ORCPT ); Thu, 27 Aug 2015 02:27:09 -0400 Received: from mail-pa0-f48.google.com ([209.85.220.48]:34959 "EHLO mail-pa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753037AbbH0G1H (ORCPT ); Thu, 27 Aug 2015 02:27:07 -0400 Received: by pacdd16 with SMTP id dd16so14407482pac.2 for ; Wed, 26 Aug 2015 23:27:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=a/9HHolYJLfS7yiwxkHN03Sky0BY8vVuD7LObqoMVnQ=; b=Fu1hdMbSTgkcL8JZuVxLfhCeMHLuX6/U1x1YOd8ImUgobao55MNd3eSBoI318lx3b3 CEQDTiiCgi0Pm9AgibLVlWB+VuWLtMeMi2a2/euA1buP4aC6UWt4x6i22VcqNXKC6mSH aqfxaXXH5kYfMmrzEV9XL6PNU07K/3vWWAVvkZxmB4XHKRWPULhkz2jchywkStXUnw5V p1omKOA0T+UvsveX6D4FENANAjpkj35Jwbe4a2Oo8W6OnnLHNdvMVVqjFryhkYTqhLtP zoLq9mAbwLFxEv/lVmBALeggsMtaVk34yLNPFxGQ7SJ2+rfbzS70RWjYaJaO7/LCSwcn nt9w== X-Gm-Message-State: ALoCoQl5Ecr918hqHvqbneH9vdtyyfiepsI3nkiar9Yoa+ak9RCz0fccaXKOaAavznyblRGP5seS X-Received: by 10.66.246.228 with SMTP id xz4mr4114144pac.46.1440656826932; Wed, 26 Aug 2015 23:27:06 -0700 (PDT) Received: from localhost.localdomain ([12.97.19.195]) by smtp.gmail.com with ESMTPSA id le8sm1098379pbc.24.2015.08.26.23.27.06 (version=TLS1_1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 26 Aug 2015 23:27:06 -0700 (PDT) From: Alexei Starovoitov To: "David S. Miller" Cc: Ingo Molnar , Steven Rostedt , Wang Nan , He Kuang , Daniel Borkmann , Brendan Gregg , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH net-next] bpf: add support for %s specifier to bpf_trace_printk() Date: Wed, 26 Aug 2015 23:26:59 -0700 Message-Id: <1440656819-25622-1-git-send-email-ast@plumgrid.com> X-Mailer: git-send-email 1.7.9.5 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org %s specifier makes bpf program and kernel debugging easier. To make sure that trace_printk won't crash the unsafe string is copied into stack and unsafe pointer is substituted. String is also sanitized for printable characters. The cost of swapping FS in probe_kernel_read is amortized over 4 chars to improve performance. Suggested-by: Brendan Gregg Signed-off-by: Alexei Starovoitov --- The following C program: #include int foo(struct pt_regs *ctx, struct filename *filename) { void *name = 0; bpf_probe_read(&name, sizeof(name), &filename->name); bpf_trace_printk("executed %s\\n", name); return 0; } when attached to kprobe do_execve() will produce output in /sys/kernel/debug/tracing/trace_pipe : make-13492 [002] d..1 3250.997277: : executed /bin/sh sh-13493 [004] d..1 3250.998716: : executed /usr/bin/gcc gcc-13494 [002] d..1 3250.999822: : executed /usr/lib/gcc/x86_64-linux-gnu/4.7/cc1 gcc-13495 [002] d..1 3251.006731: : executed /usr/bin/as gcc-13496 [002] d..1 3251.011831: : executed /usr/lib/gcc/x86_64-linux-gnu/4.7/collect2 collect2-13497 [000] d..1 3251.012941: : executed /usr/bin/ld kernel/trace/bpf_trace.c | 60 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index ef9936df1b04..60d8f95258ed 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -79,13 +79,51 @@ static const struct bpf_func_proto bpf_probe_read_proto = { .arg3_type = ARG_ANYTHING, }; +static bool is_valid_char(char c) +{ + return (isprint(c) || isspace(c)) && isascii(c); +} + +/* similar to strncpy_from_user() but with extra checks */ +static void probe_read_string(char *buf, int size, long unsafe_ptr) +{ + char dst[4]; + int i = 0; + + size--; + for (;;) { + if (probe_kernel_read(dst, (void *) unsafe_ptr, 4)) + break; + + unsafe_ptr += 4; + + if (dst[0] == 0 || !is_valid_char(dst[0]) || i >= size) + break; + buf[i++] = dst[0]; + + if (dst[1] == 0 || !is_valid_char(dst[1]) || i >= size) + break; + buf[i++] = dst[1]; + + if (dst[2] == 0 || !is_valid_char(dst[2]) || i >= size) + break; + buf[i++] = dst[2]; + + if (dst[3] == 0 || !is_valid_char(dst[3]) || i >= size) + break; + buf[i++] = dst[3]; + } + buf[i] = 0; +} + /* * limited trace_printk() - * only %d %u %x %ld %lu %lx %lld %llu %llx %p conversion specifiers allowed + * only %d %u %x %ld %lu %lx %lld %llu %llx %p %s conversion specifiers allowed */ static u64 bpf_trace_printk(u64 r1, u64 fmt_size, u64 r3, u64 r4, u64 r5) { char *fmt = (char *) (long) r1; + char buf[64]; int mod[3] = {}; int fmt_cnt = 0; int i; @@ -100,7 +138,7 @@ static u64 bpf_trace_printk(u64 r1, u64 fmt_size, u64 r3, u64 r4, u64 r5) /* check format string for allowed specifiers */ for (i = 0; i < fmt_size; i++) { - if ((!isprint(fmt[i]) && !isspace(fmt[i])) || !isascii(fmt[i])) + if (!is_valid_char(fmt[i])) return -EINVAL; if (fmt[i] != '%') @@ -114,12 +152,28 @@ static u64 bpf_trace_printk(u64 r1, u64 fmt_size, u64 r3, u64 r4, u64 r5) if (fmt[i] == 'l') { mod[fmt_cnt]++; i++; - } else if (fmt[i] == 'p') { + } else if (fmt[i] == 'p' || fmt[i] == 's') { mod[fmt_cnt]++; i++; if (!isspace(fmt[i]) && !ispunct(fmt[i]) && fmt[i] != 0) return -EINVAL; fmt_cnt++; + if (fmt[i - 1] == 's') { + switch (fmt_cnt) { + case 1: + probe_read_string(buf, sizeof(buf), r3); + r3 = (long) buf; + break; + case 2: + probe_read_string(buf, sizeof(buf), r4); + r4 = (long) buf; + break; + case 3: + probe_read_string(buf, sizeof(buf), r5); + r5 = (long) buf; + break; + } + } continue; }