From patchwork Thu Feb 9 17:41:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Marchevsky X-Patchwork-Id: 13134897 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2218CC61DA4 for ; Thu, 9 Feb 2023 17:42:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229571AbjBIRmH (ORCPT ); Thu, 9 Feb 2023 12:42:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33484 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229538AbjBIRmG (ORCPT ); Thu, 9 Feb 2023 12:42:06 -0500 Received: from mx0b-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AD3843432F for ; Thu, 9 Feb 2023 09:42:02 -0800 (PST) Received: from pps.filterd (m0109332.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 319H5slX012948 for ; Thu, 9 Feb 2023 09:42:01 -0800 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=cVA+vMf6boodN1emFjsFImlj8ZxVm6PQ6jyAjr8DqxA=; b=AZakvsc2b3AS5qj7ARHz1RtXet9aBQGjWYFSn/qoTY6CChW+3G0BpSSLhwL5nu73lqbS lOEMXzX1WCbMZ+/6hWNc1EYLDyBqogLcxwnhy6DeFJTV/Y0Ka9tD7dsqrDZsNDrVZLyF EXtpeZnUKRsoRrZX91vvvgcbmaEN0O7l/98= Received: from maileast.thefacebook.com ([163.114.130.16]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3nn1bxj6yh-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 09 Feb 2023 09:42:01 -0800 Received: from twshared25383.14.frc2.facebook.com (2620:10d:c0a8:1b::d) 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.6; Thu, 9 Feb 2023 09:42:00 -0800 Received: by devbig077.ldc1.facebook.com (Postfix, from userid 158236) id F0A2B16905FE6; Thu, 9 Feb 2023 09:41:48 -0800 (PST) From: Dave Marchevsky To: CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Kernel Team , Kumar Kartikeya Dwivedi , Tejun Heo , Dave Marchevsky Subject: [PATCH v4 bpf-next 02/11] bpf: Improve bpf_reg_state space usage for non-owning ref lock Date: Thu, 9 Feb 2023 09:41:35 -0800 Message-ID: <20230209174144.3280955-3-davemarchevsky@fb.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230209174144.3280955-1-davemarchevsky@fb.com> References: <20230209174144.3280955-1-davemarchevsky@fb.com> MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: Qx3IEbjnhc1y-QrZVKcsA5LbDGAbsG6W X-Proofpoint-GUID: Qx3IEbjnhc1y-QrZVKcsA5LbDGAbsG6W X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.170.22 definitions=2023-02-09_13,2023-02-09_03,2023-02-09_01 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net This patch eliminates extra bpf_reg_state memory usage added due to previous patch keeping a copy of lock identity in reg state for non-owning refs. Instead of copying lock identity around, this patch changes non_owning_ref_lock field to be a bool, taking advantage of the following: * There can currently only be one active lock at a time * non-owning refs are only valid in the critical section So if a verifier_state has an active_lock, any non-owning ref must've been obtained under that lock, and any non-owning ref not obtained under that lock must have been invalidated previously. Therefore if a non-owning ref is associated with a lock, it's the active_lock of the current state. So we can keep a bool "are we associated with active_lock of current state" instead of copying lock identity around. Signed-off-by: Dave Marchevsky --- include/linux/bpf_verifier.h | 2 +- kernel/bpf/verifier.c | 25 ++++++++++--------------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 7b5fbb66446c..d25446dd0413 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -84,7 +84,7 @@ struct bpf_reg_state { struct { struct btf *btf; u32 btf_id; - struct bpf_active_lock non_owning_ref_lock; + bool non_owning_ref_lock; }; struct { /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */ diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index f693cc97c574..89c09752421c 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -190,8 +190,7 @@ struct bpf_verifier_stack_elem { static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx); static int release_reference(struct bpf_verifier_env *env, int ref_obj_id); -static void invalidate_non_owning_refs(struct bpf_verifier_env *env, - struct bpf_active_lock *lock); +static void invalidate_non_owning_refs(struct bpf_verifier_env *env); static int ref_set_non_owning_lock(struct bpf_verifier_env *env, struct bpf_reg_state *reg); @@ -1077,9 +1076,8 @@ static void print_verifier_state(struct bpf_verifier_env *env, verbose_a("id=%d", reg->id); if (reg->ref_obj_id) verbose_a("ref_obj_id=%d", reg->ref_obj_id); - if (reg->non_owning_ref_lock.ptr) - verbose_a("non_own_id=(%p,%d)", reg->non_owning_ref_lock.ptr, - reg->non_owning_ref_lock.id); + if (reg->non_owning_ref_lock) + verbose_a("%s", "non_own_ref"); if (t != SCALAR_VALUE) verbose_a("off=%d", reg->off); if (type_is_pkt_pointer(t)) @@ -5049,7 +5047,7 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, } if (type_is_alloc(reg->type) && !reg->ref_obj_id && - !reg->non_owning_ref_lock.ptr) { + !reg->non_owning_ref_lock) { verbose(env, "verifier internal error: ref_obj_id for allocated object must be non-zero\n"); return -EFAULT; } @@ -6056,7 +6054,7 @@ static int process_spin_lock(struct bpf_verifier_env *env, int regno, return -EINVAL; } - invalidate_non_owning_refs(env, &cur->active_lock); + invalidate_non_owning_refs(env); cur->active_lock.ptr = NULL; cur->active_lock.id = 0; @@ -7373,16 +7371,14 @@ static int release_reference(struct bpf_verifier_env *env, return 0; } -static void invalidate_non_owning_refs(struct bpf_verifier_env *env, - struct bpf_active_lock *lock) +static void invalidate_non_owning_refs(struct bpf_verifier_env *env) { struct bpf_func_state *unused; struct bpf_reg_state *reg; bpf_for_each_reg_in_vstate(env->cur_state, unused, reg, ({ - if (reg->non_owning_ref_lock.ptr && - reg->non_owning_ref_lock.ptr == lock->ptr && - reg->non_owning_ref_lock.id == lock->id) + if (type_is_ptr_alloc_obj(reg->type) && + reg->non_owning_ref_lock) __mark_reg_unknown(env, reg); })); } @@ -8948,13 +8944,12 @@ static int ref_set_non_owning_lock(struct bpf_verifier_env *env, struct bpf_reg_ return -EFAULT; } - if (reg->non_owning_ref_lock.ptr) { + if (reg->non_owning_ref_lock) { verbose(env, "verifier internal error: non_owning_ref_lock already set\n"); return -EFAULT; } - reg->non_owning_ref_lock.id = state->active_lock.id; - reg->non_owning_ref_lock.ptr = state->active_lock.ptr; + reg->non_owning_ref_lock = true; return 0; }