From patchwork Wed Oct 21 22:58:03 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexei Starovoitov X-Patchwork-Id: 534120 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 16BB21402D4 for ; Thu, 22 Oct 2015 09:58:28 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756023AbbJUW6N (ORCPT ); Wed, 21 Oct 2015 18:58:13 -0400 Received: from mail-pa0-f42.google.com ([209.85.220.42]:33742 "EHLO mail-pa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754663AbbJUW6L (ORCPT ); Wed, 21 Oct 2015 18:58:11 -0400 Received: by pabrc13 with SMTP id rc13so67270244pab.0 for ; Wed, 21 Oct 2015 15:58:10 -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=PlltGGT5kVZoWytWOPlQQ7sHxked3ahpI3YTX969ch8=; b=kWDXG/ZcjO91zY2kZVzz7l28HcofZn3YIAIdXHyMT7lL7VTETWI3jcc0wD3KL5wmrS EpVi6w++HTksbnlMlSQ/7ESDGfKDlx6iDKDYpkZQjZ4HKItMs+KVC8kkgH3OkSfDeOLq TRpChcF5TYkzzzHw7Zls7BwxaSy/8PLNwM5+QwkXvhRIEw4Kd7ZQGjuwpeaqYZT9gvpC FPZpfHP4J54BAfQNsd6fRYNPp39+L7Znmx4vteBofiOc3VL2Roi1bVtuDoLlpS3XO1Sb o2Gu7xl8dMA8kNsiZirj3xAhjknms++rHWdUnfzRJgmuP9rPyYfKuJynEBhnL6TeB+Od DigQ== X-Gm-Message-State: ALoCoQlZzU5Nx2d3mBTgAED82vHlMGVQZ0YBrJIUGgSspsLNQSJUNswjh2ypAhFf2aqFpNjRsD3m X-Received: by 10.68.197.168 with SMTP id iv8mr13150643pbc.81.1445468290802; Wed, 21 Oct 2015 15:58:10 -0700 (PDT) Received: from localhost.localdomain ([12.97.19.195]) by smtp.gmail.com with ESMTPSA id er1sm590909pbb.6.2015.10.21.15.58.09 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 21 Oct 2015 15:58:09 -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 v2 net-next] bpf: fix bpf_perf_event_read() helper Date: Wed, 21 Oct 2015 15:58:03 -0700 Message-Id: <1445468283-4592-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. Fixes: 35578d798400 ("bpf: Implement function bpf_perf_event_read() that get the selected hardware PMU conuter") Signed-off-by: Alexei Starovoitov --- v1->v2: fix compile in case of !CONFIG_PERF_EVENTS This patch is on top of http://patchwork.ozlabs.org/patch/533585/ to avoid conflicts. Even in the worst case the crash is not possible. Only warn_on_once, so imo net-next is ok. kernel/bpf/arraymap.c | 9 +++++---- kernel/events/core.c | 16 ++++++++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c index e3cfe46b074f..75529cc94304 100644 --- a/kernel/bpf/arraymap.c +++ b/kernel/bpf/arraymap.c @@ -294,10 +294,11 @@ static void *perf_event_fd_array_get_ptr(struct bpf_map *map, int fd) if (IS_ERR(attr)) return (void *)attr; - if (attr->type != PERF_TYPE_RAW && - !(attr->type == PERF_TYPE_SOFTWARE && - attr->config == PERF_COUNT_SW_BPF_OUTPUT) && - attr->type != PERF_TYPE_HARDWARE) { + if ((attr->type != PERF_TYPE_RAW && + !(attr->type == PERF_TYPE_SOFTWARE && + attr->config == PERF_COUNT_SW_BPF_OUTPUT) && + attr->type != PERF_TYPE_HARDWARE) || + attr->inherit) { perf_event_release_kernel(event); return ERR_PTR(-EINVAL); } diff --git a/kernel/events/core.c b/kernel/events/core.c index 64754bfecd70..0b6333265872 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -3258,7 +3258,7 @@ static inline u64 perf_event_count(struct perf_event *event) u64 perf_event_read_local(struct perf_event *event) { unsigned long flags; - u64 val; + u64 val = -EINVAL; /* * Disabling interrupts avoids all counter scheduling (context @@ -3267,12 +3267,14 @@ u64 perf_event_read_local(struct perf_event *event) local_irq_save(flags); /* If this is a per-task event, it must be for current */ - WARN_ON_ONCE((event->attach_state & PERF_ATTACH_TASK) && - event->hw.target != current); + if ((event->attach_state & PERF_ATTACH_TASK) && + event->hw.target != current) + goto out; /* If this is a per-CPU event, it must be for this CPU */ - WARN_ON_ONCE(!(event->attach_state & PERF_ATTACH_TASK) && - event->cpu != smp_processor_id()); + if (!(event->attach_state & PERF_ATTACH_TASK) && + event->cpu != smp_processor_id()) + goto out; /* * It must not be an event with inherit set, we cannot read @@ -3284,7 +3286,8 @@ u64 perf_event_read_local(struct perf_event *event) * It must not have a pmu::count method, those are not * NMI safe. */ - WARN_ON_ONCE(event->pmu->count); + if (event->pmu->count) + goto out; /* * If the event is currently on this CPU, its either a per-task event, @@ -3295,6 +3298,7 @@ u64 perf_event_read_local(struct perf_event *event) event->pmu->read(event); val = local64_read(&event->count); +out: local_irq_restore(flags); return val;