From patchwork Fri Oct 23 00:10:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexei Starovoitov X-Patchwork-Id: 534664 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 3EA7A1402B7 for ; Fri, 23 Oct 2015 11:10:47 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965690AbbJWAKZ (ORCPT ); Thu, 22 Oct 2015 20:10:25 -0400 Received: from mail-pa0-f50.google.com ([209.85.220.50]:35736 "EHLO mail-pa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751387AbbJWAKX (ORCPT ); Thu, 22 Oct 2015 20:10:23 -0400 Received: by pasz6 with SMTP id z6so100210387pas.2 for ; Thu, 22 Oct 2015 17:10:23 -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=w67aI1AHnBIbp8IA4dReZlRm2To31UGbUWD6YWCddGU=; b=h6Hg1qLPeyc0/ylER6wvk1tMirsA6wARNF1RXCShm8T441fnySwB71FP+/AjzpFv9Y HY2wi0ltRJGpbNvxHr4Xx4v8oWucud7uj94KC3ddbTcCRPYEwbWNkbKSdpL60BR3iQFa xNfpZCpDPfd4SkFn/HV+8Whx/5tPEy9hu7g8kmnAYkYi/BBUJVeq/m1Yc3Ar3bTowf3F Lq0gqMPZ9K+QE7HXtZpBwQNMjfXY7wiHCX6KKZSYiOuexUxhNQURws0NMe5dogbOFf1y LTs3seULQ6JP7hLz44zdDNJw0trrJgFWCqN1ZO6T3JwZL6NTPNgdbj6G3px+CiscZIpQ OEKA== X-Gm-Message-State: ALoCoQlZ0YFI8y4N49BjkLdHzCB7iwYIfu6zs3T07pR19msLHP1MNDcZW9dPdQG1vYmY6kLxAHgg X-Received: by 10.68.131.36 with SMTP id oj4mr1385100pbb.141.1445559023096; Thu, 22 Oct 2015 17:10:23 -0700 (PDT) Received: from localhost.localdomain ([12.97.19.195]) by smtp.gmail.com with ESMTPSA id g12sm15804243pat.36.2015.10.22.17.10.21 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 22 Oct 2015 17:10:22 -0700 (PDT) From: Alexei Starovoitov X-Google-Original-From: Alexei Starovoitov To: "David S. Miller" Cc: Ingo Molnar , Peter Zijlstra , Wang Nan , He Kuang , Kaixu Xia , Daniel Borkmann , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v3 net-next] bpf: fix bpf_perf_event_read() helper Date: Thu, 22 Oct 2015 17:10:14 -0700 Message-Id: <1445559014-4667-1-git-send-email-ast@kernel.org> 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 Fix safety checks for bpf_perf_event_read(): - only non-inherited events can be added to perf_event_array map (do this check statically at map insertion time) - dynamically check that event is local and !pmu->count Otherwise buggy bpf program can cause kernel splat. Also fix error path after perf_event_attrs() and remove redundant 'extern'. Fixes: 35578d798400 ("bpf: Implement function bpf_perf_event_read() that get the selected hardware PMU conuter") Signed-off-by: Alexei Starovoitov Tested-by: Wang Nan --- v2->v3: . refactor checks based on Wangnan's and Peter's feedback while refactoring realized that these two issues need fixes as well: . fix perf_event_attrs() error path . remove redundant extern v1->v2: fix compile in case of !CONFIG_PERF_EVENTS Even in the worst case the crash is not possible. Only warn_on_once, so imo net-next is ok. include/linux/bpf.h | 1 - kernel/bpf/arraymap.c | 25 ++++++++++++++++--------- kernel/trace/bpf_trace.c | 7 ++++++- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index e3a51b74e275..75718fa28260 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -194,7 +194,6 @@ extern const struct bpf_func_proto bpf_map_lookup_elem_proto; extern const struct bpf_func_proto bpf_map_update_elem_proto; extern const struct bpf_func_proto bpf_map_delete_elem_proto; -extern const struct bpf_func_proto bpf_perf_event_read_proto; extern const struct bpf_func_proto bpf_get_prandom_u32_proto; extern const struct bpf_func_proto bpf_get_smp_processor_id_proto; extern const struct bpf_func_proto bpf_tail_call_proto; diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c index e3cfe46b074f..3f4c99e06c6b 100644 --- a/kernel/bpf/arraymap.c +++ b/kernel/bpf/arraymap.c @@ -292,16 +292,23 @@ static void *perf_event_fd_array_get_ptr(struct bpf_map *map, int fd) attr = perf_event_attrs(event); if (IS_ERR(attr)) - return (void *)attr; + goto err; - if (attr->type != PERF_TYPE_RAW && - !(attr->type == PERF_TYPE_SOFTWARE && - attr->config == PERF_COUNT_SW_BPF_OUTPUT) && - attr->type != PERF_TYPE_HARDWARE) { - perf_event_release_kernel(event); - return ERR_PTR(-EINVAL); - } - return event; + if (attr->inherit) + goto err; + + if (attr->type == PERF_TYPE_RAW) + return event; + + if (attr->type == PERF_TYPE_HARDWARE) + return event; + + if (attr->type == PERF_TYPE_SOFTWARE && + attr->config == PERF_COUNT_SW_BPF_OUTPUT) + return event; +err: + perf_event_release_kernel(event); + return ERR_PTR(-EINVAL); } static void perf_event_fd_array_put_ptr(void *ptr) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 47febbe7998e..003df3887287 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -199,6 +199,11 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5) if (!event) return -ENOENT; + /* make sure event is local and doesn't have pmu::count */ + if (event->oncpu != smp_processor_id() || + event->pmu->count) + return -EINVAL; + /* * we don't know if the function is run successfully by the * return value. It can be judged in other places, such as @@ -207,7 +212,7 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5) return perf_event_read_local(event); } -const struct bpf_func_proto bpf_perf_event_read_proto = { +static const struct bpf_func_proto bpf_perf_event_read_proto = { .func = bpf_perf_event_read, .gpl_only = false, .ret_type = RET_INTEGER,