From patchwork Sat Mar 25 21:31:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Vernet X-Patchwork-Id: 74983 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp616381vqo; Sat, 25 Mar 2023 14:34:44 -0700 (PDT) X-Google-Smtp-Source: AKy350bsvHTS/hWU3g21lB6g1+hDUy0Z87aegD8JI9Rhf1HF+iFqh0hsHhRlkev6bFBlbxUPAx2W X-Received: by 2002:a17:906:b849:b0:93d:c570:5b3a with SMTP id ga9-20020a170906b84900b0093dc5705b3amr7963440ejb.67.1679780083912; Sat, 25 Mar 2023 14:34:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1679780083; cv=none; d=google.com; s=arc-20160816; b=vcV89yYQnpd2h5/hbARfi2V0nl+Q4nAUs9amh1ab/TbzVwiTP2MUiQlcwub5iZEPKN NJpdSvTLf4cZ7Q5LDTYt91LjneXu8Yv3ZYJJcpGcrfOr8oX+xrdjBl3lmPobV+k6XBbY D68cj4APTDJe+yIqdYvcxifpjJRKj6qw7wnkrsk7foyQkl2uQgLDqAlIO1WIXElvFEXR lx5Rx9fUrcgS5iT8/7OxUto41Ba473lh22gBoKhTrv50VtBHi4epNszqj/yIPcn6IxOC ac+xhlwMiaaUCJiz2S3wYP8GxX6lcM0MSx2CwHgZLzQjDg6djR60focl2HXoFhNKea/6 93qg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=WvytPOgIy3Mzp1dIziBHWQjN9qpIziod7hYaS4cen+M=; b=hqNZVBUs5OaXHdicFtiUWFqHyxuYPcxw3yQ044lSVnjVuBeGHUkhRKWMm/XVJOle4H KGYwDS1mQ9MZbQ+J3lmvQmS6UQoFA7FaRagkgksyzbHQkgjt6Aamkxj6mTNnCMvpv3h1 5gVbDEq93A6md26twAguJBcjROYmUo5HULMlN6mNbbxJyjRotQSV35uyhhaza9OVPv38 pfroU8bjRluqnQ7zBpGABe66gctE9bW+m0WPiHZFBJ0bwMcLLuSStjdSBb5PVCByazhI IIa79DcAK/MHqcuYySwhZ32Jm7MqZ2VlhtGCM8/G9hNQD99wouES+R8uODf4xKEDOuer yFXg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id gt36-20020a1709072da400b0093defbd6292si6057017ejc.1049.2023.03.25.14.34.18; Sat, 25 Mar 2023 14:34:43 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230123AbjCYVcG (ORCPT + 99 others); Sat, 25 Mar 2023 17:32:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59788 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229446AbjCYVcF (ORCPT ); Sat, 25 Mar 2023 17:32:05 -0400 Received: from mail-qv1-f52.google.com (mail-qv1-f52.google.com [209.85.219.52]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 80EAFCC19; Sat, 25 Mar 2023 14:32:04 -0700 (PDT) Received: by mail-qv1-f52.google.com with SMTP id g9so4334827qvt.8; Sat, 25 Mar 2023 14:32:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679779923; 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=WvytPOgIy3Mzp1dIziBHWQjN9qpIziod7hYaS4cen+M=; b=Y34KnqYZlK5vXV2421T4gbHnREimFrxzx4Ih2ombUGEVfWA+K7YvaFN0K2TvFxJheU L3Tz4T2Gcd+lBK7GZbR99eyrFHcyU60IQUeAov7c4s0EuO4XIYugttsQa/TLKrdCanKs gnLC1lH9W/haShPG+puuTA7z+xXT+Hw5AKZjaFi3uEXdzL77qy1Ot/BqP5CcXSIVn9VL yU2M/OGa1n1F9o53M5m13twf3Nqlxnv2GUfQSwfI0ULfE8eJYPpepT8rxT/LaD20nq/H ebSLaHl7BRnyVZV0AE5y0z8z0zMgpidKJBImTyus2yLF6O9Auk7qMZzV10M0feTGzjke JqUQ== X-Gm-Message-State: AAQBX9dKCw5tX26CwU9l0wMSEQwqYoQVz94tnXLplxhgk6gGQR2+dxgT vyTGHLdlU9Kdy//0ygFFEdF+uIHtdjSUAQD4kzU= X-Received: by 2002:a05:6214:ac6:b0:5ac:208c:4e64 with SMTP id g6-20020a0562140ac600b005ac208c4e64mr13523457qvi.37.1679779923301; Sat, 25 Mar 2023 14:32:03 -0700 (PDT) Received: from localhost ([24.1.27.177]) by smtp.gmail.com with ESMTPSA id z8-20020a0cd788000000b005dd8b934587sm1713947qvi.31.2023.03.25.14.32.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Mar 2023 14:32:03 -0700 (PDT) From: David Vernet To: bpf@vger.kernel.org Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com, haoluo@google.com, jolsa@kernel.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: [PATCH bpf-next 1/3] bpf: Only invoke kptr dtor following non-NULL xchg Date: Sat, 25 Mar 2023 16:31:42 -0500 Message-Id: <20230325213144.486885-2-void@manifault.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230325213144.486885-1-void@manifault.com> References: <20230325213144.486885-1-void@manifault.com> MIME-Version: 1.0 X-Spam-Status: No, score=0.5 required=5.0 tests=FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1761377081233563604?= X-GMAIL-MSGID: =?utf-8?q?1761377081233563604?= When a map value is being freed, we loop over all of the fields of the corresponding BPF object and issue the appropriate cleanup calls corresponding to the field's type. If the field is a referenced kptr, we atomically xchg the value out of the map, and invoke the kptr's destructor on whatever was there before (or bpf_obj_drop() it if it was a local kptr). Currently, we always invoke the destructor (either bpf_obj_drop() or the kptr's registered destructor) on any KPTR_REF-type field in a map, even if there wasn't a value in the map. This means that any function serving as the kptr's KF_RELEASE destructor must always treat the argument as possibly NULL, as the following can and regularly does happen: void *xchgd_field; /* No value was in the map, so xchgd_field is NULL */ xchgd_field = (void *)xchg(unsigned long *field_ptr, 0); field->kptr.dtor(xchgd_field); These are odd semantics to impose on KF_RELEASE kfuncs -- BPF programs are prohibited by the verifier from passing NULL pointers to KF_RELEASE kfuncs, so it doesn't make sense to require this of BPF programs, but not the main kernel destructor path. It's also unnecessary to invoke any cleanup logic for local kptrs. If there is no object there, there's nothing to drop. So as to allow KF_RELEASE kfuncs to fully assume that an argument is non-NULL, this patch updates a KPTR_REF's destructor to only be invoked when a non-NULL value is xchg'd out of the kptr map field. Signed-off-by: David Vernet --- kernel/bpf/syscall.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index a09597c95029..e18ac7fdc210 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -677,6 +677,9 @@ void bpf_obj_free_fields(const struct btf_record *rec, void *obj) break; case BPF_KPTR_REF: xchgd_field = (void *)xchg((unsigned long *)field_ptr, 0); + if (!xchgd_field) + break; + if (!btf_is_kernel(field->kptr.btf)) { pointee_struct_meta = btf_find_struct_meta(field->kptr.btf, field->kptr.btf_id); From patchwork Sat Mar 25 21:31:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Vernet X-Patchwork-Id: 74984 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp622749vqo; Sat, 25 Mar 2023 14:57:20 -0700 (PDT) X-Google-Smtp-Source: AKy350ak8vYUiyx+Ty1VdgJJLYwNYRNpowrdxTg3Sbm8RHq8QwgHTRQWTsGK211nw4Q/LCJvwFmT X-Received: by 2002:a17:906:5a81:b0:92e:fcc9:aa22 with SMTP id l1-20020a1709065a8100b0092efcc9aa22mr6939380ejq.37.1679781440339; Sat, 25 Mar 2023 14:57:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1679781440; cv=none; d=google.com; s=arc-20160816; b=LRsaI+Hz2aSuZXPgX92XBNienLBKX6vh5c3Fwkf+uafq+nAmIxVN/Fljp1nFDu5u3D rmAOlnN0/olMjPgOVsxFsEvWU4ipUZSjjM/wyn5mlpnKTsCNGeRHyuInSoog8UngW8zK gYKMAOLaUPCW5PIzsJSWd6icjKUOiIbqVOUIQOE9gDb4btpYyC00/9GbhNg6YR78jrpZ ODfXFCVUflsEEaVh1nf7+dpJPbLJ6DEKaroMtke/0Wt2w5A3qF5QuWLMTkmuQT3ZnU7F 0QBUb7e/eJxZpDOM4lOe8BUpoU47D8G5sblKqiDHsLikrxr6O6GGqyiikGZSB4yM2drP JEuQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=beTKJfiS4cdFGvjwvhWeZJkcVPSdZmhKDIoBjmdTTxM=; b=l6sAwxKXiUoMFJ9A5pHO2NLlCPPTo2OMpwnavcS1MJtH9qn/1RSoZ8hDq2AerJDjgc tHi4C3BwHfGRyIVVOp37FZupOqQktCUaBQZoiwk1a1omXvM12PR8xl/SZX6gDiHrV2hR AbUCWsgsCq0l5jogldxdsPO02MRloTU6hOwp5lVVfM62ZQGNE/GLD3Z4F4XAJRlJA4QT SxIH7qMG60luF6UEIQ/9WlcqimGyOLHTqY8oxhCFr6/VafEHLdqudW35lvXuLdTfTMMT yi1BLBLJCiggnEOLxh86FNp0pC0z/OOovynT6oVahAptgRqt2w3Sm6eugCSz54Gm14UX vlhw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id hs12-20020a1709073e8c00b0093beaa0092dsi10497867ejc.96.2023.03.25.14.56.48; Sat, 25 Mar 2023 14:57:20 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230237AbjCYVca (ORCPT + 99 others); Sat, 25 Mar 2023 17:32:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60676 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229446AbjCYVc3 (ORCPT ); Sat, 25 Mar 2023 17:32:29 -0400 Received: from mail-qv1-f53.google.com (mail-qv1-f53.google.com [209.85.219.53]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EE2ACCC00; Sat, 25 Mar 2023 14:32:19 -0700 (PDT) Received: by mail-qv1-f53.google.com with SMTP id 59so4293676qva.11; Sat, 25 Mar 2023 14:32:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679779939; 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=beTKJfiS4cdFGvjwvhWeZJkcVPSdZmhKDIoBjmdTTxM=; b=wROKc2odk+PGDIC0jrAC4F2WpEt4JtXwUtXOKofljq3jGiNDXJeGB4l/0p3bh5xeyZ hkNz6CuuBn2C4warfq/d5OajSwR65ZF/GVVYMf60qQRLafQ3fBgCk+9I7zi0dHRBw1O0 HW73iNciwpVy2qjVn/zmO12PwVZ1xxwbU3uF3RYYEhLYSszE+oFNmyDZfAcylQbBQ/P/ umUnVJqs+9UgsrLOwTGi5NVyuKIt1T73GGBjmbGORR1SZ+VnhZmc11QKC8yuCyLWukhd A7XYgIkyyDyRhNKEIeUWZBE5PWfVtu3hFjNYY3rRE6Y2vrTl+PTMtorA2P7hH+ZNm+xg 2E9A== X-Gm-Message-State: AAQBX9ew0igcySMQM/n4P/QIFp9fGzM7CiYzlyRcnRMDy4hu0KiZWKoF /0TXVVwDLTeTo9bk6FP0fKdJuUFPS14FNCHeqmo= X-Received: by 2002:a05:6214:2a87:b0:5c6:403d:7af4 with SMTP id jr7-20020a0562142a8700b005c6403d7af4mr10878487qvb.43.1679779938528; Sat, 25 Mar 2023 14:32:18 -0700 (PDT) Received: from localhost ([24.1.27.177]) by smtp.gmail.com with ESMTPSA id mg18-20020a056214561200b005dd8b93457dsm1711403qvb.21.2023.03.25.14.32.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Mar 2023 14:32:18 -0700 (PDT) From: David Vernet To: bpf@vger.kernel.org Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com, haoluo@google.com, jolsa@kernel.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: [PATCH bpf-next 2/3] bpf: Remove now-unnecessary NULL checks for KF_RELEASE kfuncs Date: Sat, 25 Mar 2023 16:31:45 -0500 Message-Id: <20230325213144.486885-3-void@manifault.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230325213144.486885-1-void@manifault.com> References: <20230325213144.486885-1-void@manifault.com> MIME-Version: 1.0 X-Spam-Status: No, score=0.5 required=5.0 tests=FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1761378503556307380?= X-GMAIL-MSGID: =?utf-8?q?1761378503556307380?= Now that we're not invoking kfunc destructors when the kptr in a map was NULL, we no longer require NULL checks in many of our KF_RELEASE kfuncs. This patch removes those NULL checks. Signed-off-by: David Vernet --- drivers/hid/bpf/hid_bpf_dispatch.c | 3 --- kernel/bpf/cpumask.c | 3 --- kernel/bpf/helpers.c | 6 ------ net/bpf/test_run.c | 3 --- net/netfilter/nf_conntrack_bpf.c | 2 -- 5 files changed, 17 deletions(-) diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c index 8a034a555d4c..d9ef45fcaeab 100644 --- a/drivers/hid/bpf/hid_bpf_dispatch.c +++ b/drivers/hid/bpf/hid_bpf_dispatch.c @@ -342,9 +342,6 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx) { struct hid_bpf_ctx_kern *ctx_kern; - if (!ctx) - return; - ctx_kern = container_of(ctx, struct hid_bpf_ctx_kern, ctx); kfree(ctx_kern); diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c index db9da2194c1a..e991af7dc13c 100644 --- a/kernel/bpf/cpumask.c +++ b/kernel/bpf/cpumask.c @@ -102,9 +102,6 @@ static void cpumask_free_cb(struct rcu_head *head) */ __bpf_kfunc void bpf_cpumask_release(struct bpf_cpumask *cpumask) { - if (!cpumask) - return; - if (refcount_dec_and_test(&cpumask->usage)) call_rcu(&cpumask->rcu, cpumask_free_cb); } diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index f753676ef652..8980f6859443 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -2089,9 +2089,6 @@ __bpf_kfunc struct task_struct *bpf_task_kptr_get(struct task_struct **pp) */ __bpf_kfunc void bpf_task_release(struct task_struct *p) { - if (!p) - return; - put_task_struct(p); } @@ -2148,9 +2145,6 @@ __bpf_kfunc struct cgroup *bpf_cgroup_kptr_get(struct cgroup **cgrpp) */ __bpf_kfunc void bpf_cgroup_release(struct cgroup *cgrp) { - if (!cgrp) - return; - cgroup_put(cgrp); } diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 8d6b31209bd6..27587f1c5f36 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -615,9 +615,6 @@ bpf_kfunc_call_memb_acquire(void) __bpf_kfunc void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) { - if (!p) - return; - refcount_dec(&p->cnt); } diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c index cd99e6dc1f35..002e9d24a1e9 100644 --- a/net/netfilter/nf_conntrack_bpf.c +++ b/net/netfilter/nf_conntrack_bpf.c @@ -401,8 +401,6 @@ __bpf_kfunc struct nf_conn *bpf_ct_insert_entry(struct nf_conn___init *nfct_i) */ __bpf_kfunc void bpf_ct_release(struct nf_conn *nfct) { - if (!nfct) - return; nf_ct_put(nfct); } From patchwork Sat Mar 25 21:31:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Vernet X-Patchwork-Id: 74985 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp622748vqo; Sat, 25 Mar 2023 14:57:20 -0700 (PDT) X-Google-Smtp-Source: AKy350Y6/vuk97vgVa0Ed6e7N/qQgBdt7zdX31ihQZFv36oPEmVBH6hvj/14k8mrgGy3cxbJKZll X-Received: by 2002:aa7:db59:0:b0:4fa:7fd8:8f6a with SMTP id n25-20020aa7db59000000b004fa7fd88f6amr6317510edt.38.1679781440343; Sat, 25 Mar 2023 14:57:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1679781440; cv=none; d=google.com; s=arc-20160816; b=HPB/TfVaOPOD7LZgUNSMceQNuBEs8tO2fLgxr5+zyXS5hPjHeAkSY2pBRrjkn1fS60 YkXJzaKGbAdReTUyvAr3LpQMCXyEGkQ42JHLejbBqDDyWBJxWNJqrAUrYc9SL8pzgcpd GGr7qr5O4nL29avrvgKmzg678BM/tNUJva1UpdJM/zKx1qaNz+aOEWnTUBs+mzNOhfr0 +1G8LMVVIP6w48h/gO/KxLeyBNdE/hUi1MhFikpunAH/jQ3fnT9qFQt1dwk/WizgaHlj DTSgPA8m9Rd8O3pr8cloKNYEcV64azsPRtqto7JPR1MuBbmvI+SOOxRGOAmFXlRHi8Xr q7uA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=ymXFAuFh5pPtwakSStE/sGoAQn9sSnVKHVs/cMSsm24=; b=e/SSyjgLjFqxHxbZGR0uRawF8IOlQOkeW5iY19VxVT+OWNLcmV4ijrlBOAjMOrUN9G fxPbVNve9ogc8MrmIushM/Le/28fWIx/HXi3Of2m9QTEKq1+Dcg6u3lU5WZcu4gU/KK0 w9UdiT6Tdp3gju7JVBIOfY1bBuRit3TG0B5k6ySCk9xpRA65OkmZVoXo9aPI+d1N+oAU EwGL4fQvlIIAUBnoQ6QZE/rcUD7he+3ztrBIB7Y5mnTYn1goGYuykq8W5ogBjzzIQBMj BIesNNTV2niMGF6gG0OxwMED7oNI7aQeoA2YFGbLDRL3MS8RmlvP9L1pjXGYhFCZ190E q49A== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id r13-20020aa7cfcd000000b00501d50c5d7csi14177871edy.417.2023.03.25.14.56.48; Sat, 25 Mar 2023 14:57:20 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230286AbjCYVcj (ORCPT + 99 others); Sat, 25 Mar 2023 17:32:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60960 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230317AbjCYVch (ORCPT ); Sat, 25 Mar 2023 17:32:37 -0400 Received: from mail-qv1-f54.google.com (mail-qv1-f54.google.com [209.85.219.54]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 93ED9CDC4; Sat, 25 Mar 2023 14:32:22 -0700 (PDT) Received: by mail-qv1-f54.google.com with SMTP id oe8so4357673qvb.6; Sat, 25 Mar 2023 14:32:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679779941; 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=ymXFAuFh5pPtwakSStE/sGoAQn9sSnVKHVs/cMSsm24=; b=urKKe0As+/i1yMuTglJxqBP1mOnE+zMUZrQaNSv1T82YKu0+KP950K+b85iNskooRl opdsBDY0g8nodhYggisrnzYhGuMvAQMDcn6/5AlzeMQMExR9nGiB3ggA/vGsS0E6nzw4 nksmaDeIAYXnxCrpeulD58J0jw0Zqv4kbc1cROEsblSTOMA5W/VB6taTQXxKuiqdHUT/ LWIKikhkAxZCQjMUHBGWZB/s1HSjZqV08LWUfgphyHlWjyx+vn3MkIH7l4c9QYUGg4gA 0/gvH6JdtXnctcKRYkXBM5DSNSBJ39V7raRfC6B53WzWG7od6qL8BaF529pRAV0bs2xy 4GNA== X-Gm-Message-State: AAQBX9dPvpgVgc7p0Qp9Sxp50XWWwGrmOUuTN5cUro78bRwUr8/Nq9so FA9LpIF6bFLkdpGEG4qcdXe5bvK689H41ecvNjI= X-Received: by 2002:a05:6214:2588:b0:56e:a290:aa92 with SMTP id fq8-20020a056214258800b0056ea290aa92mr13749016qvb.9.1679779941048; Sat, 25 Mar 2023 14:32:21 -0700 (PDT) Received: from localhost ([24.1.27.177]) by smtp.gmail.com with ESMTPSA id mk16-20020a056214581000b005dd8b9345a9sm1698176qvb.65.2023.03.25.14.32.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Mar 2023 14:32:20 -0700 (PDT) From: David Vernet To: bpf@vger.kernel.org Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com, haoluo@google.com, jolsa@kernel.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: [PATCH bpf-next 3/3] bpf: Treat KF_RELEASE kfuncs as KF_TRUSTED_ARGS Date: Sat, 25 Mar 2023 16:31:46 -0500 Message-Id: <20230325213144.486885-4-void@manifault.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230325213144.486885-1-void@manifault.com> References: <20230325213144.486885-1-void@manifault.com> MIME-Version: 1.0 X-Spam-Status: No, score=0.5 required=5.0 tests=FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1761378503319073816?= X-GMAIL-MSGID: =?utf-8?q?1761378503319073816?= KF_RELEASE kfuncs are not currently treated as having KF_TRUSTED_ARGS, even though they have a superset of the requirements of KF_TRUSTED_ARGS. Like KF_TRUSTED_ARGS, KF_RELEASE kfuncs require a 0-offset argument, and don't allow NULL-able arguments. Unlike KF_TRUSTED_ARGS which require _either_ an argument with ref_obj_id > 0, _or_ (ref->type & BPF_REG_TRUSTED_MODIFIERS) (and no unsafe modifiers allowed), KF_RELEASE only allows for ref_obj_id > 0. Because KF_RELEASE today doesn't automatically imply KF_TRUSTED_ARGS, some of these requirements are enforced in different ways that can make the behavior of the verifier feel unpredictable. For example, a KF_RELEASE kfunc with a NULL-able argument will currently fail in the verifier with a message like, "arg#0 is ptr_or_null_ expected ptr_ or socket" rather than "Possibly NULL pointer passed to trusted arg0". Our intention is the same, but the semantics are different due to implemenetation details that kfunc authors and BPF program writers should not need to care about. Let's make the behavior of the verifier more consistent and intuitive by having KF_RELEASE kfuncs imply the presence of KF_TRUSTED_ARGS. Our eventual goal is to have all kfuncs assume KF_TRUSTED_ARGS by default anyways, so this takes us a step in that direction. Note that it does not make sense to assume KF_TRUSTED_ARGS for all KF_ACQUIRE kfuncs. KF_ACQUIRE kfuncs can have looser semantics than KF_RELEASE, with e.g. KF_RCU | KF_RET_NULL. We may want to have KF_ACQUIRE imply KF_TRUSTED_ARGS _unless_ KF_RCU is specified, but that can be left to another patch set, and there are no such subtleties to address for KF_RELEASE. Signed-off-by: David Vernet --- Documentation/bpf/kfuncs.rst | 7 ++++--- kernel/bpf/cpumask.c | 2 +- kernel/bpf/verifier.c | 2 +- net/bpf/test_run.c | 6 ++++++ tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c | 4 ++-- tools/testing/selftests/bpf/progs/task_kfunc_failure.c | 6 +++--- tools/testing/selftests/bpf/verifier/calls.c | 10 +++++++--- tools/testing/selftests/bpf/verifier/ref_tracking.c | 6 +++--- 8 files changed, 27 insertions(+), 16 deletions(-) diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst index 69eccf6f98ef..bf1b85941452 100644 --- a/Documentation/bpf/kfuncs.rst +++ b/Documentation/bpf/kfuncs.rst @@ -179,9 +179,10 @@ both are orthogonal to each other. --------------------- The KF_RELEASE flag is used to indicate that the kfunc releases the pointer -passed in to it. There can be only one referenced pointer that can be passed in. -All copies of the pointer being released are invalidated as a result of invoking -kfunc with this flag. +passed in to it. There can be only one referenced pointer that can be passed +in. All copies of the pointer being released are invalidated as a result of +invoking kfunc with this flag. KF_RELEASE kfuncs automatically receive the +protection afforded by the KF_TRUSTED_ARGS flag described below. 2.4.4 KF_KPTR_GET flag ---------------------- diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c index e991af7dc13c..7efdf5d770ca 100644 --- a/kernel/bpf/cpumask.c +++ b/kernel/bpf/cpumask.c @@ -402,7 +402,7 @@ __diag_pop(); BTF_SET8_START(cpumask_kfunc_btf_ids) BTF_ID_FLAGS(func, bpf_cpumask_create, KF_ACQUIRE | KF_RET_NULL) -BTF_ID_FLAGS(func, bpf_cpumask_release, KF_RELEASE | KF_TRUSTED_ARGS) +BTF_ID_FLAGS(func, bpf_cpumask_release, KF_RELEASE) BTF_ID_FLAGS(func, bpf_cpumask_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS) BTF_ID_FLAGS(func, bpf_cpumask_first, KF_RCU) BTF_ID_FLAGS(func, bpf_cpumask_first_zero, KF_RCU) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 64f06f6e16bf..20eb2015842f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9307,7 +9307,7 @@ static bool is_kfunc_release(struct bpf_kfunc_call_arg_meta *meta) static bool is_kfunc_trusted_args(struct bpf_kfunc_call_arg_meta *meta) { - return meta->kfunc_flags & KF_TRUSTED_ARGS; + return (meta->kfunc_flags & KF_TRUSTED_ARGS) || is_kfunc_release(meta); } static bool is_kfunc_sleepable(struct bpf_kfunc_call_arg_meta *meta) diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 27587f1c5f36..f1652f5fbd2e 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -606,6 +606,11 @@ bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) return &prog_test_struct; } +__bpf_kfunc void bpf_kfunc_call_test_offset(struct prog_test_ref_kfunc *p) +{ + WARN_ON_ONCE(1); +} + __bpf_kfunc struct prog_test_member * bpf_kfunc_call_memb_acquire(void) { @@ -800,6 +805,7 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail2) BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS | KF_RCU) BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE) BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg) +BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset) BTF_SET8_END(test_sk_check_kfunc_ids) static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size, diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c index 807fb0ac41e9..48b2034cadb3 100644 --- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c @@ -206,7 +206,7 @@ int BPF_PROG(cgrp_kfunc_get_unreleased, struct cgroup *cgrp, const char *path) } SEC("tp_btf/cgroup_mkdir") -__failure __msg("expects refcounted") +__failure __msg("Possibly NULL pointer passed to trusted arg0") int BPF_PROG(cgrp_kfunc_release_untrusted, struct cgroup *cgrp, const char *path) { struct __cgrps_kfunc_map_value *v; @@ -234,7 +234,7 @@ int BPF_PROG(cgrp_kfunc_release_fp, struct cgroup *cgrp, const char *path) } SEC("tp_btf/cgroup_mkdir") -__failure __msg("arg#0 is ptr_or_null_ expected ptr_ or socket") +__failure __msg("Possibly NULL pointer passed to trusted arg0") int BPF_PROG(cgrp_kfunc_release_null, struct cgroup *cgrp, const char *path) { struct __cgrps_kfunc_map_value local, *v; diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c index 27994d6b2914..2c374a7ffece 100644 --- a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c @@ -206,7 +206,7 @@ int BPF_PROG(task_kfunc_get_unreleased, struct task_struct *task, u64 clone_flag } SEC("tp_btf/task_newtask") -__failure __msg("arg#0 is untrusted_ptr_or_null_ expected ptr_ or socket") +__failure __msg("Possibly NULL pointer passed to trusted arg0") int BPF_PROG(task_kfunc_release_untrusted, struct task_struct *task, u64 clone_flags) { struct __tasks_kfunc_map_value *v; @@ -234,7 +234,7 @@ int BPF_PROG(task_kfunc_release_fp, struct task_struct *task, u64 clone_flags) } SEC("tp_btf/task_newtask") -__failure __msg("arg#0 is ptr_or_null_ expected ptr_ or socket") +__failure __msg("Possibly NULL pointer passed to trusted arg0") int BPF_PROG(task_kfunc_release_null, struct task_struct *task, u64 clone_flags) { struct __tasks_kfunc_map_value local, *v; @@ -277,7 +277,7 @@ int BPF_PROG(task_kfunc_release_unacquired, struct task_struct *task, u64 clone_ } SEC("tp_btf/task_newtask") -__failure __msg("arg#0 is ptr_or_null_ expected ptr_ or socket") +__failure __msg("Possibly NULL pointer passed to trusted arg0") int BPF_PROG(task_kfunc_from_pid_no_null_check, struct task_struct *task, u64 clone_flags) { struct task_struct *acquired; diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c index 5702fc9761ef..1bdf2b43e49e 100644 --- a/tools/testing/selftests/bpf/verifier/calls.c +++ b/tools/testing/selftests/bpf/verifier/calls.c @@ -109,7 +109,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "arg#0 is ptr_or_null_ expected ptr_ or socket", + .errstr = "Possibly NULL pointer passed to trusted arg0", .fixup_kfunc_btf_id = { { "bpf_kfunc_call_test_acquire", 3 }, { "bpf_kfunc_call_test_release", 5 }, @@ -165,19 +165,23 @@ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8), BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 0), BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_0), BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1), BPF_EXIT_INSN(), BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), - BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 16), BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -4), BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0), BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_2), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0), + BPF_MOV64_IMM(BPF_REG_0, 0), BPF_EXIT_INSN(), }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .fixup_kfunc_btf_id = { { "bpf_kfunc_call_test_acquire", 3 }, - { "bpf_kfunc_call_test_release", 9 }, + { "bpf_kfunc_call_test_offset", 9 }, + { "bpf_kfunc_call_test_release", 12 }, }, .result_unpriv = REJECT, .result = REJECT, diff --git a/tools/testing/selftests/bpf/verifier/ref_tracking.c b/tools/testing/selftests/bpf/verifier/ref_tracking.c index 9540164712b7..5a2e154dd1e0 100644 --- a/tools/testing/selftests/bpf/verifier/ref_tracking.c +++ b/tools/testing/selftests/bpf/verifier/ref_tracking.c @@ -142,7 +142,7 @@ .kfunc = "bpf", .expected_attach_type = BPF_LSM_MAC, .flags = BPF_F_SLEEPABLE, - .errstr = "arg#0 is ptr_or_null_ expected ptr_ or socket", + .errstr = "Possibly NULL pointer passed to trusted arg0", .fixup_kfunc_btf_id = { { "bpf_lookup_user_key", 2 }, { "bpf_key_put", 4 }, @@ -163,7 +163,7 @@ .kfunc = "bpf", .expected_attach_type = BPF_LSM_MAC, .flags = BPF_F_SLEEPABLE, - .errstr = "arg#0 is ptr_or_null_ expected ptr_ or socket", + .errstr = "Possibly NULL pointer passed to trusted arg0", .fixup_kfunc_btf_id = { { "bpf_lookup_system_key", 1 }, { "bpf_key_put", 3 }, @@ -182,7 +182,7 @@ .kfunc = "bpf", .expected_attach_type = BPF_LSM_MAC, .flags = BPF_F_SLEEPABLE, - .errstr = "arg#0 pointer type STRUCT bpf_key must point to scalar, or struct with scalar", + .errstr = "Possibly NULL pointer passed to trusted arg0", .fixup_kfunc_btf_id = { { "bpf_key_put", 1 }, },