From patchwork Fri Jan 6 15:43:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Moore X-Patchwork-Id: 13091480 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 21F24C3DA7A for ; Fri, 6 Jan 2023 15:44:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229824AbjAFPoE (ORCPT ); Fri, 6 Jan 2023 10:44:04 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229703AbjAFPoE (ORCPT ); Fri, 6 Jan 2023 10:44:04 -0500 Received: from mail-yw1-x1131.google.com (mail-yw1-x1131.google.com [IPv6:2607:f8b0:4864:20::1131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B995C41648 for ; Fri, 6 Jan 2023 07:44:02 -0800 (PST) Received: by mail-yw1-x1131.google.com with SMTP id 00721157ae682-4a2f8ad29d5so27464077b3.8 for ; Fri, 06 Jan 2023 07:44:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore.com; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=8YtDOALCm4j61oIDe5piCQL9JbkyPVTVUGZdht/Bzik=; b=GOrjdpE2ofeY7dSVxIvqIegvdqC/XSgn2KbTF+B24stkxJXTR3glZcrW239gFcX5QL cTUCdJsy7qKM3tCjVKdwAhSQZLrr54WeQ+aaFI3mWC5e1wGe9dA0sOjp/hGyuUVaKw0d 1Z4ORXMIg1b7mQBM2xHzIfz9g9X2rHwqZbHa1AcizLoHCfUu+KBakhDtj3mWdLKy6GuL mOURkxKMef6w7YQDzUGb+BHwkti9MhSR+fOcSpPApLo+GjxctvRmNm0lZpdFYRacX8eo igWF0g2ngJFrBzNMDqEN+a1jIM2jQLmBihc1arV/7k4gzXtGO1jOUXuxpD9c1Z8CdSjA PLSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=8YtDOALCm4j61oIDe5piCQL9JbkyPVTVUGZdht/Bzik=; b=IsevpGibzSU4Mp9W1Bj4PXKciqUFUuuVr3lvrjw3z6PE4oL5IGwte3b6ZkmcRVEz+i Vuqyz82y7gmXUJC3RiizaoEEU2P3zyyMcTs9nJG+WS+SFejFjpVdOj0CYwfvAYEZawiT kOwsSaZS9x5LWOAPOXIMq7Z0ufm8UJ4Ppzb2Kc6mA/x3bW8P4JAsQsNWwQahG6aic69o vZ8+ebwNU+7187AyKj23me7g3ARP4feEygFmjEJDbEYZllFRsGXyqj7dGw4MnJoDDf0b c1Gcr7KL54MJwAy0+T3xCWFikz+tdf8SSP0+iGRq5Iv9QBZgPzzhY2Kg0GQEnsN/YPTf /0kA== X-Gm-Message-State: AFqh2ko8GIXGkNRtgyfAKiNTCEUilP2cEqqVqw2g6z1k3yRx5wjEykat 6sUcMAKKVtIo4JBGeuitkGCu X-Google-Smtp-Source: AMrXdXudGOJNnZV0xOrQj49/7/cL6W3q8iHq8Fpvr7yr6sQYlnvLQxoqwakTM93PjIK7EQjTCLQoXQ== X-Received: by 2002:a05:7500:5708:b0:f0:2e10:4737 with SMTP id by8-20020a057500570800b000f02e104737mr521506gab.37.1673019841713; Fri, 06 Jan 2023 07:44:01 -0800 (PST) Received: from localhost (pool-108-26-161-203.bstnma.fios.verizon.net. [108.26.161.203]) by smtp.gmail.com with ESMTPSA id z9-20020ac87ca9000000b003a7e2aea23esm633021qtv.86.2023.01.06.07.44.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Jan 2023 07:44:00 -0800 (PST) From: Paul Moore To: linux-audit@redhat.com, bpf@vger.kernel.org Cc: Burn Alting , Stanislav Fomichev , Alexei Starovoitov Subject: [PATCH v3 1/2] bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD Date: Fri, 6 Jan 2023 10:43:59 -0500 Message-Id: <20230106154400.74211-1-paul@paul-moore.com> X-Mailer: git-send-email 2.39.0 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net When changing the ebpf program put() routines to support being called from within IRQ context the program ID was reset to zero prior to calling the perf event and audit UNLOAD record generators, which resulted in problems as the ebpf program ID was bogus (always zero). This patch addresses this problem by removing an unnecessary call to bpf_prog_free_id() in __bpf_prog_offload_destroy() and adjusting __bpf_prog_put() to only call bpf_prog_free_id() after audit and perf have finished their bpf program unload tasks in bpf_prog_put_deferred(). For the record, no one can determine, or remember, why it was necessary to free the program ID, and remove it from the IDR, prior to executing bpf_prog_put_deferred(); regardless, both Stanislav and Alexei agree that the approach in this patch should be safe. It is worth noting that when moving the bpf_prog_free_id() call, the do_idr_lock parameter was forced to true as the ebpf devs determined this was the correct as the do_idr_lock should always be true. The do_idr_lock parameter will be removed in a follow-up patch, but it was kept here to keep the patch small in an effort to ease any stable backports. I also modified the bpf_audit_prog() logic used to associate the AUDIT_BPF record with other associated records, e.g. @ctx != NULL. Instead of keying off the operation, it now keys off the execution context, e.g. '!in_irg && !irqs_disabled()', which is much more appropriate and should help better connect the UNLOAD operations with the associated audit state (other audit records). Cc: stable@vger.kernel.org Fixes: d809e134be7a ("bpf: Prepare bpf_prog_put() to be called from irq context.") Reported-by: Burn Alting Reported-by: Jiri Olsa Suggested-by: Stanislav Fomichev Suggested-by: Alexei Starovoitov Signed-off-by: Paul Moore Acked-by: Stanislav Fomichev --- * v3 - abandon most of the changes in v2 - move bpf_prog_free_id() after the audit/perf unload hooks - remove bpf_prog_free_id() from __bpf_prog_offload_destroy() - added stable tag * v2 - change subj - add mention of the perf regression - drop the dedicated program audit ID - add the bpf_prog::valid_id flag, bpf_prog_get_id() getter - convert prog ID users to new ID getter * v1 - subj was: "bpf: restore the ebpf audit UNLOAD id field" - initial draft --- kernel/bpf/offload.c | 3 --- kernel/bpf/syscall.c | 6 ++---- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c index 13e4efc971e6..190d9f9dc987 100644 --- a/kernel/bpf/offload.c +++ b/kernel/bpf/offload.c @@ -216,9 +216,6 @@ static void __bpf_prog_offload_destroy(struct bpf_prog *prog) if (offload->dev_state) offload->offdev->ops->destroy(prog); - /* Make sure BPF_PROG_GET_NEXT_ID can't find this dead program */ - bpf_prog_free_id(prog, true); - list_del_init(&offload->offloads); kfree(offload); prog->aux->offload = NULL; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 64131f88c553..61bb19e81b9c 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1972,7 +1972,7 @@ static void bpf_audit_prog(const struct bpf_prog *prog, unsigned int op) return; if (audit_enabled == AUDIT_OFF) return; - if (op == BPF_AUDIT_LOAD) + if (!in_irq() && !irqs_disabled()) ctx = audit_context(); ab = audit_log_start(ctx, GFP_ATOMIC, AUDIT_BPF); if (unlikely(!ab)) @@ -2067,6 +2067,7 @@ static void bpf_prog_put_deferred(struct work_struct *work) prog = aux->prog; perf_event_bpf_event(prog, PERF_BPF_EVENT_PROG_UNLOAD, 0); bpf_audit_prog(prog, BPF_AUDIT_UNLOAD); + bpf_prog_free_id(prog, true); __bpf_prog_put_noref(prog, true); } @@ -2075,9 +2076,6 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock) struct bpf_prog_aux *aux = prog->aux; if (atomic64_dec_and_test(&aux->refcnt)) { - /* bpf_prog_free_id() must be called first */ - bpf_prog_free_id(prog, do_idr_lock); - if (in_irq() || irqs_disabled()) { INIT_WORK(&aux->work, bpf_prog_put_deferred); schedule_work(&aux->work); From patchwork Fri Jan 6 15:44:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Moore X-Patchwork-Id: 13091481 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC646C4708D for ; Fri, 6 Jan 2023 15:44:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229703AbjAFPoF (ORCPT ); Fri, 6 Jan 2023 10:44:05 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54206 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230047AbjAFPoE (ORCPT ); Fri, 6 Jan 2023 10:44:04 -0500 Received: from mail-vs1-xe34.google.com (mail-vs1-xe34.google.com [IPv6:2607:f8b0:4864:20::e34]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2665C3D5FA for ; Fri, 6 Jan 2023 07:44:04 -0800 (PST) Received: by mail-vs1-xe34.google.com with SMTP id m129so1797984vsc.11 for ; Fri, 06 Jan 2023 07:44:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore.com; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=fd5JRFn3Cr5IWNkuRQ9nibDUI9vKExSd4SCBuH3OmNw=; b=W3BOxt4a7zLzcHt5RbOYmVdoHtne+0CNMYaCjuAlZ9GrHaRCSgiq5pAPd3KQqye6oq axxavehyxKqrmf4Gvj0BZGFH+nd7kVyWug5Gt4p/iJzcW9crHQ2O1IIgCcPqIvxV2/QI bODjxUlpV5Hb6Wk/dR9Q1OnABzB3T9BmoAhkjnxEBwiUPTNMZdhXNlyHsVv/nF1Exxsr f0/6aGV3qQgLTxICpzYYOMnNlIgVSARZu6ZW3Rlx5quL/egJ/oFQXi0ITJ005k73Wn67 KjXWBcma04sLmLsLZR1i22/8RpI69T/x/2wBIthk8qQHVT+kg0erEoYsOkhhXv7/iv0r CEcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=fd5JRFn3Cr5IWNkuRQ9nibDUI9vKExSd4SCBuH3OmNw=; b=KetlojoTMO2ScxRIwkNXJRWqLmMvqfsoGv6q2o1TFTXVadGf5Z4nbrhNOU/fq3ttb4 4aH5pser9cWNbi48CFToI7DeyPa4+UE58YDSLImvl0b2jyjTohemFDEX1p3DHKwYbzPx mZMEvKzCe6xErEdFDZiu5OcjtswHgBHi0w9oLoUl3Qg2Rp/0skADQP1xifsfG5YCsHt4 g2zZMlCVTTFoHhYbs6gLGawwcGIxrbiuQZkWCX388bVOVgr12PkJh0YrvnzwRwqnkuDv Dofz0aNLppI6jY4zU/4eJJLkw/KSO8XeJWnhoNq4mTQ59XY1rAnVwXXK79hyEl6fAb3z M8zA== X-Gm-Message-State: AFqh2krRltLUAzKO6xCnumG+XZw86plueDpj3YtKmjDXNkO5f+SJiaaw 5aHsvH0LJjRHIsqBMJhuMSp5KxanQe6Gwo8= X-Google-Smtp-Source: AMrXdXs8rijHgVUdSfFZAXBKIH6T7FgQK3p9S5Uiy+/kLe4hL1kOCqS28Jglq6Smsm8NfhFBWxp6fA== X-Received: by 2002:a67:fe8c:0:b0:3cc:fb78:e403 with SMTP id b12-20020a67fe8c000000b003ccfb78e403mr13173588vsr.10.1673019843168; Fri, 06 Jan 2023 07:44:03 -0800 (PST) Received: from localhost (pool-108-26-161-203.bstnma.fios.verizon.net. [108.26.161.203]) by smtp.gmail.com with ESMTPSA id w19-20020a05620a445300b006fc3fa1f589sm678167qkp.114.2023.01.06.07.44.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Jan 2023 07:44:02 -0800 (PST) From: Paul Moore To: linux-audit@redhat.com, bpf@vger.kernel.org Cc: Burn Alting , Stanislav Fomichev , Alexei Starovoitov Subject: [PATCH v3 2/2] bpf: remove the do_idr_lock parameter from bpf_prog_free_id() Date: Fri, 6 Jan 2023 10:44:00 -0500 Message-Id: <20230106154400.74211-2-paul@paul-moore.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230106154400.74211-1-paul@paul-moore.com> References: <20230106154400.74211-1-paul@paul-moore.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net It was determined that the do_idr_lock parameter to bpf_prog_free_id() was not necessary as it should always be true. Suggested-by: Stanislav Fomichev Signed-off-by: Paul Moore Acked-by: Stanislav Fomichev --- * v3 - initial draft --- include/linux/bpf.h | 2 +- kernel/bpf/syscall.c | 20 ++++++-------------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 3de24cfb7a3d..634d37a599fa 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1832,7 +1832,7 @@ void bpf_prog_inc(struct bpf_prog *prog); struct bpf_prog * __must_check bpf_prog_inc_not_zero(struct bpf_prog *prog); void bpf_prog_put(struct bpf_prog *prog); -void bpf_prog_free_id(struct bpf_prog *prog, bool do_idr_lock); +void bpf_prog_free_id(struct bpf_prog *prog); void bpf_map_free_id(struct bpf_map *map, bool do_idr_lock); struct btf_field *btf_record_find(const struct btf_record *rec, diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 61bb19e81b9c..ecca9366c7a6 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -2001,7 +2001,7 @@ static int bpf_prog_alloc_id(struct bpf_prog *prog) return id > 0 ? 0 : id; } -void bpf_prog_free_id(struct bpf_prog *prog, bool do_idr_lock) +void bpf_prog_free_id(struct bpf_prog *prog) { unsigned long flags; @@ -2013,18 +2013,10 @@ void bpf_prog_free_id(struct bpf_prog *prog, bool do_idr_lock) if (!prog->aux->id) return; - if (do_idr_lock) - spin_lock_irqsave(&prog_idr_lock, flags); - else - __acquire(&prog_idr_lock); - + spin_lock_irqsave(&prog_idr_lock, flags); idr_remove(&prog_idr, prog->aux->id); prog->aux->id = 0; - - if (do_idr_lock) - spin_unlock_irqrestore(&prog_idr_lock, flags); - else - __release(&prog_idr_lock); + spin_unlock_irqrestore(&prog_idr_lock, flags); } static void __bpf_prog_put_rcu(struct rcu_head *rcu) @@ -2067,11 +2059,11 @@ static void bpf_prog_put_deferred(struct work_struct *work) prog = aux->prog; perf_event_bpf_event(prog, PERF_BPF_EVENT_PROG_UNLOAD, 0); bpf_audit_prog(prog, BPF_AUDIT_UNLOAD); - bpf_prog_free_id(prog, true); + bpf_prog_free_id(prog); __bpf_prog_put_noref(prog, true); } -static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock) +static void __bpf_prog_put(struct bpf_prog *prog) { struct bpf_prog_aux *aux = prog->aux; @@ -2087,7 +2079,7 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock) void bpf_prog_put(struct bpf_prog *prog) { - __bpf_prog_put(prog, true); + __bpf_prog_put(prog); } EXPORT_SYMBOL_GPL(bpf_prog_put);