From patchwork Fri Jun 2 02:26:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13264673 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6690E15C5 for ; Fri, 2 Jun 2023 02:27:11 +0000 (UTC) Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C411218D for ; Thu, 1 Jun 2023 19:27:09 -0700 (PDT) Received: from pps.filterd (m0109333.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 351NtUor001168 for ; Thu, 1 Jun 2023 19:27:09 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=facebook; bh=IMALPJuf/MNN1opfG4ETfKGSVvmPnkhGOfCzAfooE4o=; b=ngbYDLsufn2eHabHjNV+m6gkuUh41TAsWAY8ojkBuKMChw3QU9mP/PDqcMzapVUPLqUF LdH/FQ3qDf4vn+2egdGUNttQmzbdr04bli4UAStF1mnI3MB1AkF2jx5Nef2hw3Y/fxEK iW4bSZ/tT7e52EPGZVtQTOoJNNBr5aG2JrM= Received: from maileast.thefacebook.com ([163.114.130.16]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3qxxpa457v-3 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 01 Jun 2023 19:27:09 -0700 Received: from twshared8338.02.ash9.facebook.com (2620:10d:c0a8:1c::11) by mail.thefacebook.com (2620:10d:c0a8:83::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 1 Jun 2023 19:27:06 -0700 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id 1BF651EF7C85D; Thu, 1 Jun 2023 19:26:49 -0700 (PDT) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kernel Team , Dave Marchevsky Subject: [PATCH v2 bpf-next 1/9] [DONOTAPPLY] Revert "bpf: Disable bpf_refcount_acquire kfunc calls until race conditions are fixed" Date: Thu, 1 Jun 2023 19:26:39 -0700 Message-ID: <20230602022647.1571784-2-davemarchevsky@fb.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230602022647.1571784-1-davemarchevsky@fb.com> References: <20230602022647.1571784-1-davemarchevsky@fb.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-GUID: aq_pfzQLBNg7Wxr57IMeBZf6Ij50wKmz X-Proofpoint-ORIG-GUID: aq_pfzQLBNg7Wxr57IMeBZf6Ij50wKmz X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net This patch should not be applied with the rest of the series as the series doesn't fix all bpf_refcount issues. It is included here to allow CI to run the test added later in the series. Followup series which fixes the remaining problems will include a revert that should be applied. Signed-off-by: Dave Marchevsky --- kernel/bpf/verifier.c | 4 ---- tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c | 2 ++ 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 086b2a14905b..0f04e7fe4843 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -10893,10 +10893,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ verbose(env, "arg#%d doesn't point to a type with bpf_refcount field\n", i); return -EINVAL; } - if (rec->refcount_off >= 0) { - verbose(env, "bpf_refcount_acquire calls are disabled for now\n"); - return -EINVAL; - } meta->arg_btf = reg->btf; meta->arg_btf_id = reg->btf_id; break; diff --git a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c index 595cbf92bff5..2ab23832062d 100644 --- a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c +++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c @@ -9,8 +9,10 @@ void test_refcounted_kptr(void) { + RUN_TESTS(refcounted_kptr); } void test_refcounted_kptr_fail(void) { + RUN_TESTS(refcounted_kptr_fail); } From patchwork Fri Jun 2 02:26:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13264671 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 600CD15A3 for ; Fri, 2 Jun 2023 02:27:07 +0000 (UTC) Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 34E6D195 for ; Thu, 1 Jun 2023 19:27:03 -0700 (PDT) Received: from pps.filterd (m0148461.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 351NtGeN005118 for ; Thu, 1 Jun 2023 19:27:03 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=facebook; bh=wEpYHCqmxsS8+kFTO4SOb2VhUOHcTo0DuZ5l1z/dDsk=; b=RCbA26toQR7gU5ZPnEctfJWyM2Q5HBErsml2165XikRyn0LAVOFEg2jYxoZ/zGVZMyhz hixXKtegcnGMdF41fEGTCf0ae8O5Ppx09Y/aSKm96JOS0sD8CySJ7umGuyhYoqXKg6Qs N+/wrLaHoXhpbUwymoY5aukuSVTKXyJvlaA= Received: from mail.thefacebook.com ([163.114.132.120]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3qxt1kpuad-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 01 Jun 2023 19:27:02 -0700 Received: from twshared19038.03.ash8.facebook.com (2620:10d:c085:208::f) by mail.thefacebook.com (2620:10d:c085:21d::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 1 Jun 2023 19:27:02 -0700 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id 19C4A1EF7C8B7; Thu, 1 Jun 2023 19:26:53 -0700 (PDT) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kernel Team , Dave Marchevsky Subject: [PATCH v2 bpf-next 2/9] bpf: Set kptr_struct_meta for node param to list and rbtree insert funcs Date: Thu, 1 Jun 2023 19:26:40 -0700 Message-ID: <20230602022647.1571784-3-davemarchevsky@fb.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230602022647.1571784-1-davemarchevsky@fb.com> References: <20230602022647.1571784-1-davemarchevsky@fb.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: qCB4MmxFU26-GOmJaFUENHPSIfwmMb4Z X-Proofpoint-GUID: qCB4MmxFU26-GOmJaFUENHPSIfwmMb4Z X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net In verifier.c, fixup_kfunc_call uses struct bpf_insn_aux_data's kptr_struct_meta field to pass information about local kptr types to various helpers and kfuncs at runtime. The recent bpf_refcount series added a few functions to the set that need this information: * bpf_refcount_acquire * Needs to know where the refcount field is in order to increment * Graph collection insert kfuncs: bpf_rbtree_add, bpf_list_push_{front,back} * Were migrated to possibly fail by the bpf_refcount series. If insert fails, the input node is bpf_obj_drop'd. bpf_obj_drop needs the kptr_struct_meta in order to decr refcount and properly free special fields. Unfortunately the verifier handling of collection insert kfuncs was not modified to actually populate kptr_struct_meta. Accordingly, when the node input to those kfuncs is passed to bpf_obj_drop, it is done so without the information necessary to decr refcount. This patch fixes the issue by populating kptr_struct_meta for those kfuncs. Fixes: d2dcc67df910 ("bpf: Migrate bpf_rbtree_add and bpf_list_push_{front,back} to possibly fail") Signed-off-by: Dave Marchevsky --- kernel/bpf/verifier.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 0f04e7fe4843..43d26b65a3ad 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -10475,6 +10475,8 @@ __process_kf_arg_ptr_to_graph_node(struct bpf_verifier_env *env, node_off, btf_name_by_offset(reg->btf, t->name_off)); return -EINVAL; } + meta->arg_btf = reg->btf; + meta->arg_btf_id = reg->btf_id; if (node_off != field->graph_root.node_offset) { verbose(env, "arg#1 offset=%d, but expected %s at offset=%d in struct %s\n", @@ -11040,6 +11042,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, meta.func_id == special_kfunc_list[KF_bpf_rbtree_add_impl]) { release_ref_obj_id = regs[BPF_REG_2].ref_obj_id; insn_aux->insert_off = regs[BPF_REG_2].off; + insn_aux->kptr_struct_meta = btf_find_struct_meta(meta.arg_btf, meta.arg_btf_id); err = ref_convert_owning_non_owning(env, release_ref_obj_id); if (err) { verbose(env, "kfunc %s#%d conversion of owning ref to non-owning failed\n", From patchwork Fri Jun 2 02:26:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13264670 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 138F015A3 for ; Fri, 2 Jun 2023 02:27:07 +0000 (UTC) Received: from mx0b-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1405318D for ; Thu, 1 Jun 2023 19:27:01 -0700 (PDT) Received: from pps.filterd (m0148460.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 351NtdPt019778 for ; Thu, 1 Jun 2023 19:27:01 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=facebook; bh=+Q5M06ZRuB/Ny2GViAb654mq7R5hQJIac26GyJ/9MR8=; b=LZvlN1wsDekKs5vZUN9oAJi8orkk4eZ+Lp0P0qKyNgUcb7wGWhNyi0c4uqRFSdKMf1RS 2Gv7pAobXmU3jrC0u2tVaZ2LorNY2KtsZb82llBDueZOPOUK8co4dVgDQZiKDlqejc2M jAEXlUfS0ZDctR0PNdtgJUHkOn3Lft0Ki2s= Received: from maileast.thefacebook.com ([163.114.130.16]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3qxt1s6cvt-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 01 Jun 2023 19:27:01 -0700 Received: from twshared35445.38.frc1.facebook.com (2620:10d:c0a8:1c::1b) by mail.thefacebook.com (2620:10d:c0a8:83::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 1 Jun 2023 19:27:00 -0700 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id C4AFC1EF7C8C0; Thu, 1 Jun 2023 19:26:53 -0700 (PDT) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kernel Team , Dave Marchevsky Subject: [PATCH v2 bpf-next 3/9] bpf: Fix __bpf_{list,rbtree}_add's beginning-of-node calculation Date: Thu, 1 Jun 2023 19:26:41 -0700 Message-ID: <20230602022647.1571784-4-davemarchevsky@fb.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230602022647.1571784-1-davemarchevsky@fb.com> References: <20230602022647.1571784-1-davemarchevsky@fb.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: DnI0TbZEndiWZRsVHIJkl87Bso9Tbmdb X-Proofpoint-GUID: DnI0TbZEndiWZRsVHIJkl87Bso9Tbmdb X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net Given the pointer to struct bpf_{rb,list}_node within a local kptr and the byte offset of that field within the kptr struct, the calculation changed by this patch is meant to find the beginning of the kptr so that it can be passed to bpf_obj_drop. Unfortunately instead of doing ptr_to_kptr = ptr_to_node_field - offset_bytes the calculation is erroneously doing ptr_to_ktpr = ptr_to_node_field - (offset_bytes * sizeof(struct bpf_rb_node)) or the bpf_list_node equivalent. This patch fixes the calculation. Fixes: d2dcc67df910 ("bpf: Migrate bpf_rbtree_add and bpf_list_push_{front,back} to possibly fail") Signed-off-by: Dave Marchevsky --- kernel/bpf/helpers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 4ef4c4f8a355..a4e437eabcb4 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1950,7 +1950,7 @@ static int __bpf_list_add(struct bpf_list_node *node, struct bpf_list_head *head INIT_LIST_HEAD(h); if (!list_empty(n)) { /* Only called from BPF prog, no need to migrate_disable */ - __bpf_obj_drop_impl(n - off, rec); + __bpf_obj_drop_impl((void *)n - off, rec); return -EINVAL; } @@ -2032,7 +2032,7 @@ static int __bpf_rbtree_add(struct bpf_rb_root *root, struct bpf_rb_node *node, if (!RB_EMPTY_NODE(n)) { /* Only called from BPF prog, no need to migrate_disable */ - __bpf_obj_drop_impl(n - off, rec); + __bpf_obj_drop_impl((void *)n - off, rec); return -EINVAL; } From patchwork Fri Jun 2 02:26:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13264669 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1925015A3 for ; Fri, 2 Jun 2023 02:27:06 +0000 (UTC) Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F3D6D184 for ; Thu, 1 Jun 2023 19:27:01 -0700 (PDT) Received: from pps.filterd (m0109333.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 351NtU1D001147 for ; Thu, 1 Jun 2023 19:27:01 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : content-type : content-transfer-encoding : mime-version; s=facebook; bh=R+PLP9Ajq/IVxQTuFFWfZSzFMlSvZnYS3K619WP2fS4=; b=bGefK7bPdINEomMLmsqnjlCtmvDs5VUeNA4XLaTz+RpJdlfvQvFcSy7LM9h3vl4MIoxI f8zzTam3VsbMZXVtwSpRgGjvg1SDKnBbBmOsPM1COCToW/jS7y2EpXTxwe2PiPAUt/Mg EMKDqJFWfFzWGWPABQtnvgCs2bhW5FTyYpc= Received: from mail.thefacebook.com ([163.114.132.120]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3qxxpa4574-3 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 01 Jun 2023 19:27:01 -0700 Received: from twshared16624.09.ash9.facebook.com (2620:10d:c085:108::8) by mail.thefacebook.com (2620:10d:c085:11d::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 1 Jun 2023 19:26:59 -0700 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id 5EF0E1EF7C8CB; Thu, 1 Jun 2023 19:26:54 -0700 (PDT) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kernel Team , Dave Marchevsky , Kumar Kartikeya Dwivedi Subject: [PATCH v2 bpf-next 4/9] bpf: Make bpf_refcount_acquire fallible for non-owning refs Date: Thu, 1 Jun 2023 19:26:42 -0700 Message-ID: <20230602022647.1571784-5-davemarchevsky@fb.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230602022647.1571784-1-davemarchevsky@fb.com> References: <20230602022647.1571784-1-davemarchevsky@fb.com> X-FB-Internal: Safe X-Proofpoint-GUID: VldP4yUdy_o8CS7MeSlgAHiBZWx4vJd8 X-Proofpoint-ORIG-GUID: VldP4yUdy_o8CS7MeSlgAHiBZWx4vJd8 X-Proofpoint-UnRewURL: 0 URL was un-rewritten Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net This patch fixes an incorrect assumption made in the original bpf_refcount series [0], specifically that the BPF program calling bpf_refcount_acquire on some node can always guarantee that the node is alive. In that series, the patch adding failure behavior to rbtree_add and list_push_{front, back} breaks this assumption for non-owning references. Consider the following program: n = bpf_kptr_xchg(&mapval, NULL); /* skip error checking */ bpf_spin_lock(&l); if(bpf_rbtree_add(&t, &n->rb, less)) { bpf_refcount_acquire(n); /* Failed to add, do something else with the node */ } bpf_spin_unlock(&l); It's incorrect to assume that bpf_refcount_acquire will always succeed in this scenario. bpf_refcount_acquire is being called in a critical section here, but the lock being held is associated with rbtree t, which isn't necessarily the lock associated with the tree that the node is already in. So after bpf_rbtree_add fails to add the node and calls bpf_obj_drop in it, the program has no ownership of the node's lifetime. Therefore the node's refcount can be decr'd to 0 at any time after the failing rbtree_add. If this happens before the refcount_acquire above, the node might be free'd, and regardless refcount_acquire will be incrementing a 0 refcount. Later patches in the series exercise this scenario, resulting in the expected complaint from the kernel (without this patch's changes): refcount_t: addition on 0; use-after-free. WARNING: CPU: 1 PID: 207 at lib/refcount.c:25 refcount_warn_saturate+0xbc/0x110 Modules linked in: bpf_testmod(O) CPU: 1 PID: 207 Comm: test_progs Tainted: G O 6.3.0-rc7-02231-g723de1a718a2-dirty #371 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014 RIP: 0010:refcount_warn_saturate+0xbc/0x110 Code: 6f 64 f6 02 01 e8 84 a3 5c ff 0f 0b eb 9d 80 3d 5e 64 f6 02 00 75 94 48 c7 c7 e0 13 d2 82 c6 05 4e 64 f6 02 01 e8 64 a3 5c ff <0f> 0b e9 7a ff ff ff 80 3d 38 64 f6 02 00 0f 85 6d ff ff ff 48 c7 RSP: 0018:ffff88810b9179b0 EFLAGS: 00010082 RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000 RDX: 0000000000000202 RSI: 0000000000000008 RDI: ffffffff857c3680 RBP: ffff88810027d3c0 R08: ffffffff8125f2a4 R09: ffff88810b9176e7 R10: ffffed1021722edc R11: 746e756f63666572 R12: ffff88810027d388 R13: ffff88810027d3c0 R14: ffffc900005fe030 R15: ffffc900005fe048 FS: 00007fee0584a700(0000) GS:ffff88811b280000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00005634a96f6c58 CR3: 0000000108ce9002 CR4: 0000000000770ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: bpf_refcount_acquire_impl+0xb5/0xc0 (rest of output snipped) The patch addresses this by changing bpf_refcount_acquire_impl to use refcount_inc_not_zero instead of refcount_inc and marking bpf_refcount_acquire KF_RET_NULL. For owning references, though, we know the above scenario is not possible and thus that bpf_refcount_acquire will always succeed. Some verifier bookkeeping is added to track "is input owning ref?" for bpf_refcount_acquire calls and return false from is_kfunc_ret_null for bpf_refcount_acquire on owning refs despite it being marked KF_RET_NULL. Existing selftests using bpf_refcount_acquire are modified where necessary to NULL-check its return value. [0]: https://lore.kernel.org/bpf/20230415201811.343116-1-davemarchevsky@fb.com/ Fixes: d2dcc67df910 ("bpf: Migrate bpf_rbtree_add and bpf_list_push_{front,back} to possibly fail") Reported-by: Kumar Kartikeya Dwivedi Signed-off-by: Dave Marchevsky --- kernel/bpf/helpers.c | 8 ++++-- kernel/bpf/verifier.c | 26 +++++++++++++------ .../selftests/bpf/progs/refcounted_kptr.c | 2 ++ .../bpf/progs/refcounted_kptr_fail.c | 4 ++- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index a4e437eabcb4..9e80efa59a5d 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1933,8 +1933,12 @@ __bpf_kfunc void *bpf_refcount_acquire_impl(void *p__refcounted_kptr, void *meta * bpf_refcount type so that it is emitted in vmlinux BTF */ ref = (struct bpf_refcount *)(p__refcounted_kptr + meta->record->refcount_off); + if (!refcount_inc_not_zero((refcount_t *)ref)) + return NULL; - refcount_inc((refcount_t *)ref); + /* Verifier strips KF_RET_NULL if input is owned ref, see is_kfunc_ret_null + * in verifier.c + */ return (void *)p__refcounted_kptr; } @@ -2406,7 +2410,7 @@ BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE) #endif BTF_ID_FLAGS(func, bpf_obj_new_impl, KF_ACQUIRE | KF_RET_NULL) BTF_ID_FLAGS(func, bpf_obj_drop_impl, KF_RELEASE) -BTF_ID_FLAGS(func, bpf_refcount_acquire_impl, KF_ACQUIRE) +BTF_ID_FLAGS(func, bpf_refcount_acquire_impl, KF_ACQUIRE | KF_RET_NULL) BTF_ID_FLAGS(func, bpf_list_push_front_impl) BTF_ID_FLAGS(func, bpf_list_push_back_impl) BTF_ID_FLAGS(func, bpf_list_pop_front, KF_ACQUIRE | KF_RET_NULL) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 43d26b65a3ad..48c3e2bbcc4a 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -298,16 +298,19 @@ struct bpf_kfunc_call_arg_meta { bool found; } arg_constant; - /* arg_btf and arg_btf_id are used by kfunc-specific handling, + /* arg_{btf,btf_id,owning_ref} are used by kfunc-specific handling, * generally to pass info about user-defined local kptr types to later * verification logic * bpf_obj_drop * Record the local kptr type to be drop'd * bpf_refcount_acquire (via KF_ARG_PTR_TO_REFCOUNTED_KPTR arg type) - * Record the local kptr type to be refcount_incr'd + * Record the local kptr type to be refcount_incr'd and use + * arg_owning_ref to determine whether refcount_acquire should be + * fallible */ struct btf *arg_btf; u32 arg_btf_id; + bool arg_owning_ref; struct { struct btf_field *field; @@ -9678,11 +9681,6 @@ static bool is_kfunc_acquire(struct bpf_kfunc_call_arg_meta *meta) return meta->kfunc_flags & KF_ACQUIRE; } -static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta) -{ - return meta->kfunc_flags & KF_RET_NULL; -} - static bool is_kfunc_release(struct bpf_kfunc_call_arg_meta *meta) { return meta->kfunc_flags & KF_RELEASE; @@ -9998,6 +9996,16 @@ BTF_ID(func, bpf_dynptr_slice) BTF_ID(func, bpf_dynptr_slice_rdwr) BTF_ID(func, bpf_dynptr_clone) +static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta) +{ + if (meta->func_id == special_kfunc_list[KF_bpf_refcount_acquire_impl] && + meta->arg_owning_ref) { + return false; + } + + return meta->kfunc_flags & KF_RET_NULL; +} + static bool is_kfunc_bpf_rcu_read_lock(struct bpf_kfunc_call_arg_meta *meta) { return meta->func_id == special_kfunc_list[KF_bpf_rcu_read_lock]; @@ -10880,10 +10888,12 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ meta->subprogno = reg->subprogno; break; case KF_ARG_PTR_TO_REFCOUNTED_KPTR: - if (!type_is_ptr_alloc_obj(reg->type) && !type_is_non_owning_ref(reg->type)) { + if (!type_is_ptr_alloc_obj(reg->type)) { verbose(env, "arg#%d is neither owning or non-owning ref\n", i); return -EINVAL; } + if (!type_is_non_owning_ref(reg->type)) + meta->arg_owning_ref = true; rec = reg_btf_record(reg); if (!rec) { diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c index 1d348a225140..a3da610b1e6b 100644 --- a/tools/testing/selftests/bpf/progs/refcounted_kptr.c +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c @@ -375,6 +375,8 @@ long rbtree_refcounted_node_ref_escapes(void *ctx) bpf_rbtree_add(&aroot, &n->node, less_a); m = bpf_refcount_acquire(n); bpf_spin_unlock(&alock); + if (!m) + return 2; m->key = 2; bpf_obj_drop(m); diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c index efcb308f80ad..0b09e5c915b1 100644 --- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c @@ -29,7 +29,7 @@ static bool less(struct bpf_rb_node *a, const struct bpf_rb_node *b) } SEC("?tc") -__failure __msg("Unreleased reference id=3 alloc_insn=21") +__failure __msg("Unreleased reference id=4 alloc_insn=21") long rbtree_refcounted_node_ref_escapes(void *ctx) { struct node_acquire *n, *m; @@ -43,6 +43,8 @@ long rbtree_refcounted_node_ref_escapes(void *ctx) /* m becomes an owning ref but is never drop'd or added to a tree */ m = bpf_refcount_acquire(n); bpf_spin_unlock(&glock); + if (!m) + return 2; m->key = 2; return 0; From patchwork Fri Jun 2 02:26:43 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13264667 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AEAC11363 for ; Fri, 2 Jun 2023 02:27:00 +0000 (UTC) Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E24B518D for ; Thu, 1 Jun 2023 19:26:58 -0700 (PDT) Received: from pps.filterd (m0109334.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 351Nt4ho016162 for ; Thu, 1 Jun 2023 19:26:58 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=facebook; bh=UQU9wiJb0Ih5CzkMPhP3koVPPJKeT1ByxWjFlhKEBDQ=; b=I/aoPS/6kuP8gt7/l1p/kTtMjhTCauIZFaRhd90YGrclz6rUIh7lgK8/6nXNYbLWNGtV DjSdq9NrzwjOoNDygywdWpvAnOYS+taAcMPmuSuGSk7lr5SY4hoSssAM+68Ba2L4MRXp 3052kngH+CHN57+J2s5w1QYq3MevzZv19sQ= Received: from mail.thefacebook.com ([163.114.132.120]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3qxxb5mffj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 01 Jun 2023 19:26:58 -0700 Received: from twshared29819.07.ash9.facebook.com (2620:10d:c085:208::11) by mail.thefacebook.com (2620:10d:c085:11d::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 1 Jun 2023 19:26:57 -0700 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id E324D1EF7C8D5; Thu, 1 Jun 2023 19:26:54 -0700 (PDT) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kernel Team , Dave Marchevsky Subject: [PATCH v2 bpf-next 5/9] [DONOTAPPLY] bpf: Allow KF_DESTRUCTIVE-flagged kfuncs to be called under spinlock Date: Thu, 1 Jun 2023 19:26:43 -0700 Message-ID: <20230602022647.1571784-6-davemarchevsky@fb.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230602022647.1571784-1-davemarchevsky@fb.com> References: <20230602022647.1571784-1-davemarchevsky@fb.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-GUID: DRiWprJKdkoMnKj6zDblRwonuacI6aaR X-Proofpoint-ORIG-GUID: DRiWprJKdkoMnKj6zDblRwonuacI6aaR X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net In order to prevent deadlock the verifier currently disallows any function calls under bpf_spin_lock save for a small set of allowlisted helpers/kfuncs. A BPF program that calls destructive kfuncs might be trying to cause deadlock, and regardless is understood to be capable of causing system breakage of similar severity. Per kfuncs.rst: The KF_DESTRUCTIVE flag is used to indicate functions calling which is destructive to the system. For example such a call can result in system rebooting or panicking. Due to this additional restrictions apply to these calls. Preventing BPF programs from crashing or otherwise blowing up the system is generally the verifier's goal, but destructive kfuncs might have such a state be their intended result. Preventing KF_DESTRUCTIVE kfunc calls under spinlock with the goal of safety is therefore unnecessarily strict. This patch modifies the "function calls are not allowed while holding a lock" check to allow calling destructive kfuncs with an active_lock. The motivating usecase for this change - unsafe locking of bpf_spin_locks for easy testing of race conditions - is implemented in the next two patches in the series. Note that the removed insn->off check was rejecting any calls to kfuncs defined in non-vmlinux BTF. In order to get the system in a broken or otherwise interesting state for inspection, developers might load a module implementing destructive kfuncs particular to their usecase. The unsafe_spin_{lock, unlock} kfuncs later in this series are a good example: there's no clear reason for them to be in vmlinux as they're specifically for BPF selftests, so they live in bpf_testmod. The check is removed in favor of a newly-added helper function to enable such usecases. Signed-off-by: Dave Marchevsky --- kernel/bpf/verifier.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 48c3e2bbcc4a..1bf0e6411feb 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -330,6 +330,11 @@ struct bpf_kfunc_call_arg_meta { u64 mem_size; }; +static int fetch_kfunc_meta(struct bpf_verifier_env *env, + struct bpf_insn *insn, + struct bpf_kfunc_call_arg_meta *meta, + const char **kfunc_name); + struct btf *btf_vmlinux; static DEFINE_MUTEX(bpf_verifier_lock); @@ -10313,6 +10318,21 @@ static bool is_rbtree_lock_required_kfunc(u32 btf_id) return is_bpf_rbtree_api_kfunc(btf_id); } +static bool is_kfunc_callable_in_spinlock(struct bpf_verifier_env *env, + struct bpf_insn *insn) +{ + struct bpf_kfunc_call_arg_meta meta; + + /* insn->off is idx into btf fd_array - 0 for vmlinux btf, else nonzero */ + if (!insn->off && is_bpf_graph_api_kfunc(insn->imm)) + return true; + + if (fetch_kfunc_meta(env, insn, &meta, NULL)) + return false; + + return is_kfunc_destructive(&meta); +} + static bool check_kfunc_is_graph_root_api(struct bpf_verifier_env *env, enum btf_field_type head_field_type, u32 kfunc_btf_id) @@ -16218,7 +16238,7 @@ static int do_check(struct bpf_verifier_env *env) if ((insn->src_reg == BPF_REG_0 && insn->imm != BPF_FUNC_spin_unlock) || (insn->src_reg == BPF_PSEUDO_CALL) || (insn->src_reg == BPF_PSEUDO_KFUNC_CALL && - (insn->off != 0 || !is_bpf_graph_api_kfunc(insn->imm)))) { + !is_kfunc_callable_in_spinlock(env, insn))) { verbose(env, "function calls are not allowed while holding a lock\n"); return -EINVAL; } From patchwork Fri Jun 2 02:26:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13264674 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BB1E315AC for ; Fri, 2 Jun 2023 02:27:12 +0000 (UTC) Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 33F42180 for ; Thu, 1 Jun 2023 19:27:11 -0700 (PDT) Received: from pps.filterd (m0044012.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 351Ntt2i020129 for ; Thu, 1 Jun 2023 19:27:11 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=facebook; bh=WnkbqwD469opZcsZwnn6tu/4uQcYVyNcJxQq+7/GH/M=; b=IGXS36vFb9rHdJbsK31LdSoQMztEKY5AdrC8upAbNZhAIpmF49hyQrpcI+2jBjrtg010 WevrKyeiQzKpgEQKBgNUA619IgZBsMiJr7GgXa3gFVxsy0EU/xO/QrTdG2DuQ0yhnF8Q dc30ya9/XCAVlhRc8IzY+Wt5aKO4s0zkX1M= Received: from mail.thefacebook.com ([163.114.132.120]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3qxcvfhm8s-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 01 Jun 2023 19:27:10 -0700 Received: from twshared9332.02.ash9.facebook.com (2620:10d:c085:208::11) by mail.thefacebook.com (2620:10d:c085:21d::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 1 Jun 2023 19:27:10 -0700 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id 6D2A71EF7C8E3; Thu, 1 Jun 2023 19:26:55 -0700 (PDT) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kernel Team , Dave Marchevsky Subject: [PATCH v2 bpf-next 6/9] [DONOTAPPLY] selftests/bpf: Add unsafe lock/unlock and refcount_read kfuncs to bpf_testmod Date: Thu, 1 Jun 2023 19:26:44 -0700 Message-ID: <20230602022647.1571784-7-davemarchevsky@fb.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230602022647.1571784-1-davemarchevsky@fb.com> References: <20230602022647.1571784-1-davemarchevsky@fb.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: JboYpShVh5WqsSkzjhVwFeh9w_L7fZfo X-Proofpoint-GUID: JboYpShVh5WqsSkzjhVwFeh9w_L7fZfo X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net [ RFC: This patch currently copies static inline helpers: __bpf_spin_lock __bpf_spin_unlock __bpf_spin_lock_irqsave __bpf_spin_unlock_irqrestore from kernel/bpf/helpers.c . The definition of these helpers is config-dependant and they're not meant to be called from a module, so not sure how to proceed here. ] This patch adds three unsafe kfuncs to bpf_testmod for use in selftests: - bpf__unsafe_spin_lock - bpf__unsafe_spin_unlock - bpf_refcount_read The first two are equivalent to bpf_spin_{lock, unlock}, except without any special treatment from the verifier, which allows them to be used in tests to guarantee a specific interleaving of program execution. This will simplify testing race conditions in BPF programs, as demonstrated in further patches in the series. The kfuncs are marked KF_DESTRUCTIVE as they can easily cause deadlock, and are only intended to be used in tests. bpf_refcount_read simply reads the refcount from the uapi-opaque bpf_refcount struct and returns it. This allows more precise testing of specific bpf_refcount scenarios, also demonstrated in further patches in the series. Although this kfunc can't break the system as catastrophically as the unsafe locking kfuncs, it's also marked KF_DESTRUCTIVE as it relies on bpf_refcount implementation details, and shouldn't be used outside of tests regardless. Signed-off-by: Dave Marchevsky --- .../selftests/bpf/bpf_testmod/bpf_testmod.c | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c index cf216041876c..abac7a212ec2 100644 --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c @@ -109,6 +109,64 @@ __bpf_kfunc void bpf_iter_testmod_seq_destroy(struct bpf_iter_testmod_seq *it) it->cnt = 0; } +/* BEGIN copied from kernel/bpf/helpers.c */ +static DEFINE_PER_CPU(unsigned long, irqsave_flags); + +static inline void __bpf_spin_lock(struct bpf_spin_lock *lock) +{ + arch_spinlock_t *l = (void *)lock; + union { + __u32 val; + arch_spinlock_t lock; + } u = { .lock = __ARCH_SPIN_LOCK_UNLOCKED }; + + compiletime_assert(u.val == 0, "__ARCH_SPIN_LOCK_UNLOCKED not 0"); + BUILD_BUG_ON(sizeof(*l) != sizeof(__u32)); + BUILD_BUG_ON(sizeof(*lock) != sizeof(__u32)); + arch_spin_lock(l); +} + +static inline void __bpf_spin_unlock(struct bpf_spin_lock *lock) +{ + arch_spinlock_t *l = (void *)lock; + + arch_spin_unlock(l); +} + +static inline void __bpf_spin_lock_irqsave(struct bpf_spin_lock *lock) +{ + unsigned long flags; + + local_irq_save(flags); + __bpf_spin_lock(lock); + __this_cpu_write(irqsave_flags, flags); +} + +static inline void __bpf_spin_unlock_irqrestore(struct bpf_spin_lock *lock) +{ + unsigned long flags; + + flags = __this_cpu_read(irqsave_flags); + __bpf_spin_unlock(lock); + local_irq_restore(flags); +} +/* END copied from kernel/bpf/helpers.c */ + +__bpf_kfunc void bpf__unsafe_spin_lock(void *lock__ign) +{ + __bpf_spin_lock_irqsave((struct bpf_spin_lock *)lock__ign); +} + +__bpf_kfunc void bpf__unsafe_spin_unlock(void *lock__ign) +{ + __bpf_spin_unlock_irqrestore((struct bpf_spin_lock *)lock__ign); +} + +__bpf_kfunc int bpf_refcount_read(void *refcount__ign) +{ + return refcount_read((refcount_t *)refcount__ign); +} + struct bpf_testmod_btf_type_tag_1 { int a; }; @@ -283,6 +341,9 @@ BTF_SET8_START(bpf_testmod_common_kfunc_ids) BTF_ID_FLAGS(func, bpf_iter_testmod_seq_new, KF_ITER_NEW) BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL) BTF_ID_FLAGS(func, bpf_iter_testmod_seq_destroy, KF_ITER_DESTROY) +BTF_ID_FLAGS(func, bpf__unsafe_spin_lock, KF_DESTRUCTIVE) +BTF_ID_FLAGS(func, bpf__unsafe_spin_unlock, KF_DESTRUCTIVE) +BTF_ID_FLAGS(func, bpf_refcount_read, KF_DESTRUCTIVE) BTF_SET8_END(bpf_testmod_common_kfunc_ids) static const struct btf_kfunc_id_set bpf_testmod_common_kfunc_set = { From patchwork Fri Jun 2 02:26:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13264676 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0F3281855 for ; Fri, 2 Jun 2023 02:27:16 +0000 (UTC) Received: from mx0a-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1807A18D for ; Thu, 1 Jun 2023 19:27:14 -0700 (PDT) Received: from pps.filterd (m0089730.ppops.net [127.0.0.1]) by m0089730.ppops.net (8.17.1.19/8.17.1.19) with ESMTP id 351NtW3h025378 for ; Thu, 1 Jun 2023 19:27:13 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : content-type : content-transfer-encoding : mime-version; s=facebook; bh=sMkWbAkq5YkcH2ZwIaxp411OtvXYjEWVas3ZsCgZ9uk=; b=Yfo+rjO1rimgLh6YWrmHoRCPRKDxJmLGuyakPftrcuJhndtM9tH/sf7rrY3A4ut0iYWS oNcEZPTbi1tp5F9dd/bV7dA3v81qK+CRe+MV+BZ+6a+FRtsSa231kPrHILyFeT3EElVR et/R9BeGYUoLuZBkfjd6zVGSxcL+Q53o8sI= Received: from mail.thefacebook.com ([163.114.132.120]) by m0089730.ppops.net (PPS) with ESMTPS id 3qxhr11bpm-3 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 01 Jun 2023 19:27:13 -0700 Received: from twshared16624.09.ash9.facebook.com (2620:10d:c085:208::11) by mail.thefacebook.com (2620:10d:c085:11d::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 1 Jun 2023 19:27:11 -0700 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id EF6781EF7C8E6; Thu, 1 Jun 2023 19:26:55 -0700 (PDT) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kernel Team , Dave Marchevsky Subject: [PATCH v2 bpf-next 7/9] [DONOTAPPLY] selftests/bpf: Add test exercising bpf_refcount_acquire race condition Date: Thu, 1 Jun 2023 19:26:45 -0700 Message-ID: <20230602022647.1571784-8-davemarchevsky@fb.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230602022647.1571784-1-davemarchevsky@fb.com> References: <20230602022647.1571784-1-davemarchevsky@fb.com> X-FB-Internal: Safe X-Proofpoint-GUID: qKfR0RjsK7BLWmqpsycC6fJlcmviZGGE X-Proofpoint-ORIG-GUID: qKfR0RjsK7BLWmqpsycC6fJlcmviZGGE X-Proofpoint-UnRewURL: 0 URL was un-rewritten Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net The selftest added in this patch is the exact scenario described by Kumar in [0] and fixed by earlier patches in this series. The long comment added in progs/refcounted_kptr.c restates the use-after-free scenario. The added test uses bpf__unsafe_spin_{lock, unlock} to force the specific problematic interleaving we're interested in testing, and bpf_refcount_read to confirm refcount incr/decr work as expected. [0]: https://lore.kernel.org/bpf/atfviesiidev4hu53hzravmtlau3wdodm2vqs7rd7tnwft34e3@xktodqeqevir/ Signed-off-by: Dave Marchevsky --- .../bpf/prog_tests/refcounted_kptr.c | 104 +++++++++++- .../selftests/bpf/progs/refcounted_kptr.c | 158 ++++++++++++++++++ 2 files changed, 261 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c index 2ab23832062d..e7fcc1dd8864 100644 --- a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c +++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c @@ -1,8 +1,10 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */ - +#define _GNU_SOURCE #include #include +#include +#include #include "refcounted_kptr.skel.h" #include "refcounted_kptr_fail.skel.h" @@ -16,3 +18,103 @@ void test_refcounted_kptr_fail(void) { RUN_TESTS(refcounted_kptr_fail); } + +static void force_cpu(pthread_t thread, int cpunum) +{ + cpu_set_t cpuset; + int err; + + CPU_ZERO(&cpuset); + CPU_SET(cpunum, &cpuset); + err = pthread_setaffinity_np(thread, sizeof(cpuset), &cpuset); + if (!ASSERT_OK(err, "pthread_setaffinity_np")) + return; +} + +struct refcounted_kptr *skel; + +static void *run_unstash_acq_ref(void *unused) +{ + LIBBPF_OPTS(bpf_test_run_opts, opts, + .data_in = &pkt_v4, + .data_size_in = sizeof(pkt_v4), + .repeat = 1, + ); + long ret, unstash_acq_ref_fd; + force_cpu(pthread_self(), 1); + + unstash_acq_ref_fd = bpf_program__fd(skel->progs.unstash_add_and_acquire_refcount); + + ret = bpf_prog_test_run_opts(unstash_acq_ref_fd, &opts); + ASSERT_EQ(opts.retval, 0, "unstash_add_and_acquire_refcount retval"); + ASSERT_EQ(skel->bss->ref_check_3, 2, "ref_check_3"); + ASSERT_EQ(skel->bss->ref_check_4, 1, "ref_check_4"); + ASSERT_EQ(skel->bss->ref_check_5, 0, "ref_check_5"); + pthread_exit((void *)ret); +} + +void test_refcounted_kptr_races(void) +{ + LIBBPF_OPTS(bpf_test_run_opts, opts, + .data_in = &pkt_v4, + .data_size_in = sizeof(pkt_v4), + .repeat = 1, + ); + int ref_acq_lock_fd, ref_acq_unlock_fd, rem_node_lock_fd; + int add_stash_fd, remove_tree_fd; + pthread_t thread_id; + int ret; + + force_cpu(pthread_self(), 0); + skel = refcounted_kptr__open_and_load(); + if (!ASSERT_OK_PTR(skel, "refcounted_kptr__open_and_load")) + return; + + add_stash_fd = bpf_program__fd(skel->progs.add_refcounted_node_to_tree_and_stash); + remove_tree_fd = bpf_program__fd(skel->progs.remove_refcounted_node_from_tree); + ref_acq_lock_fd = bpf_program__fd(skel->progs.unsafe_ref_acq_lock); + ref_acq_unlock_fd = bpf_program__fd(skel->progs.unsafe_ref_acq_unlock); + rem_node_lock_fd = bpf_program__fd(skel->progs.unsafe_rem_node_lock); + + ret = bpf_prog_test_run_opts(rem_node_lock_fd, &opts); + if (!ASSERT_OK(ret, "rem_node_lock")) + return; + + ret = bpf_prog_test_run_opts(ref_acq_lock_fd, &opts); + if (!ASSERT_OK(ret, "ref_acq_lock")) + return; + + ret = bpf_prog_test_run_opts(add_stash_fd, &opts); + if (!ASSERT_OK(ret, "add_stash")) + return; + if (!ASSERT_OK(opts.retval, "add_stash retval")) + return; + + ret = pthread_create(&thread_id, NULL, &run_unstash_acq_ref, NULL); + if (!ASSERT_OK(ret, "pthread_create")) + goto cleanup; + + force_cpu(thread_id, 1); + + /* This program will execute before unstash_acq_ref's refcount_acquire, then + * unstash_acq_ref can proceed after unsafe_unlock + */ + ret = bpf_prog_test_run_opts(remove_tree_fd, &opts); + if (!ASSERT_OK(ret, "remove_tree")) + goto cleanup; + + ret = bpf_prog_test_run_opts(ref_acq_unlock_fd, &opts); + if (!ASSERT_OK(ret, "ref_acq_unlock")) + goto cleanup; + + ret = pthread_join(thread_id, NULL); + if (!ASSERT_OK(ret, "pthread_join")) + goto cleanup; + + refcounted_kptr__destroy(skel); + return; +cleanup: + bpf_prog_test_run_opts(ref_acq_unlock_fd, &opts); + refcounted_kptr__destroy(skel); + return; +} diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c index a3da610b1e6b..2951f45291c1 100644 --- a/tools/testing/selftests/bpf/progs/refcounted_kptr.c +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c @@ -39,9 +39,20 @@ private(A) struct bpf_spin_lock lock; private(A) struct bpf_rb_root root __contains(node_data, r); private(A) struct bpf_list_head head __contains(node_data, l); +private(C) struct bpf_spin_lock lock2; +private(C) struct bpf_rb_root root2 __contains(node_data, r); + private(B) struct bpf_spin_lock alock; private(B) struct bpf_rb_root aroot __contains(node_acquire, node); +private(D) struct bpf_spin_lock ref_acq_lock; +private(E) struct bpf_spin_lock rem_node_lock; + +/* Provided by bpf_testmod */ +extern void bpf__unsafe_spin_lock(void *lock__ign) __ksym; +extern void bpf__unsafe_spin_unlock(void *lock__ign) __ksym; +extern volatile int bpf_refcount_read(void *refcount__ign) __ksym; + static bool less(struct bpf_rb_node *node_a, const struct bpf_rb_node *node_b) { struct node_data *a; @@ -405,4 +416,151 @@ long rbtree_refcounted_node_ref_escapes_owning_input(void *ctx) return 0; } +SEC("tc") +long unsafe_ref_acq_lock(void *ctx) +{ + bpf__unsafe_spin_lock(&ref_acq_lock); + return 0; +} + +SEC("tc") +long unsafe_ref_acq_unlock(void *ctx) +{ + bpf__unsafe_spin_unlock(&ref_acq_lock); + return 0; +} + +SEC("tc") +long unsafe_rem_node_lock(void *ctx) +{ + bpf__unsafe_spin_lock(&rem_node_lock); + return 0; +} + +/* The following 3 progs are used in concert to test a bpf_refcount-related + * race. Consider the following pseudocode interleaving of rbtree operations: + * + * (Assumptions: n, m, o, p, q are pointers to nodes, t1 and t2 are different + * rbtrees, l1 and l2 are locks accompanying the trees, mapval is some + * kptr_xchg'able ptr_to_map_value. A single node is being manipulated by both + * programs. Irrelevant error-checking and casting is omitted.) + * + * CPU O CPU 1 + * ----------------------------------|--------------------------- + * n = bpf_obj_new [0] | + * lock(l1) | + * bpf_rbtree_add(t1, &n->r, less) | + * m = bpf_refcount_acquire(n) [1] | + * unlock(l1) | + * kptr_xchg(mapval, m) [2] | + * -------------------------------------------------------------- + * | o = kptr_xchg(mapval, NULL) [3] + * | lock(l2) + * | rbtree_add(t2, &o->r, less) [4] + * -------------------------------------------------------------- + * lock(l1) | + * p = rbtree_first(t1) | + * p = rbtree_remove(t1, p) | + * unlock(l1) | + * if (p) | + * bpf_obj_drop(p) [5] | + * -------------------------------------------------------------- + * | q = bpf_refcount_acquire(o) [6] + * | unlock(l2) + * + * If bpf_refcount_acquire can't fail, the sequence of operations on the node's + * refcount is: + * [0] - refcount initialized to 1 + * [1] - refcount bumped to 2 + * [2] - refcount is still 2, but m's ownership passed to mapval + * [3] - refcount is still 2, mapval's ownership passed to o + * [4] - refcount is decr'd to 1, rbtree_add fails, node is already in t1 + * o is converted to non-owning reference + * [5] - refcount is decr'd to 0, node free'd + * [6] - refcount is incr'd to 1 from 0, ERROR + * + * To prevent [6] bpf_refcount_acquire was made failable. This interleaving is + * used to test failable refcount_acquire. + * + * The two halves of CPU 0's operations are implemented by + * add_refcounted_node_to_tree_and_stash and remove_refcounted_node_from_tree. + * We can't do the same for CPU 1's operations due to l2 critical section. + * Instead, bpf__unsafe_spin_{lock, unlock} are used to ensure the expected + * order of operations. + */ + +SEC("tc") +long add_refcounted_node_to_tree_and_stash(void *ctx) +{ + long err; + + err = __stash_map_insert_tree(0, 42, &root, &lock); + if (err) + return err; + + return 0; +} + +SEC("tc") +long remove_refcounted_node_from_tree(void *ctx) +{ + long ret = 0; + + /* rem_node_lock is held by another program to force race */ + bpf__unsafe_spin_lock(&rem_node_lock); + ret = __read_from_tree(&root, &lock, true); + if (ret != 42) + return ret; + + bpf__unsafe_spin_unlock(&rem_node_lock); + return 0; +} + +/* ref_check_n numbers correspond to refcount operation points in comment above */ +int ref_check_3, ref_check_4, ref_check_5; + +SEC("tc") +long unstash_add_and_acquire_refcount(void *ctx) +{ + struct map_value *mapval; + struct node_data *n, *m; + int idx = 0; + + mapval = bpf_map_lookup_elem(&stashed_nodes, &idx); + if (!mapval) + return -1; + + n = bpf_kptr_xchg(&mapval->node, NULL); + if (!n) + return -2; + ref_check_3 = bpf_refcount_read(&n->ref); + + bpf_spin_lock(&lock2); + bpf_rbtree_add(&root2, &n->r, less); + ref_check_4 = bpf_refcount_read(&n->ref); + + /* Let CPU 0 do first->remove->drop */ + bpf__unsafe_spin_unlock(&rem_node_lock); + + /* ref_acq_lock is held by another program to force race + * when this program holds the lock, remove_refcounted_node_from_tree + * has finished + */ + bpf__unsafe_spin_lock(&ref_acq_lock); + ref_check_5 = bpf_refcount_read(&n->ref); + + /* Error-causing use-after-free incr ([6] in long comment above) */ + m = bpf_refcount_acquire(n); + bpf__unsafe_spin_unlock(&ref_acq_lock); + + bpf_spin_unlock(&lock2); + + if (m) { + bpf_obj_drop(m); + return -3; + } + + return !!m; +} + char _license[] SEC("license") = "GPL"; From patchwork Fri Jun 2 02:26:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13264675 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 98F5817FA for ; Fri, 2 Jun 2023 02:27:13 +0000 (UTC) Received: from mx0b-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D01DA192 for ; Thu, 1 Jun 2023 19:27:11 -0700 (PDT) Received: from pps.filterd (m0148460.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 351NtcJI019682 for ; Thu, 1 Jun 2023 19:27:11 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=facebook; bh=XWrSlb+AG1qaHY7bnOaCSLx8bqGpMVGegcEPyDdpsRg=; b=a7QifC8aYykTy9yNFQrpH9NI8M2NtC/fuIUVTK1RoaJVqy34/iyALIFtV4I/7hcfLWW4 gfFGPM3rrdz8KLaych1QT/8h1QnYg2zMGoQMBrKM7YoJYbOlJj2hQSNe4ThllteZNm8n wcyMPNn8NdFg1pG0Gt/CKbJZZ6w1ZqE4XeI= Received: from maileast.thefacebook.com ([163.114.130.16]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3qxt1s6cwm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 01 Jun 2023 19:27:10 -0700 Received: from twshared35445.38.frc1.facebook.com (2620:10d:c0a8:1c::1b) by mail.thefacebook.com (2620:10d:c0a8:82::d) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 1 Jun 2023 19:27:10 -0700 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id 6D3001EF7C8EA; Thu, 1 Jun 2023 19:26:56 -0700 (PDT) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kernel Team , Dave Marchevsky Subject: [PATCH v2 bpf-next 8/9] [DONOTAPPLY] selftests/bpf: Disable newly-added refcounted_kptr_races test Date: Thu, 1 Jun 2023 19:26:46 -0700 Message-ID: <20230602022647.1571784-9-davemarchevsky@fb.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230602022647.1571784-1-davemarchevsky@fb.com> References: <20230602022647.1571784-1-davemarchevsky@fb.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: b6t6AMa5NFZkjCmnb6dCcsfb3Fuaw8_m X-Proofpoint-GUID: b6t6AMa5NFZkjCmnb6dCcsfb3Fuaw8_m X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net The previous patch added a new test exercising a race condition which was fixed earlier in the series. Similarly to other tests in this file, the new test should not run while bpf_refcount_acquire is disabled as it requires that kfunc. Signed-off-by: Dave Marchevsky --- .../bpf/prog_tests/refcounted_kptr.c | 100 ------------------ 1 file changed, 100 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c index e7fcc1dd8864..6a53f304f3e4 100644 --- a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c +++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c @@ -18,103 +18,3 @@ void test_refcounted_kptr_fail(void) { RUN_TESTS(refcounted_kptr_fail); } - -static void force_cpu(pthread_t thread, int cpunum) -{ - cpu_set_t cpuset; - int err; - - CPU_ZERO(&cpuset); - CPU_SET(cpunum, &cpuset); - err = pthread_setaffinity_np(thread, sizeof(cpuset), &cpuset); - if (!ASSERT_OK(err, "pthread_setaffinity_np")) - return; -} - -struct refcounted_kptr *skel; - -static void *run_unstash_acq_ref(void *unused) -{ - LIBBPF_OPTS(bpf_test_run_opts, opts, - .data_in = &pkt_v4, - .data_size_in = sizeof(pkt_v4), - .repeat = 1, - ); - long ret, unstash_acq_ref_fd; - force_cpu(pthread_self(), 1); - - unstash_acq_ref_fd = bpf_program__fd(skel->progs.unstash_add_and_acquire_refcount); - - ret = bpf_prog_test_run_opts(unstash_acq_ref_fd, &opts); - ASSERT_EQ(opts.retval, 0, "unstash_add_and_acquire_refcount retval"); - ASSERT_EQ(skel->bss->ref_check_3, 2, "ref_check_3"); - ASSERT_EQ(skel->bss->ref_check_4, 1, "ref_check_4"); - ASSERT_EQ(skel->bss->ref_check_5, 0, "ref_check_5"); - pthread_exit((void *)ret); -} - -void test_refcounted_kptr_races(void) -{ - LIBBPF_OPTS(bpf_test_run_opts, opts, - .data_in = &pkt_v4, - .data_size_in = sizeof(pkt_v4), - .repeat = 1, - ); - int ref_acq_lock_fd, ref_acq_unlock_fd, rem_node_lock_fd; - int add_stash_fd, remove_tree_fd; - pthread_t thread_id; - int ret; - - force_cpu(pthread_self(), 0); - skel = refcounted_kptr__open_and_load(); - if (!ASSERT_OK_PTR(skel, "refcounted_kptr__open_and_load")) - return; - - add_stash_fd = bpf_program__fd(skel->progs.add_refcounted_node_to_tree_and_stash); - remove_tree_fd = bpf_program__fd(skel->progs.remove_refcounted_node_from_tree); - ref_acq_lock_fd = bpf_program__fd(skel->progs.unsafe_ref_acq_lock); - ref_acq_unlock_fd = bpf_program__fd(skel->progs.unsafe_ref_acq_unlock); - rem_node_lock_fd = bpf_program__fd(skel->progs.unsafe_rem_node_lock); - - ret = bpf_prog_test_run_opts(rem_node_lock_fd, &opts); - if (!ASSERT_OK(ret, "rem_node_lock")) - return; - - ret = bpf_prog_test_run_opts(ref_acq_lock_fd, &opts); - if (!ASSERT_OK(ret, "ref_acq_lock")) - return; - - ret = bpf_prog_test_run_opts(add_stash_fd, &opts); - if (!ASSERT_OK(ret, "add_stash")) - return; - if (!ASSERT_OK(opts.retval, "add_stash retval")) - return; - - ret = pthread_create(&thread_id, NULL, &run_unstash_acq_ref, NULL); - if (!ASSERT_OK(ret, "pthread_create")) - goto cleanup; - - force_cpu(thread_id, 1); - - /* This program will execute before unstash_acq_ref's refcount_acquire, then - * unstash_acq_ref can proceed after unsafe_unlock - */ - ret = bpf_prog_test_run_opts(remove_tree_fd, &opts); - if (!ASSERT_OK(ret, "remove_tree")) - goto cleanup; - - ret = bpf_prog_test_run_opts(ref_acq_unlock_fd, &opts); - if (!ASSERT_OK(ret, "ref_acq_unlock")) - goto cleanup; - - ret = pthread_join(thread_id, NULL); - if (!ASSERT_OK(ret, "pthread_join")) - goto cleanup; - - refcounted_kptr__destroy(skel); - return; -cleanup: - bpf_prog_test_run_opts(ref_acq_unlock_fd, &opts); - refcounted_kptr__destroy(skel); - return; -} From patchwork Fri Jun 2 02:26:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13264672 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 42DE815A3 for ; Fri, 2 Jun 2023 02:27:11 +0000 (UTC) Received: from mx0a-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D6C03192 for ; Thu, 1 Jun 2023 19:27:09 -0700 (PDT) Received: from pps.filterd (m0001303.ppops.net [127.0.0.1]) by m0001303.ppops.net (8.17.1.19/8.17.1.19) with ESMTP id 351Nt5kx020789 for ; Thu, 1 Jun 2023 19:27:09 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=facebook; bh=MfdQDA9uhS6eEkZuafW+fI3gyC3ZMjiWPTRJzziSk1E=; b=nYjEl2nOY00G5yWQHBg0YZBp11gSOT3HzsLxsmZv/NYBANHQjBxwaJUJ5Xb80US5DeM2 3KvZTs7jI4hcpwnfiOj7cdYWcX3GEolCSE4GwrhToCBCPLB2nNUBTWHL7Pu69F6um/ne PLNCiNTJ02/r5gejqeQMaJGQpJtoCECvka4= Received: from mail.thefacebook.com ([163.114.132.120]) by m0001303.ppops.net (PPS) with ESMTPS id 3qxawf46ye-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 01 Jun 2023 19:27:08 -0700 Received: from twshared8528.02.ash9.facebook.com (2620:10d:c085:108::8) by mail.thefacebook.com (2620:10d:c085:21d::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 1 Jun 2023 19:27:07 -0700 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id 0B8561EF7C8F9; Thu, 1 Jun 2023 19:26:57 -0700 (PDT) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kernel Team , Dave Marchevsky Subject: [PATCH v2 bpf-next 9/9] [DONOTAPPLY] Revert "selftests/bpf: Disable newly-added refcounted_kptr_races test" Date: Thu, 1 Jun 2023 19:26:47 -0700 Message-ID: <20230602022647.1571784-10-davemarchevsky@fb.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230602022647.1571784-1-davemarchevsky@fb.com> References: <20230602022647.1571784-1-davemarchevsky@fb.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: zsKpKc_I5JoUliv9UHykg7KxBaRcMRrg X-Proofpoint-GUID: zsKpKc_I5JoUliv9UHykg7KxBaRcMRrg X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net This patch reverts the previous patch's disabling of refcounted_kptr_races selftest. It is included with the series so that BPF CI will be able to run the test. This patch should not be applied - followups which fix remaining bpf_refcount issues will re-enable this test. Signed-off-by: Dave Marchevsky --- .../bpf/prog_tests/refcounted_kptr.c | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c index 6a53f304f3e4..e7fcc1dd8864 100644 --- a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c +++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr.c @@ -18,3 +18,103 @@ void test_refcounted_kptr_fail(void) { RUN_TESTS(refcounted_kptr_fail); } + +static void force_cpu(pthread_t thread, int cpunum) +{ + cpu_set_t cpuset; + int err; + + CPU_ZERO(&cpuset); + CPU_SET(cpunum, &cpuset); + err = pthread_setaffinity_np(thread, sizeof(cpuset), &cpuset); + if (!ASSERT_OK(err, "pthread_setaffinity_np")) + return; +} + +struct refcounted_kptr *skel; + +static void *run_unstash_acq_ref(void *unused) +{ + LIBBPF_OPTS(bpf_test_run_opts, opts, + .data_in = &pkt_v4, + .data_size_in = sizeof(pkt_v4), + .repeat = 1, + ); + long ret, unstash_acq_ref_fd; + force_cpu(pthread_self(), 1); + + unstash_acq_ref_fd = bpf_program__fd(skel->progs.unstash_add_and_acquire_refcount); + + ret = bpf_prog_test_run_opts(unstash_acq_ref_fd, &opts); + ASSERT_EQ(opts.retval, 0, "unstash_add_and_acquire_refcount retval"); + ASSERT_EQ(skel->bss->ref_check_3, 2, "ref_check_3"); + ASSERT_EQ(skel->bss->ref_check_4, 1, "ref_check_4"); + ASSERT_EQ(skel->bss->ref_check_5, 0, "ref_check_5"); + pthread_exit((void *)ret); +} + +void test_refcounted_kptr_races(void) +{ + LIBBPF_OPTS(bpf_test_run_opts, opts, + .data_in = &pkt_v4, + .data_size_in = sizeof(pkt_v4), + .repeat = 1, + ); + int ref_acq_lock_fd, ref_acq_unlock_fd, rem_node_lock_fd; + int add_stash_fd, remove_tree_fd; + pthread_t thread_id; + int ret; + + force_cpu(pthread_self(), 0); + skel = refcounted_kptr__open_and_load(); + if (!ASSERT_OK_PTR(skel, "refcounted_kptr__open_and_load")) + return; + + add_stash_fd = bpf_program__fd(skel->progs.add_refcounted_node_to_tree_and_stash); + remove_tree_fd = bpf_program__fd(skel->progs.remove_refcounted_node_from_tree); + ref_acq_lock_fd = bpf_program__fd(skel->progs.unsafe_ref_acq_lock); + ref_acq_unlock_fd = bpf_program__fd(skel->progs.unsafe_ref_acq_unlock); + rem_node_lock_fd = bpf_program__fd(skel->progs.unsafe_rem_node_lock); + + ret = bpf_prog_test_run_opts(rem_node_lock_fd, &opts); + if (!ASSERT_OK(ret, "rem_node_lock")) + return; + + ret = bpf_prog_test_run_opts(ref_acq_lock_fd, &opts); + if (!ASSERT_OK(ret, "ref_acq_lock")) + return; + + ret = bpf_prog_test_run_opts(add_stash_fd, &opts); + if (!ASSERT_OK(ret, "add_stash")) + return; + if (!ASSERT_OK(opts.retval, "add_stash retval")) + return; + + ret = pthread_create(&thread_id, NULL, &run_unstash_acq_ref, NULL); + if (!ASSERT_OK(ret, "pthread_create")) + goto cleanup; + + force_cpu(thread_id, 1); + + /* This program will execute before unstash_acq_ref's refcount_acquire, then + * unstash_acq_ref can proceed after unsafe_unlock + */ + ret = bpf_prog_test_run_opts(remove_tree_fd, &opts); + if (!ASSERT_OK(ret, "remove_tree")) + goto cleanup; + + ret = bpf_prog_test_run_opts(ref_acq_unlock_fd, &opts); + if (!ASSERT_OK(ret, "ref_acq_unlock")) + goto cleanup; + + ret = pthread_join(thread_id, NULL); + if (!ASSERT_OK(ret, "pthread_join")) + goto cleanup; + + refcounted_kptr__destroy(skel); + return; +cleanup: + bpf_prog_test_run_opts(ref_acq_unlock_fd, &opts); + refcounted_kptr__destroy(skel); + return; +}