From patchwork Wed Dec 4 03:03:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13893167 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 371C93A1B5 for ; Wed, 4 Dec 2024 03:04:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.65 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281451; cv=none; b=Qd+QD87DWFU9OiIAfs7oL6EjzpzT8647jw2FiNYxXWdKRvWcq3ILMRJCJkSDj5UACGCRw/xQ8P+FSyQW9NTbAUozZ9HyITa8et6PnC9nfPNd/YkVm4C7iNX0S0mrKM4W7Tvok+U9hBegphH8rWQ3QT9m1vCwgY4mKblv5FQ85GE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281451; c=relaxed/simple; bh=iEVu8kmSZyLeJeiEoglbCCB6IHu0xSeC3JqgT9TdIHw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=M0WjYbgXNoI514Lwvr4dm/rLu3mWjI3gTEqNtukCpnUiJJlfj3VBQtj9j0DRVCx/05oWDVf9nRUiZGoW1ApK3AXG/cYUkt5yxW8KsN3Cfb+Qgks14z6r0vuDeljRh+D9xWJfRRBBkGIV90ybIE1jB5dSnx8lGllV9u+yZBf6LgU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ASAQLwGk; arc=none smtp.client-ip=209.85.221.65 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ASAQLwGk" Received: by mail-wr1-f65.google.com with SMTP id ffacd0b85a97d-385ddcfc97bso4271050f8f.1 for ; Tue, 03 Dec 2024 19:04:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1733281447; x=1733886247; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=60lXYy4/0XzCplNgdhpSsswOi6BXdkBKKPAUudO2oJo=; b=ASAQLwGk3nOlhFFCPbULwdl4urbaU4KJSts+cp9BDlwalWpBKRPQ/Ulu2kV6tA9kok 0ZXCWXHyXZDx5NCrg2DCT0Pjyl2RMugC073BXvkP8/0cIuAqHvjzq7l1lrXAaIHp88nW xDckwT+Nd0izSy0sbIDPisoPSR75tUVUj17bGkO7UmpsllKfxaqoyI25oaB6/u6vMwZL HOo1GBFQpyns+DLxaBZ264xGduz4EcrzYT7dFw77FBMLD9oldcBewH/IliuH2/z8oSYR GBzw4fNIk+tCnvsqupgwWCCchZkIxa/QiOuFxGfVxDD6TnTasWc00YbDG049r/7XcpjK sJeg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733281447; x=1733886247; 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=60lXYy4/0XzCplNgdhpSsswOi6BXdkBKKPAUudO2oJo=; b=Ai183GknSxPrsd+XFDLCAWi1V2WQ4kX+r4UqfCCRLNX5AJp3u6W+qTatk5kGYbTJwl JaUGhATmSB5V1md0BHbQmnkxFqP6Ma9s6x0X2BPIpUOWcUjqk6cN+CV9oqB71V5CSCA2 pP6S+U6GlO9vpclnlOlCaJdKxh5GhaKKLSBromw3T+CrSsTRHKnaBsrJ3b7oNL5Klx/E 51NHgywbv4zUr7REYOSyjhD2V5yDen0/bGgz1BBj9a1B9SFmH3dm9QUcfZm0Cy8pANT9 StnZsNh0L7peROqmKDY0AlECDYYkN/YjkeDhSXXO8U8Lb07hV/cP9G0MgPnvGEIXry47 Zsvg== X-Gm-Message-State: AOJu0YxyEzqA+uRjD78sT93ous9d71yx8CDqsxcTz/riPXeOBr3phKqS tLAOTJ07/jnPwhtYSebLGJq+GV8qxv2vDlS9FjW6mUI4Vb5Q1pxBLm1Va/nsZGA= X-Gm-Gg: ASbGncvCQXwKePdt6rd3loP5jE4i2Gf0ghRqL5CozzRaWBWq/tDagF5foAH+n9ylBkY sjxPsjQheVO+7EazmXUrUHE/Wk2sW14Rh6RczIs4aS917OV9cB+XyiYP9uySxDvs6Ti/nN0MD9A 9yr5alxeZP6Pq/0jnSuPJxxyiHts1pbZte0Ml3/OF2aeTNE342hgXApRXZHOhzGs5qEcPdujYz2 mBokJ1zfp3j7Yd2zE/kEuqc73Mf1tVlv5dbLIeem8CR7zubziSJAbqJ5itpRhPWpjIqWG32L+eH sQ== X-Google-Smtp-Source: AGHT+IGkPQaFSC6hCMqunTGlUIxiaH5OqVdboDmSXHYgnq3/MwmaDdmg3xy6hUqNsgFjoWJ4yVLgWg== X-Received: by 2002:a5d:47a7:0:b0:382:4aa9:b620 with SMTP id ffacd0b85a97d-385fd3c6843mr3961208f8f.5.1733281446879; Tue, 03 Dec 2024 19:04:06 -0800 (PST) Received: from localhost (fwdproxy-cln-033.fbsv.net. [2a03:2880:31ff:21::face:b00c]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-385eaf1d099sm9878459f8f.61.2024.12.03.19.04.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 19:04:06 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Eduard Zingerman , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , kernel-team@fb.com Subject: [PATCH bpf-next v6 1/7] bpf: Consolidate locks and reference state in verifier state Date: Tue, 3 Dec 2024 19:03:54 -0800 Message-ID: <20241204030400.208005-2-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241204030400.208005-1-memxor@gmail.com> References: <20241204030400.208005-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=24904; h=from:subject; bh=iEVu8kmSZyLeJeiEoglbCCB6IHu0xSeC3JqgT9TdIHw=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnT8Q/LMwEt3JDzkWeqNT/RZVGqAqb5jy4Yn42Cxgh RnEl0FuJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ0/EPwAKCRBM4MiGSL8RygG6D/ 0V6vgaTsSN95rwrxdyAA+ZvlnKyvWoY9qDGio95tSFgUVqW3x3d9DOmMkhEshs5JC634gfGjqsmNpL mIIXSsxZmfNQqDHEHb7NX1goCt7qqmxgnAzPXWeaQjZRrt0nvOTpcGrFg8qiPFuE9VaBw54WD4YAib qOURxQRQTOgjl4fJYC4krSmBILWvOqS0LAidcI8wAdAfER/6tkDs2seJcOd2w1p2IdqnUHcq9VqmB/ XTN1UO1Kqawy2KZdH/CmAsXETcCUmkuGQLjaKhb2nLkF/hhdiYOMEwMSacUh9pErPfPVfnBMwwI843 71bNX6lum5k0vUt+W6fHWqlBajdMQWyL3Zb8GnMYMREUs5DzLwdJMPzKPbYhm4Y018HlS9eB2sfEcN OZNYS5J6buXfb68FJFp59Ah6f2bqQqigFxaNQIMkN60XA7Ow0O0aoLnHphjj5tU+xz5RtKzSIZNZju veBG3Tgqt6kEUu+U/m8nLWxpfKD6njCdWAYrqh5rhjfdKvFmsPDlpQqRYIMKeqx6apLmfJ72X0Z8Oh GMKQxP/WLsktkHPzvD6LtoBYVQeMP//yV16sxD9rTDpUcZt827UkW2e0Aig62uWp22GkQcjEfV6rht I+ilJM286qVG4wXiuqu6Sk/veRJeuAqgk+5TgXBGsE5YMUyfKKkt5q9RPw2A== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net Currently, state for RCU read locks and preemption is in bpf_verifier_state, while locks and pointer reference state remains in bpf_func_state. There is no particular reason to keep the latter in bpf_func_state. Additionally, it is copied into a new frame's state and copied back to the caller frame's state everytime the verifier processes a pseudo call instruction. This is a bit wasteful, given this state is global for a given verification state / path. Move all resource and reference related state in bpf_verifier_state structure in this patch, in preparation for introducing new reference state types in the future. Since we switch print_verifier_state and friends to print using vstate, we now need to explicitly pass in the verifier state from the caller along with the bpf_func_state, so modify the prototype and callers to do so. To ensure func state matches the verifier state when we're printing data, take in frame number instead of bpf_func_state pointer instead and avoid inconsistencies induced by the caller. Acked-by: Eduard Zingerman Signed-off-by: Kumar Kartikeya Dwivedi --- include/linux/bpf_verifier.h | 19 +++-- kernel/bpf/log.c | 20 ++--- kernel/bpf/verifier.c | 140 +++++++++++++++++------------------ 3 files changed, 88 insertions(+), 91 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index f4290c179bee..03e351c43fa8 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -315,9 +315,6 @@ struct bpf_func_state { u32 callback_depth; /* The following fields should be last. See copy_func_state() */ - int acquired_refs; - int active_locks; - struct bpf_reference_state *refs; /* The state of the stack. Each element of the array describes BPF_REG_SIZE * (i.e. 8) bytes worth of stack memory. * stack[0] represents bytes [*(r10-8)..*(r10-1)] @@ -370,6 +367,8 @@ struct bpf_verifier_state { /* call stack tracking */ struct bpf_func_state *frame[MAX_CALL_FRAMES]; struct bpf_verifier_state *parent; + /* Acquired reference states */ + struct bpf_reference_state *refs; /* * 'branches' field is the number of branches left to explore: * 0 - all possible paths from this state reached bpf_exit or @@ -419,9 +418,12 @@ struct bpf_verifier_state { u32 insn_idx; u32 curframe; - bool speculative; + u32 acquired_refs; + u32 active_locks; + u32 active_preempt_locks; bool active_rcu_lock; - u32 active_preempt_lock; + + bool speculative; /* If this state was ever pointed-to by other state's loop_entry field * this flag would be set to true. Used to avoid freeing such states * while they are still in use. @@ -979,8 +981,9 @@ const char *dynptr_type_str(enum bpf_dynptr_type type); const char *iter_type_str(const struct btf *btf, u32 btf_id); const char *iter_state_str(enum bpf_iter_state state); -void print_verifier_state(struct bpf_verifier_env *env, - const struct bpf_func_state *state, bool print_all); -void print_insn_state(struct bpf_verifier_env *env, const struct bpf_func_state *state); +void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_verifier_state *vstate, + u32 frameno, bool print_all); +void print_insn_state(struct bpf_verifier_env *env, const struct bpf_verifier_state *vstate, + u32 frameno); #endif /* _LINUX_BPF_VERIFIER_H */ diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c index 4a858fdb6476..2d28ce926053 100644 --- a/kernel/bpf/log.c +++ b/kernel/bpf/log.c @@ -753,9 +753,10 @@ static void print_reg_state(struct bpf_verifier_env *env, verbose(env, ")"); } -void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_func_state *state, - bool print_all) +void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_verifier_state *vstate, + u32 frameno, bool print_all) { + const struct bpf_func_state *state = vstate->frame[frameno]; const struct bpf_reg_state *reg; int i; @@ -843,11 +844,11 @@ void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_func_st break; } } - if (state->acquired_refs && state->refs[0].id) { - verbose(env, " refs=%d", state->refs[0].id); - for (i = 1; i < state->acquired_refs; i++) - if (state->refs[i].id) - verbose(env, ",%d", state->refs[i].id); + if (vstate->acquired_refs && vstate->refs[0].id) { + verbose(env, " refs=%d", vstate->refs[0].id); + for (i = 1; i < vstate->acquired_refs; i++) + if (vstate->refs[i].id) + verbose(env, ",%d", vstate->refs[i].id); } if (state->in_callback_fn) verbose(env, " cb"); @@ -864,7 +865,8 @@ static inline u32 vlog_alignment(u32 pos) BPF_LOG_MIN_ALIGNMENT) - pos - 1; } -void print_insn_state(struct bpf_verifier_env *env, const struct bpf_func_state *state) +void print_insn_state(struct bpf_verifier_env *env, const struct bpf_verifier_state *vstate, + u32 frameno) { if (env->prev_log_pos && env->prev_log_pos == env->log.end_pos) { /* remove new line character */ @@ -873,5 +875,5 @@ void print_insn_state(struct bpf_verifier_env *env, const struct bpf_func_state } else { verbose(env, "%d:", env->insn_idx); } - print_verifier_state(env, state, false); + print_verifier_state(env, vstate, frameno, false); } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 1c4ebb326785..019c56c782a2 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1279,15 +1279,17 @@ static void *realloc_array(void *arr, size_t old_n, size_t new_n, size_t size) return arr ? arr : ZERO_SIZE_PTR; } -static int copy_reference_state(struct bpf_func_state *dst, const struct bpf_func_state *src) +static int copy_reference_state(struct bpf_verifier_state *dst, const struct bpf_verifier_state *src) { dst->refs = copy_array(dst->refs, src->refs, src->acquired_refs, sizeof(struct bpf_reference_state), GFP_KERNEL); if (!dst->refs) return -ENOMEM; - dst->active_locks = src->active_locks; dst->acquired_refs = src->acquired_refs; + dst->active_locks = src->active_locks; + dst->active_preempt_locks = src->active_preempt_locks; + dst->active_rcu_lock = src->active_rcu_lock; return 0; } @@ -1304,7 +1306,7 @@ static int copy_stack_state(struct bpf_func_state *dst, const struct bpf_func_st return 0; } -static int resize_reference_state(struct bpf_func_state *state, size_t n) +static int resize_reference_state(struct bpf_verifier_state *state, size_t n) { state->refs = realloc_array(state->refs, state->acquired_refs, n, sizeof(struct bpf_reference_state)); @@ -1349,7 +1351,7 @@ static int grow_stack_state(struct bpf_verifier_env *env, struct bpf_func_state */ static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx) { - struct bpf_func_state *state = cur_func(env); + struct bpf_verifier_state *state = env->cur_state; int new_ofs = state->acquired_refs; int id, err; @@ -1367,7 +1369,7 @@ static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx) static int acquire_lock_state(struct bpf_verifier_env *env, int insn_idx, enum ref_state_type type, int id, void *ptr) { - struct bpf_func_state *state = cur_func(env); + struct bpf_verifier_state *state = env->cur_state; int new_ofs = state->acquired_refs; int err; @@ -1384,7 +1386,7 @@ static int acquire_lock_state(struct bpf_verifier_env *env, int insn_idx, enum r } /* release function corresponding to acquire_reference_state(). Idempotent. */ -static int release_reference_state(struct bpf_func_state *state, int ptr_id) +static int release_reference_state(struct bpf_verifier_state *state, int ptr_id) { int i, last_idx; @@ -1404,7 +1406,7 @@ static int release_reference_state(struct bpf_func_state *state, int ptr_id) return -EINVAL; } -static int release_lock_state(struct bpf_func_state *state, int type, int id, void *ptr) +static int release_lock_state(struct bpf_verifier_state *state, int type, int id, void *ptr) { int i, last_idx; @@ -1425,10 +1427,9 @@ static int release_lock_state(struct bpf_func_state *state, int type, int id, vo return -EINVAL; } -static struct bpf_reference_state *find_lock_state(struct bpf_verifier_env *env, enum ref_state_type type, +static struct bpf_reference_state *find_lock_state(struct bpf_verifier_state *state, enum ref_state_type type, int id, void *ptr) { - struct bpf_func_state *state = cur_func(env); int i; for (i = 0; i < state->acquired_refs; i++) { @@ -1447,7 +1448,6 @@ static void free_func_state(struct bpf_func_state *state) { if (!state) return; - kfree(state->refs); kfree(state->stack); kfree(state); } @@ -1461,6 +1461,7 @@ static void free_verifier_state(struct bpf_verifier_state *state, free_func_state(state->frame[i]); state->frame[i] = NULL; } + kfree(state->refs); if (free_self) kfree(state); } @@ -1471,12 +1472,7 @@ static void free_verifier_state(struct bpf_verifier_state *state, static int copy_func_state(struct bpf_func_state *dst, const struct bpf_func_state *src) { - int err; - - memcpy(dst, src, offsetof(struct bpf_func_state, acquired_refs)); - err = copy_reference_state(dst, src); - if (err) - return err; + memcpy(dst, src, offsetof(struct bpf_func_state, stack)); return copy_stack_state(dst, src); } @@ -1493,9 +1489,10 @@ static int copy_verifier_state(struct bpf_verifier_state *dst_state, free_func_state(dst_state->frame[i]); dst_state->frame[i] = NULL; } + err = copy_reference_state(dst_state, src); + if (err) + return err; dst_state->speculative = src->speculative; - dst_state->active_rcu_lock = src->active_rcu_lock; - dst_state->active_preempt_lock = src->active_preempt_lock; dst_state->in_sleepable = src->in_sleepable; dst_state->curframe = src->curframe; dst_state->branches = src->branches; @@ -4499,7 +4496,7 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno) fmt_stack_mask(env->tmp_str_buf, TMP_STR_BUF_LEN, bt_frame_stack_mask(bt, fr)); verbose(env, "stack=%s: ", env->tmp_str_buf); - print_verifier_state(env, func, true); + print_verifier_state(env, st, fr, true); } } @@ -5496,7 +5493,7 @@ static bool in_sleepable(struct bpf_verifier_env *env) static bool in_rcu_cs(struct bpf_verifier_env *env) { return env->cur_state->active_rcu_lock || - cur_func(env)->active_locks || + env->cur_state->active_locks || !in_sleepable(env); } @@ -7850,15 +7847,15 @@ static int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg * Since only one bpf_spin_lock is allowed the checks are simpler than * reg_is_refcounted() logic. The verifier needs to remember only * one spin_lock instead of array of acquired_refs. - * cur_func(env)->active_locks remembers which map value element or allocated + * env->cur_state->active_locks remembers which map value element or allocated * object got locked and clears it after bpf_spin_unlock. */ static int process_spin_lock(struct bpf_verifier_env *env, int regno, bool is_lock) { struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; + struct bpf_verifier_state *cur = env->cur_state; bool is_const = tnum_is_const(reg->var_off); - struct bpf_func_state *cur = cur_func(env); u64 val = reg->var_off.value; struct bpf_map *map = NULL; struct btf *btf = NULL; @@ -7925,7 +7922,7 @@ static int process_spin_lock(struct bpf_verifier_env *env, int regno, return -EINVAL; } - if (release_lock_state(cur_func(env), REF_TYPE_LOCK, reg->id, ptr)) { + if (release_lock_state(env->cur_state, REF_TYPE_LOCK, reg->id, ptr)) { verbose(env, "bpf_spin_unlock of different lock\n"); return -EINVAL; } @@ -9679,7 +9676,7 @@ static int release_reference(struct bpf_verifier_env *env, struct bpf_reg_state *reg; int err; - err = release_reference_state(cur_func(env), ref_obj_id); + err = release_reference_state(env->cur_state, ref_obj_id); if (err) return err; @@ -9757,9 +9754,7 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls callsite, state->curframe + 1 /* frameno within this callchain */, subprog /* subprog number within this prog */); - /* Transfer references to the callee */ - err = copy_reference_state(callee, caller); - err = err ?: set_callee_state_cb(env, caller, callee, callsite); + err = set_callee_state_cb(env, caller, callee, callsite); if (err) goto err_out; @@ -9992,14 +9987,14 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, const char *sub_name = subprog_name(env, subprog); /* Only global subprogs cannot be called with a lock held. */ - if (cur_func(env)->active_locks) { + if (env->cur_state->active_locks) { verbose(env, "global function calls are not allowed while holding a lock,\n" "use static function instead\n"); return -EINVAL; } /* Only global subprogs cannot be called with preemption disabled. */ - if (env->cur_state->active_preempt_lock) { + if (env->cur_state->active_preempt_locks) { verbose(env, "global function calls are not allowed with preemption disabled,\n" "use static function instead\n"); return -EINVAL; @@ -10039,9 +10034,9 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, if (env->log.level & BPF_LOG_LEVEL) { verbose(env, "caller:\n"); - print_verifier_state(env, caller, true); + print_verifier_state(env, state, caller->frameno, true); verbose(env, "callee:\n"); - print_verifier_state(env, state->frame[state->curframe], true); + print_verifier_state(env, state, state->curframe, true); } return 0; @@ -10333,11 +10328,6 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) caller->regs[BPF_REG_0] = *r0; } - /* Transfer references to the caller */ - err = copy_reference_state(caller, callee); - if (err) - return err; - /* for callbacks like bpf_loop or bpf_for_each_map_elem go back to callsite, * there function call logic would reschedule callback visit. If iteration * converges is_state_visited() would prune that visit eventually. @@ -10350,9 +10340,9 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) if (env->log.level & BPF_LOG_LEVEL) { verbose(env, "returning from callee:\n"); - print_verifier_state(env, callee, true); + print_verifier_state(env, state, callee->frameno, true); verbose(env, "to caller at %d:\n", *insn_idx); - print_verifier_state(env, caller, true); + print_verifier_state(env, state, caller->frameno, true); } /* clear everything in the callee. In case of exceptional exits using * bpf_throw, this will be done by copy_verifier_state for extra frames. */ @@ -10502,11 +10492,11 @@ record_func_key(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, static int check_reference_leak(struct bpf_verifier_env *env, bool exception_exit) { - struct bpf_func_state *state = cur_func(env); + struct bpf_verifier_state *state = env->cur_state; bool refs_lingering = false; int i; - if (!exception_exit && state->frameno) + if (!exception_exit && cur_func(env)->frameno) return 0; for (i = 0; i < state->acquired_refs; i++) { @@ -10523,7 +10513,7 @@ static int check_resource_leak(struct bpf_verifier_env *env, bool exception_exit { int err; - if (check_lock && cur_func(env)->active_locks) { + if (check_lock && env->cur_state->active_locks) { verbose(env, "%s cannot be used inside bpf_spin_lock-ed region\n", prefix); return -EINVAL; } @@ -10539,7 +10529,7 @@ static int check_resource_leak(struct bpf_verifier_env *env, bool exception_exit return -EINVAL; } - if (check_lock && env->cur_state->active_preempt_lock) { + if (check_lock && env->cur_state->active_preempt_locks) { verbose(env, "%s cannot be used inside bpf_preempt_disable-ed region\n", prefix); return -EINVAL; } @@ -10727,7 +10717,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn env->insn_aux_data[insn_idx].storage_get_func_atomic = true; } - if (env->cur_state->active_preempt_lock) { + if (env->cur_state->active_preempt_locks) { if (fn->might_sleep) { verbose(env, "sleepable helper %s#%d in non-preemptible region\n", func_id_name(func_id), func_id); @@ -10784,7 +10774,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn struct bpf_func_state *state; struct bpf_reg_state *reg; - err = release_reference_state(cur_func(env), ref_obj_id); + err = release_reference_state(env->cur_state, ref_obj_id); if (!err) { bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({ if (reg->ref_obj_id == ref_obj_id) { @@ -11746,7 +11736,7 @@ static int ref_set_non_owning(struct bpf_verifier_env *env, struct bpf_reg_state { struct btf_record *rec = reg_btf_record(reg); - if (!cur_func(env)->active_locks) { + if (!env->cur_state->active_locks) { verbose(env, "verifier internal error: ref_set_non_owning w/o active lock\n"); return -EFAULT; } @@ -11765,12 +11755,11 @@ static int ref_set_non_owning(struct bpf_verifier_env *env, struct bpf_reg_state static int ref_convert_owning_non_owning(struct bpf_verifier_env *env, u32 ref_obj_id) { - struct bpf_func_state *state, *unused; + struct bpf_verifier_state *state = env->cur_state; + struct bpf_func_state *unused; struct bpf_reg_state *reg; int i; - state = cur_func(env); - if (!ref_obj_id) { verbose(env, "verifier internal error: ref_obj_id is zero for " "owning -> non-owning conversion\n"); @@ -11860,9 +11849,9 @@ static int check_reg_allocation_locked(struct bpf_verifier_env *env, struct bpf_ } id = reg->id; - if (!cur_func(env)->active_locks) + if (!env->cur_state->active_locks) return -EINVAL; - s = find_lock_state(env, REF_TYPE_LOCK, id, ptr); + s = find_lock_state(env->cur_state, REF_TYPE_LOCK, id, ptr); if (!s) { verbose(env, "held lock and object are not in the same allocation\n"); return -EINVAL; @@ -12789,17 +12778,17 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, return -EINVAL; } - if (env->cur_state->active_preempt_lock) { + if (env->cur_state->active_preempt_locks) { if (preempt_disable) { - env->cur_state->active_preempt_lock++; + env->cur_state->active_preempt_locks++; } else if (preempt_enable) { - env->cur_state->active_preempt_lock--; + env->cur_state->active_preempt_locks--; } else if (sleepable) { verbose(env, "kernel func %s is sleepable within non-preemptible region\n", func_name); return -EACCES; } } else if (preempt_disable) { - env->cur_state->active_preempt_lock++; + env->cur_state->active_preempt_locks++; } else if (preempt_enable) { verbose(env, "unmatched attempt to enable preemption (kernel function %s)\n", func_name); return -EINVAL; @@ -14495,12 +14484,12 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env, /* Got here implies adding two SCALAR_VALUEs */ if (WARN_ON_ONCE(ptr_reg)) { - print_verifier_state(env, state, true); + print_verifier_state(env, vstate, vstate->curframe, true); verbose(env, "verifier internal error: unexpected ptr_reg\n"); return -EINVAL; } if (WARN_ON(!src_reg)) { - print_verifier_state(env, state, true); + print_verifier_state(env, vstate, vstate->curframe, true); verbose(env, "verifier internal error: no src_reg\n"); return -EINVAL; } @@ -15398,7 +15387,7 @@ static void mark_ptr_or_null_regs(struct bpf_verifier_state *vstate, u32 regno, * No one could have freed the reference state before * doing the NULL check. */ - WARN_ON_ONCE(release_reference_state(state, id)); + WARN_ON_ONCE(release_reference_state(vstate, id)); bpf_for_each_reg_in_vstate(vstate, state, reg, ({ mark_ptr_or_null_reg(state, reg, id, is_null); @@ -15708,7 +15697,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, *insn_idx)) return -EFAULT; if (env->log.level & BPF_LOG_LEVEL) - print_insn_state(env, this_branch->frame[this_branch->curframe]); + print_insn_state(env, this_branch, this_branch->curframe); *insn_idx += insn->off; return 0; } else if (pred == 0) { @@ -15722,7 +15711,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, *insn_idx)) return -EFAULT; if (env->log.level & BPF_LOG_LEVEL) - print_insn_state(env, this_branch->frame[this_branch->curframe]); + print_insn_state(env, this_branch, this_branch->curframe); return 0; } @@ -15839,7 +15828,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, return -EACCES; } if (env->log.level & BPF_LOG_LEVEL) - print_insn_state(env, this_branch->frame[this_branch->curframe]); + print_insn_state(env, this_branch, this_branch->curframe); return 0; } @@ -17750,7 +17739,7 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old, return true; } -static bool refsafe(struct bpf_func_state *old, struct bpf_func_state *cur, +static bool refsafe(struct bpf_verifier_state *old, struct bpf_verifier_state *cur, struct bpf_idmap *idmap) { int i; @@ -17758,6 +17747,15 @@ static bool refsafe(struct bpf_func_state *old, struct bpf_func_state *cur, if (old->acquired_refs != cur->acquired_refs) return false; + if (old->active_locks != cur->active_locks) + return false; + + if (old->active_preempt_locks != cur->active_preempt_locks) + return false; + + if (old->active_rcu_lock != cur->active_rcu_lock) + return false; + for (i = 0; i < old->acquired_refs; i++) { if (!check_ids(old->refs[i].id, cur->refs[i].id, idmap) || old->refs[i].type != cur->refs[i].type) @@ -17820,9 +17818,6 @@ static bool func_states_equal(struct bpf_verifier_env *env, struct bpf_func_stat if (!stacksafe(env, old, cur, &env->idmap_scratch, exact)) return false; - if (!refsafe(old, cur, &env->idmap_scratch)) - return false; - return true; } @@ -17850,13 +17845,10 @@ static bool states_equal(struct bpf_verifier_env *env, if (old->speculative && !cur->speculative) return false; - if (old->active_rcu_lock != cur->active_rcu_lock) - return false; - - if (old->active_preempt_lock != cur->active_preempt_lock) + if (old->in_sleepable != cur->in_sleepable) return false; - if (old->in_sleepable != cur->in_sleepable) + if (!refsafe(old, cur, &env->idmap_scratch)) return false; /* for states to be equal callsites have to be the same @@ -18249,9 +18241,9 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx) verbose_linfo(env, insn_idx, "; "); verbose(env, "infinite loop detected at insn %d\n", insn_idx); verbose(env, "cur state:"); - print_verifier_state(env, cur->frame[cur->curframe], true); + print_verifier_state(env, cur, cur->curframe, true); verbose(env, "old state:"); - print_verifier_state(env, sl->state.frame[cur->curframe], true); + print_verifier_state(env, &sl->state, cur->curframe, true); return -EINVAL; } /* if the verifier is processing a loop, avoid adding new state @@ -18607,7 +18599,7 @@ static int do_check(struct bpf_verifier_env *env) env->prev_insn_idx, env->insn_idx, env->cur_state->speculative ? " (speculative execution)" : ""); - print_verifier_state(env, state->frame[state->curframe], true); + print_verifier_state(env, state, state->curframe, true); do_print_state = false; } @@ -18619,7 +18611,7 @@ static int do_check(struct bpf_verifier_env *env) }; if (verifier_state_scratched(env)) - print_insn_state(env, state->frame[state->curframe]); + print_insn_state(env, state, state->curframe); verbose_linfo(env, env->insn_idx, "; "); env->prev_log_pos = env->log.end_pos; @@ -18751,7 +18743,7 @@ static int do_check(struct bpf_verifier_env *env) return -EINVAL; } - if (cur_func(env)->active_locks) { + if (env->cur_state->active_locks) { if ((insn->src_reg == BPF_REG_0 && insn->imm != BPF_FUNC_spin_unlock) || (insn->src_reg == BPF_PSEUDO_KFUNC_CALL && (insn->off != 0 || !is_bpf_graph_api_kfunc(insn->imm)))) { From patchwork Wed Dec 4 03:03:55 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13893168 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F1ABEFC1D for ; Wed, 4 Dec 2024 03:04:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.65 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281452; cv=none; b=PNR+Rr4LLSkvCwYhDSMzcaC8Q8y4JnrEj3A4/JgWo4TFC19yZF88pjU2joabCgKlkACRiF2BS1PVhN4Cc0Zl6vFzkFtoVQPTNiqhLz3N5jK/BKFR0PDvsDkvBMcG9Y3qCZsI7XsyrfS3mubmKCr/WnhEZkY0REVUajeou5D2dcM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281452; c=relaxed/simple; bh=Td+xWtw0EqpnHOZIlUSYLza42yyOn1XyIk0b8BdVOjw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=MIgTm/7ynw+SwLYKJhimVRRTStaE4Ih4AEz+uZwGU6owbGIRE4e30ayjRQcQ7RFmxzzMo/5aJ0sxYUxhNmGuUWsf9ENBe2w35+8dL0oT1mUnrQYqH13YLZwcURbO+n/Zd65JpYSWMW+Q0e3xeFK54lVie/uJXe9f5n7LdjzMq30= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=MBP26YEj; arc=none smtp.client-ip=209.85.128.65 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="MBP26YEj" Received: by mail-wm1-f65.google.com with SMTP id 5b1f17b1804b1-434a8640763so51998885e9.1 for ; Tue, 03 Dec 2024 19:04:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1733281448; x=1733886248; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=OmAgR3K9Kvxtkhz9iXzav1JygOw2MAm1MEcN2NhYj6o=; b=MBP26YEjbuE2jza9TR56ENA9bmYI4Vy9yTUZ5laEsqoTHxRJW9YPMcvjS+w18MlZ7+ ArClhA4yaWmCue3Cjd23SflqdaKa5apuNcdHu/7++26UNFYjn/1aOFETagUM4uCzlB86 ef5J0zszYv4pgQI1u7DYSTD2U4e3DDUhLiylQmjcaQheA6QlayRQl2qb3J5M1v45+U+9 VTelNRGOUVBfPok8ukftP0NmZigRYL2MBcCWHHyVs5q0nTJhhocnByAH0jXg4UDaEIEb /y+X7umNhQTB4FJmzUIBwLW3hzRCNkyyhGGkdR1IkeUjDZs46sexhYZo6bf1XkmNkyC5 If/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733281448; x=1733886248; 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=OmAgR3K9Kvxtkhz9iXzav1JygOw2MAm1MEcN2NhYj6o=; b=XHjsjSqV+DbYp82R42HkDXbLlKg7mDoyBPkvtNNOWFKyMIv23j98yMjeeTZPFZ75V2 yQxJLMc/+0p2eoVUA22vrnTBQWFf2TroRCfCxMdxFKIoyJvqYN/swgeACvg5BGDOYWNC C0Hnv3NCMw9yD3cKZaL0KzTaFbVCh+ZQkSgx2Kc4ws28vvsQEg8fw92DVhXDhXYnXoXO tsDO6ziPA0qV6UJ5vNwuzYgWmHKP7Se+1r2/MB4FxL20Yw57NFeM9u4fYQdUcueq1PVk dDuFN+R1Mgqo2naXI+OldwVZ9C/oyotdf3zvII6+lS25eMf0FTv+4n9ULHL+qYQs14pC Al9w== X-Gm-Message-State: AOJu0YwZKh0KwQqbe1uniNlU3amT1EE8lla8YxGU+r/h7vcaQExDeKuB vHHZeupiWKNAW7OUMxvWmU/ati3WoCDHW0Eq8M7AbahT9tfxCi5zYleStg2iYK0= X-Gm-Gg: ASbGncsnoeDWEgnqvQ8M9rwKVOfNXiecY1VPQaBkvb0Yi69iok1Dt9Uk0OtQINgYqhe OZ7/xNq1zeMc9+fM2zhbKLg9CXO+Bt/Xv2qsLUWO/InTx9SvyrzUjDPOOyQwhL0dCkLLNKfiHBp PqTW14GttsO9yR5lASc40aigYnxihEOu/Q61quzc1CrNWyUyQem3J1IvhOXjAU9DxKbiqNFAjDt o8eSvqRbv6utTqQGhw84+iXfBTZ4rIyAKvg6fzgdIlHjT9CiejO6/s/it8M8UUNOt2lBIqIj4zp Vw== X-Google-Smtp-Source: AGHT+IEbtZehN5gG4itlT7TxO9S42EO6F3vUNW35CYihh//w3+a8CsC9Do/rINh8D9kVqJILVDPJ0g== X-Received: by 2002:a05:600c:35c9:b0:434:a5b6:273c with SMTP id 5b1f17b1804b1-434d09b1464mr40192245e9.2.1733281448182; Tue, 03 Dec 2024 19:04:08 -0800 (PST) Received: from localhost (fwdproxy-cln-029.fbsv.net. [2a03:2880:31ff:1d::face:b00c]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-434d5272427sm7755535e9.4.2024.12.03.19.04.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 19:04:07 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Eduard Zingerman , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , kernel-team@fb.com Subject: [PATCH bpf-next v6 2/7] bpf: Refactor {acquire,release}_reference_state Date: Tue, 3 Dec 2024 19:03:55 -0800 Message-ID: <20241204030400.208005-3-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241204030400.208005-1-memxor@gmail.com> References: <20241204030400.208005-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9185; h=from:subject; bh=Td+xWtw0EqpnHOZIlUSYLza42yyOn1XyIk0b8BdVOjw=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnT8Q/lymyMOWlJgPlrSSCqqZD2eAvjF2VlRaUjFH3 eclBV/CJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ0/EPwAKCRBM4MiGSL8RyugwD/ 9lcoTdr9yA7c/knvnmeiHzfMURlGf/90L1fFYmAmlxuylkIgGC7zmxX4clcQ5X8OxhQD26CYK04ABr 4mBF7C50Q2CkVR0CeJpHOpPht2Q3O8IsAqsD6zUjmaLkZFD5VCgrSyRBR7cg7/bSOzINeAjl9GaOl7 +UHOBkJVmwWZj/E5H/HTKjqMoriqPduwmMNWtk0GPi/7pSnl+qmrtKw4ifUIte66oOjcaZR9VJ8WXE MVSDL+s5qgHNtZ1KSbim+i9emg90/ZL2eJ4C46RqwpbeBpG9OUcLEGnF8gEqDG2ceM4+M+ZlQV9OKt W01rnnMsYIKm76kEdXKuzIJncvXwOSpNdQMazZlQQ6TYju3eKF0bR/oN0rD8hAgf2VikxEuo3+NGum UjugtjPNOTMYW0ZgrlviRRA+Wg4yA7shLh3ZGDlNbWHSMk+mRTg+s9dsYjy/0+f8V0rH2puYohYs5G dmzNZH+qinpVT/40HDRPyaCIlMGTmON7e2zECUhcPFYIf6SZN7LhgDqYOh29DrugSnneJbHi3qIq4J vLMCwBVCmGmXDcpwM7+EvTG7HvjPvaIeksAob+21nR2H1aljjoKaCa/iSKUCnClVvOqPDPtzSlBvlP eySz9EDA6mro3hOtbO7A8iOOHd5hGECFKnMl2L6VclnwDrc1oj8O+hwBey3g== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net In preparation for introducing support for more reference types which have to add and remove reference state, refactor the acquire_reference_state and release_reference_state functions to share common logic. The acquire_reference_state function simply handles growing the acquired refs and returning the pointer to the new uninitialized element, which can be filled in by the caller. The release_reference_state function simply erases a reference state entry in the acquired_refs array and shrinks it. The callers are responsible for finding the suitable element by matching on various fields of the reference state and requesting deletion through this function. It is not supposed to be called directly. Existing callers of release_reference_state were using it to find and remove state for a given ref_obj_id without scrubbing the associated registers in the verifier state. Introduce release_reference_nomark to provide this functionality and convert callers. We now use this new release_reference_nomark function within release_reference as well. It needs to operate on a verifier state instead of taking verifier env as mark_ptr_or_null_regs requires operating on verifier state of the two branches of a NULL condition check, therefore env->cur_state cannot be used directly. Acked-by: Eduard Zingerman Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 109 +++++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 50 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 019c56c782a2..41b3dc1ce450 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -196,7 +196,8 @@ struct bpf_verifier_stack_elem { #define BPF_PRIV_STACK_MIN_SIZE 64 -static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx); +static int acquire_reference(struct bpf_verifier_env *env, int insn_idx); +static int release_reference_nomark(struct bpf_verifier_state *state, int ref_obj_id); static int release_reference(struct bpf_verifier_env *env, int ref_obj_id); static void invalidate_non_owning_refs(struct bpf_verifier_env *env); static bool in_rbtree_lock_required_cb(struct bpf_verifier_env *env); @@ -771,7 +772,7 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ if (clone_ref_obj_id) id = clone_ref_obj_id; else - id = acquire_reference_state(env, insn_idx); + id = acquire_reference(env, insn_idx); if (id < 0) return id; @@ -1033,7 +1034,7 @@ static int mark_stack_slots_iter(struct bpf_verifier_env *env, if (spi < 0) return spi; - id = acquire_reference_state(env, insn_idx); + id = acquire_reference(env, insn_idx); if (id < 0) return id; @@ -1349,77 +1350,68 @@ static int grow_stack_state(struct bpf_verifier_env *env, struct bpf_func_state * On success, returns a valid pointer id to associate with the register * On failure, returns a negative errno. */ -static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx) +static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env, int insn_idx) { struct bpf_verifier_state *state = env->cur_state; int new_ofs = state->acquired_refs; - int id, err; + int err; err = resize_reference_state(state, state->acquired_refs + 1); if (err) - return err; - id = ++env->id_gen; - state->refs[new_ofs].type = REF_TYPE_PTR; - state->refs[new_ofs].id = id; + return NULL; state->refs[new_ofs].insn_idx = insn_idx; - return id; + return &state->refs[new_ofs]; +} + +static int acquire_reference(struct bpf_verifier_env *env, int insn_idx) +{ + struct bpf_reference_state *s; + + s = acquire_reference_state(env, insn_idx); + if (!s) + return -ENOMEM; + s->type = REF_TYPE_PTR; + s->id = ++env->id_gen; + return s->id; } static int acquire_lock_state(struct bpf_verifier_env *env, int insn_idx, enum ref_state_type type, int id, void *ptr) { struct bpf_verifier_state *state = env->cur_state; - int new_ofs = state->acquired_refs; - int err; + struct bpf_reference_state *s; - err = resize_reference_state(state, state->acquired_refs + 1); - if (err) - return err; - state->refs[new_ofs].type = type; - state->refs[new_ofs].id = id; - state->refs[new_ofs].insn_idx = insn_idx; - state->refs[new_ofs].ptr = ptr; + s = acquire_reference_state(env, insn_idx); + s->type = type; + s->id = id; + s->ptr = ptr; state->active_locks++; return 0; } -/* release function corresponding to acquire_reference_state(). Idempotent. */ -static int release_reference_state(struct bpf_verifier_state *state, int ptr_id) +static void release_reference_state(struct bpf_verifier_state *state, int idx) { - int i, last_idx; + int last_idx; last_idx = state->acquired_refs - 1; - for (i = 0; i < state->acquired_refs; i++) { - if (state->refs[i].type != REF_TYPE_PTR) - continue; - if (state->refs[i].id == ptr_id) { - if (last_idx && i != last_idx) - memcpy(&state->refs[i], &state->refs[last_idx], - sizeof(*state->refs)); - memset(&state->refs[last_idx], 0, sizeof(*state->refs)); - state->acquired_refs--; - return 0; - } - } - return -EINVAL; + if (last_idx && idx != last_idx) + memcpy(&state->refs[idx], &state->refs[last_idx], sizeof(*state->refs)); + memset(&state->refs[last_idx], 0, sizeof(*state->refs)); + state->acquired_refs--; + return; } static int release_lock_state(struct bpf_verifier_state *state, int type, int id, void *ptr) { - int i, last_idx; + int i; - last_idx = state->acquired_refs - 1; for (i = 0; i < state->acquired_refs; i++) { if (state->refs[i].type != type) continue; if (state->refs[i].id == id && state->refs[i].ptr == ptr) { - if (last_idx && i != last_idx) - memcpy(&state->refs[i], &state->refs[last_idx], - sizeof(*state->refs)); - memset(&state->refs[last_idx], 0, sizeof(*state->refs)); - state->acquired_refs--; + release_reference_state(state, i); state->active_locks--; return 0; } @@ -9666,21 +9658,38 @@ static void mark_pkt_end(struct bpf_verifier_state *vstate, int regn, bool range reg->range = AT_PKT_END; } +static int release_reference_nomark(struct bpf_verifier_state *state, int ref_obj_id) +{ + int i; + + for (i = 0; i < state->acquired_refs; i++) { + if (state->refs[i].type != REF_TYPE_PTR) + continue; + if (state->refs[i].id == ref_obj_id) { + release_reference_state(state, i); + return 0; + } + } + return -EINVAL; +} + /* The pointer with the specified id has released its reference to kernel * resources. Identify all copies of the same pointer and clear the reference. + * + * This is the release function corresponding to acquire_reference(). Idempotent. */ -static int release_reference(struct bpf_verifier_env *env, - int ref_obj_id) +static int release_reference(struct bpf_verifier_env *env, int ref_obj_id) { + struct bpf_verifier_state *vstate = env->cur_state; struct bpf_func_state *state; struct bpf_reg_state *reg; int err; - err = release_reference_state(env->cur_state, ref_obj_id); + err = release_reference_nomark(vstate, ref_obj_id); if (err) return err; - bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({ + bpf_for_each_reg_in_vstate(vstate, state, reg, ({ if (reg->ref_obj_id == ref_obj_id) mark_reg_invalid(env, reg); })); @@ -10774,7 +10783,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn struct bpf_func_state *state; struct bpf_reg_state *reg; - err = release_reference_state(env->cur_state, ref_obj_id); + err = release_reference_nomark(env->cur_state, ref_obj_id); if (!err) { bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({ if (reg->ref_obj_id == ref_obj_id) { @@ -11107,7 +11116,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn /* For release_reference() */ regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id; } else if (is_acquire_function(func_id, meta.map_ptr)) { - int id = acquire_reference_state(env, insn_idx); + int id = acquire_reference(env, insn_idx); if (id < 0) return id; @@ -13087,7 +13096,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, } mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *)); if (is_kfunc_acquire(&meta)) { - int id = acquire_reference_state(env, insn_idx); + int id = acquire_reference(env, insn_idx); if (id < 0) return id; @@ -15387,7 +15396,7 @@ static void mark_ptr_or_null_regs(struct bpf_verifier_state *vstate, u32 regno, * No one could have freed the reference state before * doing the NULL check. */ - WARN_ON_ONCE(release_reference_state(vstate, id)); + WARN_ON_ONCE(release_reference_nomark(vstate, id)); bpf_for_each_reg_in_vstate(vstate, state, reg, ({ mark_ptr_or_null_reg(state, reg, id, is_null); From patchwork Wed Dec 4 03:03:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13893169 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BF5DE126C1E for ; Wed, 4 Dec 2024 03:04:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.66 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281453; cv=none; b=UrwNHfwoYtRpVTmZN04C73i2skLSVOdUGEP1lDd91QiHNGeoIB2f3Jfecl7jX/3o2J+QVPZuv0PNh4/Ipb4b/Xz2PcksAPTO63ssN5vs/JPaK7e7a+IDEqk/wIgb36qHOkeUkIlGz+ClItnnvT94f5R42MbaGXHZdlK9F/hOoNw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281453; c=relaxed/simple; bh=57qSfzqNWdM68fEvHBN8TFelxnu11wjWt/bN39hl92s=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=uXx8i1OqZ9FpjPblDbrGSSQy+M7N0hKKgYz3n1aZXxDzAYVDkdyq9SuRhF3jYC0IKsp2ZRIiPv8+Fh/D+Lv4FFammoU3NCaelCYd9xUmvboPMYuRxmdBPRCY6MC4siDLw6GqzSawN3uqUBTUzJz1et+C4ZCcFg8I+yB/pdhXcvY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=GDgkVGAd; arc=none smtp.client-ip=209.85.128.66 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="GDgkVGAd" Received: by mail-wm1-f66.google.com with SMTP id 5b1f17b1804b1-434a736518eso76705045e9.1 for ; Tue, 03 Dec 2024 19:04:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1733281450; x=1733886250; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ZMGh0z6cq+JkeqtzlQVAOThgczFnVyH3wUz6lcaHUR0=; b=GDgkVGAdQ2glk2p6K+0Wypxz1pb+o+bY6a0uOfNOAs5Fc2HHs6T44o9HAUMKFN6Ybl Eg4AZu2AoXyTczD4I0e7RfYiTPaLl3AR+3j8VWnHt2TvG8oK3EswFjTjr+L8kGDCaPrV 2PA0qvNGUqKBug8INVqyk3ePTJLD0JcPGFdpNxcWTQ5pMUJZLuzW9PzMw1Moi3OkJ+mh ccJz52dpsyu9CgrVtdWxrKTHDPrwNS493NCYcvl0aHqtUThTtj4DGlPgPwquMadMAR3v tZAdU69R4zo4/RyVDcBcMDHSFpip2Gd7CeQsBb+8wqEDxkKeb5wK0wzjh4j5jlK7+rxo S8tA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733281450; x=1733886250; 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=ZMGh0z6cq+JkeqtzlQVAOThgczFnVyH3wUz6lcaHUR0=; b=CBG+SHVHmvHSn6SCzDd4n9ziIwA/FmMHkPf+peob4mhSb8xNT4m5DeKdYKh2ZUqbdh oITofoWtLzVxMqdhIu2aIi3pAGLEQOinmc1IwK13GWlsHO1ZZhwGbZhlax24YNWSGWTU C9ox2ba9/+Yi8mP/Og88ZvutOsoIe1tytIhqeuOSEyjtL4tpfnYngtKo7IlTL4E21e0v yieigPY/zem4J5Y0kiLYv9ia23slxLWcdH/G2dKoCBAtGwuCP3Y8ZbpbybFi+O/S/nbX Tuqw38dTc2Bs4MMVUL1leDwreHortJITiQUk6z15Kp+33D17/0TDygIkdl2v80zg/Iwz t1TA== X-Gm-Message-State: AOJu0YzI+RlNskRE2M/j0f8P3KmZZJWCvXZdX4/7EhJwysVPcDpKMst/ 0VV30G/sq3HBzTeN+/QCskjqFCG/MnUcBhswf/w61iKtZQdUCA6jUEl5666QFWs= X-Gm-Gg: ASbGncvRpDki1q/fWaPbggyF52wMijvJdR5X8VszhPA10wx/FvuMgNZHZlag8il49CB OnqSVlwt+gPxZrY++1Hoo72a69Je+3vrQxRRZ4OO82LVfLLKd6W1n90krMlFDnUk/PvzAsrK3e/ 2O1cRohZjqbcjJw7/fj4NiLznpzZXwk67RFsWwIHni9/lYWyKzP5uatCx6G+akGaWNAdgRR7X7R Tboryv0TQ65rbZ47X2C0nGCG4lGcC1R96icvcutyPibkzEMbOjomTzcEYwaiCwpbrVIa41K5b/P eQ== X-Google-Smtp-Source: AGHT+IEkcKp3wNIzLefhxiarIvhOiY4GfpwORu9tEhlKa+8BiadbSQQnPrH8ZpA79Lcj560S4+o/mQ== X-Received: by 2002:a05:600c:3546:b0:434:9fac:b157 with SMTP id 5b1f17b1804b1-434d09c10a5mr47903155e9.13.1733281449881; Tue, 03 Dec 2024 19:04:09 -0800 (PST) Received: from localhost (fwdproxy-cln-025.fbsv.net. [2a03:2880:31ff:19::face:b00c]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-434d52c0bfasm7674215e9.32.2024.12.03.19.04.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 19:04:08 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Eduard Zingerman , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , kernel-team@fb.com Subject: [PATCH bpf-next v6 3/7] bpf: Refactor mark_{dynptr,iter}_read Date: Tue, 3 Dec 2024 19:03:56 -0800 Message-ID: <20241204030400.208005-4-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241204030400.208005-1-memxor@gmail.com> References: <20241204030400.208005-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2790; h=from:subject; bh=57qSfzqNWdM68fEvHBN8TFelxnu11wjWt/bN39hl92s=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnT8Q/yxTRIlNDljrjf+vNp1cNjUoWpdCJJ6SMlX0A R2y4Q1SJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ0/EPwAKCRBM4MiGSL8Ryg6ZEA DAwNE9M5j4cPHELKQwoBGQtkCaRM6CLpDCiFVjDuZDUSnGlvGoX133GvF/KZ8661fxqCZKESxB0qaP 6Wb966Y/y7bpwsKyQyJIqR4Ck+NU45VJGF4zgeEX84OUW+XNZ/6+5m3n5/Z3MWa8ydT8cmvUYcHeOr 6Gfo5TxS+IkgLfdhd18g3lKQ4Nid34ozXvyETVPo518pumdqF29bldAHgPXqaPwtbx0eY6IAaUIXMS C5z5ShKpiXpY1eAXUyOXeCNwOWBJcjbw15FqAxmVsJGyf3uAiHSZZ+s29tZUq1EXtuUMOo9kfIntt4 VSN9iU5fXqFdsQnI7nEdqgIOXFsSj51NOUf8AcvDwYvyUYFsaA1VM0qbQXa7hH+JbO0v0qWudMXXj6 rITrCUrCmF8fvZ6Z4ZCxDfsjbHSbZw1ghfZtmujP1l511hVXdMC4D80caivHJ6Tyy/ir+MTJERTWeO 81UPTCJWSZIM+HzB3SfN/K6pHXGEnuyD4aG05pce0aCFBlX7+clXGpsmDu8+NO35k3dD2v5ipnpG8f MvI9vcIRsqIixkB/nPXc//VlQlkk0ZTirx44+cgQBN4vrnWsDas85dziCz37vYrZ8gwpjVfHKxhCkG 4WhwO4P4DQGO9Co98dnZokNa6c6RRlPe02yVJazgZsdZy4ij2y3nbABTk3Sw== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net There is possibility of sharing code between mark_dynptr_read and mark_iter_read for updating liveness information of their stack slots. Consolidate common logic into mark_stack_slot_obj_read function in preparation for the next patch which needs the same logic for its own stack slots. Acked-by: Eduard Zingerman Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 41b3dc1ce450..b4a486abe134 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -3191,10 +3191,27 @@ static int mark_reg_read(struct bpf_verifier_env *env, return 0; } -static int mark_dynptr_read(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +static int mark_stack_slot_obj_read(struct bpf_verifier_env *env, struct bpf_reg_state *reg, + int spi, int nr_slots) { struct bpf_func_state *state = func(env, reg); - int spi, ret; + int err, i; + + for (i = 0; i < nr_slots; i++) { + struct bpf_reg_state *st = &state->stack[spi - i].spilled_ptr; + + err = mark_reg_read(env, st, st->parent, REG_LIVE_READ64); + if (err) + return err; + + mark_stack_slot_scratched(env, spi - i); + } + return 0; +} + +static int mark_dynptr_read(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +{ + int spi; /* For CONST_PTR_TO_DYNPTR, it must have already been done by * check_reg_arg in check_helper_call and mark_btf_func_reg_size in @@ -3209,31 +3226,13 @@ static int mark_dynptr_read(struct bpf_verifier_env *env, struct bpf_reg_state * * bounds and spi is the first dynptr slot. Simply mark stack slot as * read. */ - ret = mark_reg_read(env, &state->stack[spi].spilled_ptr, - state->stack[spi].spilled_ptr.parent, REG_LIVE_READ64); - if (ret) - return ret; - return mark_reg_read(env, &state->stack[spi - 1].spilled_ptr, - state->stack[spi - 1].spilled_ptr.parent, REG_LIVE_READ64); + return mark_stack_slot_obj_read(env, reg, spi, BPF_DYNPTR_NR_SLOTS); } static int mark_iter_read(struct bpf_verifier_env *env, struct bpf_reg_state *reg, int spi, int nr_slots) { - struct bpf_func_state *state = func(env, reg); - int err, i; - - for (i = 0; i < nr_slots; i++) { - struct bpf_reg_state *st = &state->stack[spi - i].spilled_ptr; - - err = mark_reg_read(env, st, st->parent, REG_LIVE_READ64); - if (err) - return err; - - mark_stack_slot_scratched(env, spi - i); - } - - return 0; + return mark_stack_slot_obj_read(env, reg, spi, nr_slots); } /* This function is supposed to be used by the following 32-bit optimization From patchwork Wed Dec 4 03:03:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13893170 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 672F013BC0C for ; Wed, 4 Dec 2024 03:04:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.65 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281455; cv=none; b=a+t9E2sf9qF2q3Y1s+ihIdjKLGbmfl9vANR/t3KgfoIzbJt7M04yLsnIkof5zjBBu1VuJ7UxsZQzJ26F7ss6CuF0xub1AjJChAlLu5djj0/LmE1Bj1dg7xPdsfHcsCNSctU9nb1U3KlxKli9pzWDZXR49mwgW1ZL/zKt57nwxbo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281455; c=relaxed/simple; bh=SVCL5PUDSEhJx/76Q1jMElwxGkzrqVpZGRTjHUUekPE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=nWSP2YmfWz+70EY0ghwHVW7DHCRPLSXo4MXp1pl7cway+DTlo5UHKHUpgAjsS4q2k53zr1Hm4ktzuDTtZE3eXGF66QVup/wURNn3R5Dkyq0OolOHGG7MS/Lwi5/kJcgzF9VnFyFFN67zLuDqRHzrQLXlOsvXKHPB3Bu8ZBbOvm4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=fJOkBELh; arc=none smtp.client-ip=209.85.128.65 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="fJOkBELh" Received: by mail-wm1-f65.google.com with SMTP id 5b1f17b1804b1-434a2f3bae4so57995975e9.3 for ; Tue, 03 Dec 2024 19:04:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1733281451; x=1733886251; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ZIr8QeWAJgoPv7Sp0Yzqm4/KpOoG+gWOgiEMstn7X9k=; b=fJOkBELhQd7XunKyP9RVG3KRAS4q9OA8a2WwA7MW9Cttnwi3wXROlfd2AGuFGmvPG6 4UAT/3nHjKs38IFFcVMM7ltdoqozl9ZKzF4CXo918zwFdVCtSjb9cTjA3MXKPDFvi2cS 2+FdpZ/VhNtsMV7tbECuMT0gv8q3Kdq/arzCa1HuBmnZbZT9lnvTebRnrU8Dm45mbwXv oIzTbzyST0XISDd5UonQ7DxcGTZErclx/LtV68yrCmfgKnYv60qXiETWCUJ3Wien/Iu9 3ewS7umfJPy9qIBdU37h37KACMxGFlg1LazrI/kNwbHjLNiohj59HwaoUBIlGZb7cg5R xsTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733281451; x=1733886251; 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=ZIr8QeWAJgoPv7Sp0Yzqm4/KpOoG+gWOgiEMstn7X9k=; b=oRvGmDFQKBe9VoQlcI/KBJJ6MXYpjKogKvoOGOcTUBVHtx4kwcmFAXrVyDilcw2Umf jyNCsq7+VMSsDq20A36JWQ1UMilACmXLjWyMBcO2RQUHSttf3O+57gcBjmC3zioe1xU9 TeChWUy4x2KaO0bcsNTVudADo8/+93uzaw5vJZEZxLo6bF4r7zRo0GVdCTEGyHusE1jm bSPXtv7VrahUbHXcbaLLk/WtwpioH+hi7tauw3H26HBAi349H2mZ20lSNvyiv/f10srt oreuM5skIEnTi3/5DBrUUxGo1GRkBPdfUlVtIOjRFZ+6SjnSerxlo3vcu9BDfKYxDfPw dfJA== X-Gm-Message-State: AOJu0YytraSZq9DTudhtou2iIayeeTvDtiGlFkh+/noabAg9gCMFRzEX l50XNhVB5XOBxzQ8+erSERMUTWSK3qI/Nv9vHicBrvv0xCffFQqOxaNbn0G7sl4= X-Gm-Gg: ASbGncvdOSDYIioocrFwkYhIvKBtFEo4j1TVetQctyK0V5dbHgoE+yPB51DA2IgMtp4 Hqu1cPrgbRxEg7Y7PTMO71utjJcK9lTGzOy46N3kg9vQiU9kGlPIKb8gN0EP6Y3Ww7nUULQGs7O KpKe3rrlgIlMpxC4namLpoFOIZ2qy3qXAdQa5Slv1FSQMQkz6H+IAZiBhYWGkHGSi+uzXl9T8VK c0WBKOSDso7y2C4kqfD1S2FsTkMR/73nhtm6PpaJ+kc91dzXRW9usqpfqjWM8lCze1Xg97nEfaS X-Google-Smtp-Source: AGHT+IFXNKvZ72eqURdof4l/VsntuBkqk3I1DwU/Ps7OXLUXB2z2ASVfVT7EB2OjBzdfPfz1lLJS6Q== X-Received: by 2002:a05:600c:314a:b0:42c:b905:2bf9 with SMTP id 5b1f17b1804b1-434d09c3249mr44884265e9.16.1733281451061; Tue, 03 Dec 2024 19:04:11 -0800 (PST) Received: from localhost (fwdproxy-cln-002.fbsv.net. [2a03:2880:31ff:2::face:b00c]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-434d52cad51sm7599355e9.36.2024.12.03.19.04.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 19:04:10 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Eduard Zingerman , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , kernel-team@fb.com Subject: [PATCH bpf-next v6 4/7] bpf: Introduce support for bpf_local_irq_{save,restore} Date: Tue, 3 Dec 2024 19:03:57 -0800 Message-ID: <20241204030400.208005-5-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241204030400.208005-1-memxor@gmail.com> References: <20241204030400.208005-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=20520; h=from:subject; bh=SVCL5PUDSEhJx/76Q1jMElwxGkzrqVpZGRTjHUUekPE=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnT8Q/2KFzSiwwYAXaRt3z7U6lu9G3rEBPOtDKdN4c GbGqHc+JAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ0/EPwAKCRBM4MiGSL8RygR0D/ 4qKHM4U25kg3LBhsSkGMZ9Y/NT+RLEoNktaRWsbtJS3TBGqFb4MThqXZhAkkFN2CAYrViZV8EIDCF6 wSUbVTsvTD87mGPmB6vIZ0ODom0rjB8ygovGMgWk8VPrgRMRwsxZAORLCOMWblwmD2FBvJ8C7378xM 9QIjFQiv8dwN/jW/SC3IlHp1005qcvEWcP5uqO0KNDyLyJDl2GgEGoCarkvMX9arholvRvT+HGWXdk YelY/pGWDwe8Vj0ykuFt2MG7vVNIONW0UnjknpFEWUx+DME8VfqhgJji+r7Ay65u0VxpjqjWefb8ZA H1VMEXo6/t6zZXosoq7sCzrQOPMBzawBhmBQsH62EzxDC4gel51gHu9ck9l4x55bZV2H4rrckzF5dP h9f9DZ4ucdBmulzBQaFrBiwbwzj+zaZmU2MWdUv8uJWvboG+0yfY74UmJHwfCYl5JU4cQqY3pFcCEQ F+Rom8jpvptG3830D2PAXeUUNhxLo31cSiPB6TBuOuEKMUxfsdfSNfRE8qFkiXjIz4RP+gn90zYgla ePbUEdw5CsDyvXrMubXBqABWvpAoy8SKTP/ByjD6Yx+CKa0Cp5vxCb6NMf/4aPSU29A/bZyGWMuChh py4LQr3q1MrqYKnRAUV0BOsFSxX5sdxJWpXHcVDuhhIB5bvpzi6qp4aQhKuQ== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net Teach the verifier about IRQ-disabled sections through the introduction of two new kfuncs, bpf_local_irq_save, to save IRQ state and disable them, and bpf_local_irq_restore, to restore IRQ state and enable them back again. For the purposes of tracking the saved IRQ state, the verifier is taught about a new special object on the stack of type STACK_IRQ_FLAG. This is a 8 byte value which saves the IRQ flags which are to be passed back to the IRQ restore kfunc. Renumber the enums for REF_TYPE_* to simplify the check in find_lock_state, filtering out non-lock types as they grow will become cumbersome and is unecessary. To track a dynamic number of IRQ-disabled regions and their associated saved states, a new resource type RES_TYPE_IRQ is introduced, which its state management functions: acquire_irq_state and release_irq_state, taking advantage of the refactoring and clean ups made in earlier commits. One notable requirement of the kernel's IRQ save and restore API is that they cannot happen out of order. For this purpose, when releasing reference we keep track of the prev_id we saw with REF_TYPE_IRQ. Since reference states are inserted in increasing order of the index, this is used to remember the ordering of acquisitions of IRQ saved states, so that we maintain a logical stack in acquisition order of resource identities, and can enforce LIFO ordering when restoring IRQ state. The top of the stack is maintained using bpf_verifier_state's active_irq_id. To maintain the stack property when releasing reference states, we need to modify release_reference_state to instead shift the remaining array left using memmove instead of swapping deleted element with last that might break the ordering. A selftest to test this subtle behavior is added in late patches. The logic to detect initialized and unitialized irq flag slots, marking and unmarking is similar to how it's done for iterators. No additional checks are needed in refsafe for REF_TYPE_IRQ, apart from the usual check_id satisfiability check on the ref[i].id. We have to perform the same check_ids check on state->active_irq_id as well. To ensure we don't get assigned REF_TYPE_PTR by default after acquire_reference_state, if someone forgets to assign the type, let's also renumber the enum ref_state_type. This way any unassigned types get caught by refsafe's default switch statement, don't assume REF_TYPE_PTR by default. The kfuncs themselves are plain wrappers over local_irq_save and local_irq_restore macros. Acked-by: Eduard Zingerman Signed-off-by: Kumar Kartikeya Dwivedi --- include/linux/bpf_verifier.h | 7 +- kernel/bpf/helpers.c | 17 ++ kernel/bpf/log.c | 1 + kernel/bpf/verifier.c | 299 ++++++++++++++++++++++++++++++++++- 4 files changed, 320 insertions(+), 4 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 03e351c43fa8..de09ac3067ae 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -233,6 +233,7 @@ enum bpf_stack_slot_type { */ STACK_DYNPTR, STACK_ITER, + STACK_IRQ_FLAG, }; #define BPF_REG_SIZE 8 /* size of eBPF register in bytes */ @@ -254,8 +255,9 @@ struct bpf_reference_state { * default to pointer reference on zero initialization of a state. */ enum ref_state_type { - REF_TYPE_PTR = 0, - REF_TYPE_LOCK, + REF_TYPE_PTR = 1, + REF_TYPE_IRQ = 2, + REF_TYPE_LOCK = 3, } type; /* Track each reference created with a unique id, even if the same * instruction creates the reference multiple times (eg, via CALL). @@ -421,6 +423,7 @@ struct bpf_verifier_state { u32 acquired_refs; u32 active_locks; u32 active_preempt_locks; + u32 active_irq_id; bool active_rcu_lock; bool speculative; diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 751c150f9e1c..532ea74d4850 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -3057,6 +3057,21 @@ __bpf_kfunc int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void __user return ret + 1; } +/* Keep unsinged long in prototype so that kfunc is usable when emitted to + * vmlinux.h in BPF programs directly, but note that while in BPF prog, the + * unsigned long always points to 8-byte region on stack, the kernel may only + * read and write the 4-bytes on 32-bit. + */ +__bpf_kfunc void bpf_local_irq_save(unsigned long *flags__irq_flag) +{ + local_irq_save(*flags__irq_flag); +} + +__bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag) +{ + local_irq_restore(*flags__irq_flag); +} + __bpf_kfunc_end_defs(); BTF_KFUNCS_START(generic_btf_ids) @@ -3149,6 +3164,8 @@ BTF_ID_FLAGS(func, bpf_get_kmem_cache) BTF_ID_FLAGS(func, bpf_iter_kmem_cache_new, KF_ITER_NEW | KF_SLEEPABLE) BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLEEPABLE) BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | KF_SLEEPABLE) +BTF_ID_FLAGS(func, bpf_local_irq_save) +BTF_ID_FLAGS(func, bpf_local_irq_restore) BTF_KFUNCS_END(common_btf_ids) static const struct btf_kfunc_id_set common_kfunc_set = { diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c index 2d28ce926053..38050f4ee400 100644 --- a/kernel/bpf/log.c +++ b/kernel/bpf/log.c @@ -537,6 +537,7 @@ static char slot_type_char[] = { [STACK_ZERO] = '0', [STACK_DYNPTR] = 'd', [STACK_ITER] = 'i', + [STACK_IRQ_FLAG] = 'f' }; static void print_liveness(struct bpf_verifier_env *env, diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index b4a486abe134..b23f6fddf3af 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -661,6 +661,11 @@ static int iter_get_spi(struct bpf_verifier_env *env, struct bpf_reg_state *reg, return stack_slot_obj_get_spi(env, reg, "iter", nr_slots); } +static int irq_flag_get_spi(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +{ + return stack_slot_obj_get_spi(env, reg, "irq_flag", 1); +} + static enum bpf_dynptr_type arg_to_dynptr_type(enum bpf_arg_type arg_type) { switch (arg_type & DYNPTR_TYPE_FLAG_MASK) { @@ -1156,10 +1161,136 @@ static int is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_s return 0; } +static int acquire_irq_state(struct bpf_verifier_env *env, int insn_idx); +static int release_irq_state(struct bpf_verifier_state *state, int id); + +static int mark_stack_slot_irq_flag(struct bpf_verifier_env *env, + struct bpf_kfunc_call_arg_meta *meta, + struct bpf_reg_state *reg, int insn_idx) +{ + struct bpf_func_state *state = func(env, reg); + struct bpf_stack_state *slot; + struct bpf_reg_state *st; + int spi, i, id; + + spi = irq_flag_get_spi(env, reg); + if (spi < 0) + return spi; + + id = acquire_irq_state(env, insn_idx); + if (id < 0) + return id; + + slot = &state->stack[spi]; + st = &slot->spilled_ptr; + + __mark_reg_known_zero(st); + st->type = PTR_TO_STACK; /* we don't have dedicated reg type */ + st->live |= REG_LIVE_WRITTEN; + st->ref_obj_id = id; + + for (i = 0; i < BPF_REG_SIZE; i++) + slot->slot_type[i] = STACK_IRQ_FLAG; + + mark_stack_slot_scratched(env, spi); + return 0; +} + +static int unmark_stack_slot_irq_flag(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +{ + struct bpf_func_state *state = func(env, reg); + struct bpf_stack_state *slot; + struct bpf_reg_state *st; + int spi, i, err; + + spi = irq_flag_get_spi(env, reg); + if (spi < 0) + return spi; + + slot = &state->stack[spi]; + st = &slot->spilled_ptr; + + err = release_irq_state(env->cur_state, st->ref_obj_id); + WARN_ON_ONCE(err && err != -EACCES); + if (err) { + int insn_idx = 0; + + for (int i = 0; i < env->cur_state->acquired_refs; i++) { + if (env->cur_state->refs[i].id == env->cur_state->active_irq_id) { + insn_idx = env->cur_state->refs[i].insn_idx; + break; + } + } + + verbose(env, "cannot restore irq state out of order, expected id=%d acquired at insn_idx=%d\n", + env->cur_state->active_irq_id, insn_idx); + return err; + } + + __mark_reg_not_init(env, st); + + /* see unmark_stack_slots_dynptr() for why we need to set REG_LIVE_WRITTEN */ + st->live |= REG_LIVE_WRITTEN; + + for (i = 0; i < BPF_REG_SIZE; i++) + slot->slot_type[i] = STACK_INVALID; + + mark_stack_slot_scratched(env, spi); + return 0; +} + +static bool is_irq_flag_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +{ + struct bpf_func_state *state = func(env, reg); + struct bpf_stack_state *slot; + int spi, i; + + /* For -ERANGE (i.e. spi not falling into allocated stack slots), we + * will do check_mem_access to check and update stack bounds later, so + * return true for that case. + */ + spi = irq_flag_get_spi(env, reg); + if (spi == -ERANGE) + return true; + if (spi < 0) + return false; + + slot = &state->stack[spi]; + + for (i = 0; i < BPF_REG_SIZE; i++) + if (slot->slot_type[i] == STACK_IRQ_FLAG) + return false; + return true; +} + +static int is_irq_flag_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +{ + struct bpf_func_state *state = func(env, reg); + struct bpf_stack_state *slot; + struct bpf_reg_state *st; + int spi, i; + + spi = irq_flag_get_spi(env, reg); + if (spi < 0) + return -EINVAL; + + slot = &state->stack[spi]; + st = &slot->spilled_ptr; + + if (!st->ref_obj_id) + return -EINVAL; + + for (i = 0; i < BPF_REG_SIZE; i++) + if (slot->slot_type[i] != STACK_IRQ_FLAG) + return -EINVAL; + return 0; +} + /* Check if given stack slot is "special": * - spilled register state (STACK_SPILL); * - dynptr state (STACK_DYNPTR); * - iter state (STACK_ITER). + * - irq flag state (STACK_IRQ_FLAG) */ static bool is_stack_slot_special(const struct bpf_stack_state *stack) { @@ -1169,6 +1300,7 @@ static bool is_stack_slot_special(const struct bpf_stack_state *stack) case STACK_SPILL: case STACK_DYNPTR: case STACK_ITER: + case STACK_IRQ_FLAG: return true; case STACK_INVALID: case STACK_MISC: @@ -1291,6 +1423,7 @@ static int copy_reference_state(struct bpf_verifier_state *dst, const struct bpf dst->active_locks = src->active_locks; dst->active_preempt_locks = src->active_preempt_locks; dst->active_rcu_lock = src->active_rcu_lock; + dst->active_irq_id = src->active_irq_id; return 0; } @@ -1391,13 +1524,35 @@ static int acquire_lock_state(struct bpf_verifier_env *env, int insn_idx, enum r return 0; } +static int acquire_irq_state(struct bpf_verifier_env *env, int insn_idx) +{ + struct bpf_verifier_state *state = env->cur_state; + struct bpf_reference_state *s; + + s = acquire_reference_state(env, insn_idx); + if (!s) + return -ENOMEM; + s->type = REF_TYPE_IRQ; + s->id = ++env->id_gen; + + state->active_irq_id = s->id; + return s->id; +} + static void release_reference_state(struct bpf_verifier_state *state, int idx) { int last_idx; + size_t rem; + /* IRQ state requires the relative ordering of elements remaining the + * same, since it relies on the refs array to behave as a stack, so that + * it can detect out-of-order IRQ restore. Hence use memmove to shift + * the array instead of swapping the final element into the deleted idx. + */ last_idx = state->acquired_refs - 1; + rem = state->acquired_refs - idx - 1; if (last_idx && idx != last_idx) - memcpy(&state->refs[idx], &state->refs[last_idx], sizeof(*state->refs)); + memmove(&state->refs[idx], &state->refs[idx + 1], sizeof(*state->refs) * rem); memset(&state->refs[last_idx], 0, sizeof(*state->refs)); state->acquired_refs--; return; @@ -1419,6 +1574,28 @@ static int release_lock_state(struct bpf_verifier_state *state, int type, int id return -EINVAL; } +static int release_irq_state(struct bpf_verifier_state *state, int id) +{ + u32 prev_id = 0; + int i; + + if (id != state->active_irq_id) + return -EACCES; + + for (i = 0; i < state->acquired_refs; i++) { + if (state->refs[i].type != REF_TYPE_IRQ) + continue; + if (state->refs[i].id == id) { + release_reference_state(state, i); + state->active_irq_id = prev_id; + return 0; + } else { + prev_id = state->refs[i].id; + } + } + return -EINVAL; +} + static struct bpf_reference_state *find_lock_state(struct bpf_verifier_state *state, enum ref_state_type type, int id, void *ptr) { @@ -1427,7 +1604,7 @@ static struct bpf_reference_state *find_lock_state(struct bpf_verifier_state *st for (i = 0; i < state->acquired_refs; i++) { struct bpf_reference_state *s = &state->refs[i]; - if (s->type == REF_TYPE_PTR || s->type != type) + if (s->type != type) continue; if (s->id == id && s->ptr == ptr) @@ -3235,6 +3412,16 @@ static int mark_iter_read(struct bpf_verifier_env *env, struct bpf_reg_state *re return mark_stack_slot_obj_read(env, reg, spi, nr_slots); } +static int mark_irq_flag_read(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +{ + int spi; + + spi = irq_flag_get_spi(env, reg); + if (spi < 0) + return spi; + return mark_stack_slot_obj_read(env, reg, spi, 1); +} + /* This function is supposed to be used by the following 32-bit optimization * code only. It returns TRUE if the source or destination register operates * on 64-bit, otherwise return FALSE. @@ -10008,6 +10195,12 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, return -EINVAL; } + if (env->cur_state->active_irq_id) { + verbose(env, "global function calls are not allowed with IRQs disabled,\n" + "use static function instead\n"); + return -EINVAL; + } + if (err) { verbose(env, "Caller passes invalid args into func#%d ('%s')\n", subprog, sub_name); @@ -10532,6 +10725,11 @@ static int check_resource_leak(struct bpf_verifier_env *env, bool exception_exit return err; } + if (check_lock && env->cur_state->active_irq_id) { + verbose(env, "%s cannot be used inside bpf_local_irq_save-ed region\n", prefix); + return -EINVAL; + } + if (check_lock && env->cur_state->active_rcu_lock) { verbose(env, "%s cannot be used inside bpf_rcu_read_lock-ed region\n", prefix); return -EINVAL; @@ -10736,6 +10934,17 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn env->insn_aux_data[insn_idx].storage_get_func_atomic = true; } + if (env->cur_state->active_irq_id) { + if (fn->might_sleep) { + verbose(env, "sleepable helper %s#%d in IRQ-disabled region\n", + func_id_name(func_id), func_id); + return -EINVAL; + } + + if (in_sleepable(env) && is_storage_get_function(func_id)) + env->insn_aux_data[insn_idx].storage_get_func_atomic = true; + } + meta.func_id = func_id; /* check args */ for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { @@ -11297,6 +11506,11 @@ static bool is_kfunc_arg_const_str(const struct btf *btf, const struct btf_param return btf_param_match_suffix(btf, arg, "__str"); } +static bool is_kfunc_arg_irq_flag(const struct btf *btf, const struct btf_param *arg) +{ + return btf_param_match_suffix(btf, arg, "__irq_flag"); +} + static bool is_kfunc_arg_scalar_with_name(const struct btf *btf, const struct btf_param *arg, const char *name) @@ -11450,6 +11664,7 @@ enum kfunc_ptr_arg_type { KF_ARG_PTR_TO_CONST_STR, KF_ARG_PTR_TO_MAP, KF_ARG_PTR_TO_WORKQUEUE, + KF_ARG_PTR_TO_IRQ_FLAG, }; enum special_kfunc_type { @@ -11481,6 +11696,8 @@ enum special_kfunc_type { KF_bpf_iter_css_task_new, KF_bpf_session_cookie, KF_bpf_get_kmem_cache, + KF_bpf_local_irq_save, + KF_bpf_local_irq_restore, }; BTF_SET_START(special_kfunc_set) @@ -11547,6 +11764,8 @@ BTF_ID(func, bpf_session_cookie) BTF_ID_UNUSED #endif BTF_ID(func, bpf_get_kmem_cache) +BTF_ID(func, bpf_local_irq_save) +BTF_ID(func, bpf_local_irq_restore) static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta) { @@ -11637,6 +11856,9 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env, if (is_kfunc_arg_wq(meta->btf, &args[argno])) return KF_ARG_PTR_TO_WORKQUEUE; + if (is_kfunc_arg_irq_flag(meta->btf, &args[argno])) + return KF_ARG_PTR_TO_IRQ_FLAG; + if ((base_type(reg->type) == PTR_TO_BTF_ID || reg2btf_ids[base_type(reg->type)])) { if (!btf_type_is_struct(ref_t)) { verbose(env, "kernel function %s args#%d pointer type %s %s is not supported\n", @@ -11740,6 +11962,54 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env, return 0; } +static int process_irq_flag(struct bpf_verifier_env *env, int regno, + struct bpf_kfunc_call_arg_meta *meta) +{ + struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; + bool irq_save; + int err; + + if (meta->func_id == special_kfunc_list[KF_bpf_local_irq_save]) { + irq_save = true; + } else if (meta->func_id == special_kfunc_list[KF_bpf_local_irq_restore]) { + irq_save = false; + } else { + verbose(env, "verifier internal error: unknown irq flags kfunc\n"); + return -EFAULT; + } + + if (irq_save) { + if (!is_irq_flag_reg_valid_uninit(env, reg)) { + verbose(env, "expected uninitialized irq flag as arg#%d\n", regno - 1); + return -EINVAL; + } + + err = check_mem_access(env, env->insn_idx, regno, 0, BPF_DW, BPF_WRITE, -1, false, false); + if (err) + return err; + + err = mark_stack_slot_irq_flag(env, meta, reg, env->insn_idx); + if (err) + return err; + } else { + err = is_irq_flag_reg_valid_init(env, reg); + if (err) { + verbose(env, "expected an initialized irq flag as arg#%d\n", regno - 1); + return err; + } + + err = mark_irq_flag_read(env, reg); + if (err) + return err; + + err = unmark_stack_slot_irq_flag(env, reg); + if (err) + return err; + } + return 0; +} + + static int ref_set_non_owning(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { struct btf_record *rec = reg_btf_record(reg); @@ -12328,6 +12598,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ case KF_ARG_PTR_TO_REFCOUNTED_KPTR: case KF_ARG_PTR_TO_CONST_STR: case KF_ARG_PTR_TO_WORKQUEUE: + case KF_ARG_PTR_TO_IRQ_FLAG: break; default: WARN_ON_ONCE(1); @@ -12622,6 +12893,15 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ if (ret < 0) return ret; break; + case KF_ARG_PTR_TO_IRQ_FLAG: + if (reg->type != PTR_TO_STACK) { + verbose(env, "arg#%d doesn't point to an irq flag on stack\n", i); + return -EINVAL; + } + ret = process_irq_flag(env, regno, meta); + if (ret < 0) + return ret; + break; } } @@ -12802,6 +13082,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, return -EINVAL; } + if (env->cur_state->active_irq_id && sleepable) { + verbose(env, "kernel func %s is sleepable within IRQ-disabled region\n", func_name); + return -EACCES; + } + /* In case of release function, we get register number of refcounted * PTR_TO_BTF_ID in bpf_kfunc_arg_meta, do the release now. */ @@ -17735,6 +18020,12 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old, !check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap)) return false; break; + case STACK_IRQ_FLAG: + old_reg = &old->stack[spi].spilled_ptr; + cur_reg = &cur->stack[spi].spilled_ptr; + if (!check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap)) + return false; + break; case STACK_MISC: case STACK_ZERO: case STACK_INVALID: @@ -17764,12 +18055,16 @@ static bool refsafe(struct bpf_verifier_state *old, struct bpf_verifier_state *c if (old->active_rcu_lock != cur->active_rcu_lock) return false; + if (!check_ids(old->active_irq_id, cur->active_irq_id, idmap)) + return false; + for (i = 0; i < old->acquired_refs; i++) { if (!check_ids(old->refs[i].id, cur->refs[i].id, idmap) || old->refs[i].type != cur->refs[i].type) return false; switch (old->refs[i].type) { case REF_TYPE_PTR: + case REF_TYPE_IRQ: break; case REF_TYPE_LOCK: if (old->refs[i].ptr != cur->refs[i].ptr) From patchwork Wed Dec 4 03:03:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13893171 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7AF6F126C1E for ; Wed, 4 Dec 2024 03:04:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.66 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281456; cv=none; b=JDY0JEBsecfqurSuB0mV1OAOsms7p9AIDd9joZ2UOJdYDSD+UcITpYu0q9F1R2sC2bXyi9Hmym1n9w0xWg0YzQxW6MXG3PegDqFQGTILdjemR637EvkLqRt7NbvSfV/plceDBsyZ4OGgG3mo/YWxdU9+74i2hMShfnGa3QztOi8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281456; c=relaxed/simple; bh=jz4clmOncxLib17b2DjJqNcWHWzrENujw0X43qR3P6Q=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=pQiVHtbtpuK0K74clHU6R9coJ90R6ZzF6z+wmJe5pLMeDe9VWbKydrzuoBXTU6zsqPZPn7LPNEG4OmNZIcgLT+XhdEn3bZ4M1IYJqjPdOXbnfJwHysuoIorEClyS01Q92HPFOnk8krJMLQaL3Uca6iDUwhKoXvndr/FlMD5c2bg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ccbthAhD; arc=none smtp.client-ip=209.85.128.66 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ccbthAhD" Received: by mail-wm1-f66.google.com with SMTP id 5b1f17b1804b1-43497839b80so40478915e9.2 for ; Tue, 03 Dec 2024 19:04:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1733281452; x=1733886252; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=k5pGGEEG8fKxtViBtuHO4/boa5CwSA2mnjJe5gfkENU=; b=ccbthAhDCoGFftWcLHO2dCSH+smHfRmf2wIBuU6HsgVrAGG9scpPtaoruHhBfD+faX WvDsee3yoVsfzM+PYfjxCKuFtM7k2akZGe3IRdEjm0jwKT21GX1/9P/x8bqJV0/WqOL8 8ovuUML/cI+b1Aa50Twa1UIAz3f/o8dZd4GBhUNPUjnEh/fvf5XOb+6+f7Zl5Tjjf0Rm h5oLQ8qBPlvM71WRoa63Jir9B4xmEeBdE1PHWZWlyxHh4DAUKBa/E4V6pkfhDGNqvcur elY42EpAUu993QOE3fg4A4tlHYUJoI1B7k4AENdSXY25uMAGGL6KbGNyWX02ATcVTlUv 0hcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733281452; x=1733886252; 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=k5pGGEEG8fKxtViBtuHO4/boa5CwSA2mnjJe5gfkENU=; b=aPiOXVjUl9X5UYBPJafVCda9dHbajY/ksdDIH/sfKUDEKS0DWHxfAtmedpKmNubl8q tYOmyyVRB7GJrGh5UtJEMn5ghefVW/yES8LLPphiKpfE7HaUSl2KEqoLDHxOMWHVTIq/ bTtkdNgiLHD7XCA1Li1a6u2ZuG6hPmmNujtmYIZKNAGS7sfWym8iF1KD7ovI7e/ZRAKx ex0Y1e5N68IQrpQXvlquCSrs5uxOUphIvnG0HGYhcwDdZvEEK6Q496hFxV7+6fDYTOgM AilWbUf+TZNGoEmlAB9UxOFMh0r7cNBLMiD5RPw+ZHvgFTxVNTg9FYOF+q6BV//6fVen x6OQ== X-Gm-Message-State: AOJu0YzlAcx3QhQPWbPoL4f3v2Kohe+fa23KM7uwTYCaYO5uFIk5amU3 ztNfyjPb6zlnYDThJK5p8tx+0kxVmNw1qZV3qi4ako2fPJCZo4Qua7Ob3RxkBcw= X-Gm-Gg: ASbGnctS6NcDF0+T8FObRzZeh4dugKhKQWmPWfc6IHDQ52+Y/ozgdPxlwTaE1xUlZ0C aPT4M7qkQeKXyY8yNC3wl3O9Q8VMBXx6MRWF/J9qzTd+xCN1gk/L1esd+zbsYEE+sAdP7PlCapP ZhOAwjURoL4laeNO3Z+K9CyN73ZZ/H19sOCKJVcO5nJ10q+/0QtYZNlAnTvO43Bs5dkXs6vdj9s /voXnKbpn5kYXP6id2VipcJ2wN8G3aloJoTw5dE29LSEyPUXKdDyqBCNnKCGREQSQc7nnUgL9qe 7w== X-Google-Smtp-Source: AGHT+IGVWtvEbqorP4vFIVPK27ORZnGayXSbWdjVHPCD1MTMWV2v/158Sw9qG0ld7AHEOkWJoqm+NA== X-Received: by 2002:a05:600c:45ce:b0:431:60ec:7a91 with SMTP id 5b1f17b1804b1-434d4100a3fmr19972295e9.2.1733281452429; Tue, 03 Dec 2024 19:04:12 -0800 (PST) Received: from localhost (fwdproxy-cln-035.fbsv.net. [2a03:2880:31ff:23::face:b00c]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-434d52cbf57sm7586705e9.39.2024.12.03.19.04.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 19:04:11 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Eduard Zingerman , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , kernel-team@fb.com Subject: [PATCH bpf-next v6 5/7] bpf: Improve verifier log for resource leak on exit Date: Tue, 3 Dec 2024 19:03:58 -0800 Message-ID: <20241204030400.208005-6-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241204030400.208005-1-memxor@gmail.com> References: <20241204030400.208005-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=5835; h=from:subject; bh=jz4clmOncxLib17b2DjJqNcWHWzrENujw0X43qR3P6Q=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnT8Q/rN0VHGd1yxa35XMLltxvtxQW6a7hY/Xf7Fiy feHQ+D+JAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ0/EPwAKCRBM4MiGSL8RyuNgD/ 43J0WOlbjphdo+8jMqzwvEavSZXW4vczYA49OvqMLd0F8bXZGsp2SzHpClzp4jQKBRCoUW/Pxib6Up nJTSoa0D5YlMb/9RGJrTMyUt9a6xIaqPVV01QhkcFNTuWxtYaOROk2XvrgV6d9rBau7s9wc8dzJZN7 1oG4g3Sf9ZiihFDRHQXDci75vayATi/FDKrDp0UsxDL60UceR4DbhbWlEJiX0Pdi0sINbbZJq04bh3 Mj1P8+komOoAVyXz96PmhQLg2mYslSdBxO2Ye++7USDCgu7hfzQfkzfuk1e4uhJxAhXkB5sGuKT7KC A/SZR5uXiXZATvgfz9+jiAU9ksDkj++JUxyNtFh3wiVJOMQ1sGLQNLG9+pl24PVrVl1swnt8cXaA7f z1Ho0nMM+PaMyR5/Blaw71+Qy+4F2OqmSeetSnShnt+6zKAtSIB3+V7rXzjetcPGW9xR25fRLfsjND d6h1zTVmQgtokydU02IRxErj5FFjyNm3psCkuXC7BV/9xe/XVYL3ndckr10OlAysynXxIxEOXBBQFk sgOvRAKbqSubfxJiDBpJ0AHWnBr9Ne2WsETHPkE6yXQjq1T510cQyRssaQTY5e2M6Y1bkKRMs7dvLp Da0u4dvARqRfzFLqs+QtIkW2HNg+CUg0RBg7aq96UcMt/TtlfE9CgrlDPbwg== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net The verifier log when leaking resources on BPF_EXIT may be a bit confusing, as it's a problem only when finally existing from the main prog, not from any of the subprogs. Hence, update the verifier error string and the corresponding selftests matching on it. Acked-by: Eduard Zingerman Suggested-by: Eduard Zingerman Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 2 +- .../testing/selftests/bpf/progs/exceptions_fail.c | 4 ++-- tools/testing/selftests/bpf/progs/preempt_lock.c | 14 +++++++------- .../selftests/bpf/progs/verifier_spin_lock.c | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index b23f6fddf3af..31e0d33498ac 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -19102,7 +19102,7 @@ static int do_check(struct bpf_verifier_env *env) * match caller reference state when it exits. */ err = check_resource_leak(env, exception_exit, !env->cur_state->curframe, - "BPF_EXIT instruction"); + "BPF_EXIT instruction in main prog"); if (err) return err; diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c index fe0f3fa5aab6..8a0fdff89927 100644 --- a/tools/testing/selftests/bpf/progs/exceptions_fail.c +++ b/tools/testing/selftests/bpf/progs/exceptions_fail.c @@ -131,7 +131,7 @@ int reject_subprog_with_lock(void *ctx) } SEC("?tc") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_rcu_read_lock-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_rcu_read_lock-ed region") int reject_with_rcu_read_lock(void *ctx) { bpf_rcu_read_lock(); @@ -147,7 +147,7 @@ __noinline static int throwing_subprog(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_rcu_read_lock-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_rcu_read_lock-ed region") int reject_subprog_with_rcu_read_lock(void *ctx) { bpf_rcu_read_lock(); diff --git a/tools/testing/selftests/bpf/progs/preempt_lock.c b/tools/testing/selftests/bpf/progs/preempt_lock.c index 885377e83607..5269571cf7b5 100644 --- a/tools/testing/selftests/bpf/progs/preempt_lock.c +++ b/tools/testing/selftests/bpf/progs/preempt_lock.c @@ -6,7 +6,7 @@ #include "bpf_experimental.h" SEC("?tc") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_1(struct __sk_buff *ctx) { bpf_preempt_disable(); @@ -14,7 +14,7 @@ int preempt_lock_missing_1(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_2(struct __sk_buff *ctx) { bpf_preempt_disable(); @@ -23,7 +23,7 @@ int preempt_lock_missing_2(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_3(struct __sk_buff *ctx) { bpf_preempt_disable(); @@ -33,7 +33,7 @@ int preempt_lock_missing_3(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_3_minus_2(struct __sk_buff *ctx) { bpf_preempt_disable(); @@ -55,7 +55,7 @@ static __noinline void preempt_enable(void) } SEC("?tc") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_1_subprog(struct __sk_buff *ctx) { preempt_disable(); @@ -63,7 +63,7 @@ int preempt_lock_missing_1_subprog(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_2_subprog(struct __sk_buff *ctx) { preempt_disable(); @@ -72,7 +72,7 @@ int preempt_lock_missing_2_subprog(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_2_minus_1_subprog(struct __sk_buff *ctx) { preempt_disable(); diff --git a/tools/testing/selftests/bpf/progs/verifier_spin_lock.c b/tools/testing/selftests/bpf/progs/verifier_spin_lock.c index 3f679de73229..25599eac9a70 100644 --- a/tools/testing/selftests/bpf/progs/verifier_spin_lock.c +++ b/tools/testing/selftests/bpf/progs/verifier_spin_lock.c @@ -187,7 +187,7 @@ l0_%=: r6 = r0; \ SEC("cgroup/skb") __description("spin_lock: test6 missing unlock") -__failure __msg("BPF_EXIT instruction cannot be used inside bpf_spin_lock-ed region") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_spin_lock-ed region") __failure_unpriv __msg_unpriv("") __naked void spin_lock_test6_missing_unlock(void) { From patchwork Wed Dec 4 03:03:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13893172 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0C5087082C for ; Wed, 4 Dec 2024 03:04:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.67 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281457; cv=none; b=F4UKlLLgg176dWz029ZYWkw4np/JL7mkha9WUV55LrGDc9eXl3Bdnc/0ZCwEu6H/oiRPPbPjQT0XGyJ2ZaI1XdfCG3bnNECXuBkHOz7Dfj4T++v/TpYAYZXAn4+FI3Z3m04gOid/PYUSx08eEoiC3xXDFETtRHN2v4qcI0259PQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281457; c=relaxed/simple; bh=XI8afY+EEgzwCChq5uiRY5b/c1t5IWFqhWe47+0fWFE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=vEdRzJPWmaODnHWW4OO5ORrOvYKh/ip0ijmBo6Z3zycPu24h+9JotJomQcR4KH53a2IvL3gpDh6JsoCIDPpz2Qwc1ptbYRpM4AZ0j5ORuIe3WEJbULIm6H1vsbUeuX6GYo2vYY4WD5CpUqUhDy303zJZZRlUNXYUMc7tiTegpBE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ObMHtlGV; arc=none smtp.client-ip=209.85.128.67 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ObMHtlGV" Received: by mail-wm1-f67.google.com with SMTP id 5b1f17b1804b1-434ab938e37so40590025e9.0 for ; Tue, 03 Dec 2024 19:04:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1733281454; x=1733886254; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=O8hfXZNRSbjbJUCdqZD3pI8RxR9Vc0H89eeyYkgRzE0=; b=ObMHtlGVcjx/tt5j3Kq/7rNbjSb2xwk2jcmWjaHMpbf+nI3N6lywUt1VB3zadOlifO KoEpqcriNzvoIpBHws7vtGepuBqTqZkiJAjlqtaExpKra2RyWnUbUEwLGdnDSXSW3azg ancBexEdCl/hV3aN+oQZrl3D4C3Ci8ifKmSMdC49c9z61qmDfhnbl3up/rXbauRlQVJL rRzaHsogeODWfkiINN1N1u5iKXaDKAa1QrFd5KCTXY0sMOpxUE0TrGMqExREuPQL5aL3 K9i2mKpmq/GEqVgLrLJe+0jNx9yspJ+AddT4ppTqp+k+MSks1KEYAzEQv/FDh4nV9+b+ nb+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733281454; x=1733886254; 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=O8hfXZNRSbjbJUCdqZD3pI8RxR9Vc0H89eeyYkgRzE0=; b=S1jLg1X6+/qf7QkmUWOtgmCEaRnV/bjMnBQxsrDROZ/DCy4YVEGR+567Rtb9c5JoBp 5elsjjA54qmvk83vd1n659GXHwmgNfQxWEBzzLSJhDZcPt+D5jORhYGZN5VRJi/IheVV pzsVNOmK0oB6sI3k0YR+odDlTvaGf+ldyTXsX2hgs7MeSLfJwXUkmnbeqOv9MXqiAPU4 Ld4Zg3S5FRVBY8RFgmi+4slcuxdjaOtkU283KO2srfymMoZT0bXbADzuV5hguS4wOkY2 cfsvjU94ar4ZRx502KI7zozsx8ApINXTSp3IqrazkuekXEXiiqhMpcUYfT11UPQAJHwt bbaQ== X-Gm-Message-State: AOJu0YzRidksTIW5r3zE6MTKT8m8kHQNoy8Z/+XsCQ8+C6hJKc0gGny1 j9/0MPQOHEXVBHk0OQ2avehhk06YyVKJKSvgxUhi8wVxDUvcHcmLUCErWUfvFOQ= X-Gm-Gg: ASbGncuENMUgvrl89jks5Ho2+H2kfCPgQqElMtRQPwIZXCXC+aClUV4myrkSdXVnXXM lfyckN2M6hupY2BG+usOokdeszg7KCzoEOtvy3ZQOGdaDV9WgR45GLb1pEnitpfsA2HzT/2JmPT U+O0sE1XNnTk1w8lwVYny+56oFVgaK5n60rKNflq1DMOiw0lvfq34SYlthAsAeNC2qQo8XU0S+o nEX/sGee2y84phm9aAdzBjPWirY02lPaWAEUiQmK2cZpbDAFat1bV9UQCLTfAFjvqJEkG2eJfxX 0w== X-Google-Smtp-Source: AGHT+IF+jahHwKELrpEK6FGju8W8pyWR/1ghFMhvDKaznWWMW4G+4aSjbf+/cZIA7NG9kTKlxZUI8Q== X-Received: by 2002:a05:600c:35c2:b0:434:9499:9e87 with SMTP id 5b1f17b1804b1-434d3fcc5afmr19981465e9.25.1733281453568; Tue, 03 Dec 2024 19:04:13 -0800 (PST) Received: from localhost (fwdproxy-cln-112.fbsv.net. [2a03:2880:31ff:70::face:b00c]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-434d52cbf57sm7587145e9.39.2024.12.03.19.04.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 19:04:13 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Eduard Zingerman , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , kernel-team@fb.com Subject: [PATCH bpf-next v6 6/7] selftests/bpf: Expand coverage of preempt tests to sleepable kfunc Date: Tue, 3 Dec 2024 19:03:59 -0800 Message-ID: <20241204030400.208005-7-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241204030400.208005-1-memxor@gmail.com> References: <20241204030400.208005-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1596; h=from:subject; bh=XI8afY+EEgzwCChq5uiRY5b/c1t5IWFqhWe47+0fWFE=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnT8Q/ymtIJKVPVICIIc71lyz/NPShGAJs7VKCXfPI ybvJCliJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ0/EPwAKCRBM4MiGSL8RyiTmD/ 9fEowGzapceRmF0dS+FwqgD1UEEn9wnjyad0bqf9TSvkgo9LAGVDGOTltHbQ751jyMrgjRJhxFRwQh 1gEwv8EM2EuYOj3QVBQta/qwPUqZt4hf79yJnxFVn2Qe4FE7F3BNo1VINoX1OeoSOas49gxiU0vLvF yEqQwl1FAQfX+5LRBNGUN1fQIUog2yq2n983H+5VLjLoyvqhuZYcQ6UgiKaqstNoWzB4EWAnGnc3ug yWNmomVJw6k1UFKfsALGa5WcApCAhu9iyFdQj8GVRmjh52h2EaedqaBiGnmuB08wcvSdvWD7jiJwC4 O++5opcYSgnrWIGG7+srYGrNQLSeO++ltya/zfEU5O8d5opZ7/AntFL9NuDti0maVIaoidJE6ebLX7 REVakMxqD3n23pOEXBGS+7SONWBq1iaqyQc6MrGUrxfrmmwFTfpUTOYwgdFDCiEfHbKP7yNoYA9Fww /TFHTefLtPc+mnCUvOlo30o+YvW7FS+LMs5HvTAKMZKJ/t6IpNdBjmmRcExEvERiYCAWU+A/vC3Yg7 jc0Qk4oy8Xom4fheq9RXTZkFYKWQcK178mlD4gN4btAXLROwvCo5gRj/r+1BB0Conb5bjL3r+o6RvN CB+AJ4nRODeCBtPCinQkQp2ip5OLD8sLDOEHCtt5e+ju4gqU6rjWWQhPyHIw== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net For preemption-related kfuncs, we don't test their interaction with sleepable kfuncs (we do test helpers) even though the verifier has code to protect against such a pattern. Expand coverage of the selftest to include this case. Acked-by: Eduard Zingerman Signed-off-by: Kumar Kartikeya Dwivedi --- tools/testing/selftests/bpf/progs/preempt_lock.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/preempt_lock.c b/tools/testing/selftests/bpf/progs/preempt_lock.c index 5269571cf7b5..6c5797bf0ead 100644 --- a/tools/testing/selftests/bpf/progs/preempt_lock.c +++ b/tools/testing/selftests/bpf/progs/preempt_lock.c @@ -5,6 +5,8 @@ #include "bpf_misc.h" #include "bpf_experimental.h" +extern int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void *unsafe_ptr__ign, u64 flags) __weak __ksym; + SEC("?tc") __failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_1(struct __sk_buff *ctx) @@ -113,6 +115,18 @@ int preempt_sleepable_helper(void *ctx) return 0; } +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid") +__failure __msg("kernel func bpf_copy_from_user_str is sleepable within non-preemptible region") +int preempt_sleepable_kfunc(void *ctx) +{ + u32 data; + + bpf_preempt_disable(); + bpf_copy_from_user_str(&data, sizeof(data), NULL, 0); + bpf_preempt_enable(); + return 0; +} + int __noinline preempt_global_subprog(void) { preempt_balance_subprog(); From patchwork Wed Dec 4 03:04:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13893173 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BB9392CA9 for ; Wed, 4 Dec 2024 03:04:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.66 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281460; cv=none; b=GelenbTBQVa3LZ3DPm5UeSYPnwAGFxJf+xBJkUOwpGxcQ+mAJUZ/rxLfmtAIYCyI9boluoyHdlcC5TzfwVVo04agt2RnP69qV1IALawh+7hIZYfMGYVc5i6wE/g77tD+XDUFFjtvg88vQPtMf2/aumhNnUkn6cZFDxk9rV8InN4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733281460; c=relaxed/simple; bh=ehi26WvLtHDNqOg9c0fi4JyDWWJM2Ix5M04x2JY8xbA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=mV7XRvmNIIk6hGLuM08nQYDJMtGxske7F4EA8y6msY9BambfvDEKpQmYoV8ymAzzskniVKYsGFbnwW6Jc5CyL2SulwsWrVpUtdW7SDxUB0Yk3+Mqg8UiHbfkpMyLG4Aq2k7oIiUlSNgWD37LTLsLRWsi9oVYaHAhpMLLdNL/PhM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=mlTWnoph; arc=none smtp.client-ip=209.85.128.66 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="mlTWnoph" Received: by mail-wm1-f66.google.com with SMTP id 5b1f17b1804b1-4349e1467fbso55189365e9.1 for ; Tue, 03 Dec 2024 19:04:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1733281455; x=1733886255; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=HWQu++WV789z3AsgN8NSsS0Hi3vDX++q+Q3xLlXGnZo=; b=mlTWnoph7XT81LS0ulAQfcRmnC3XVS5dTcG7wqYnIKDFbyI0Yb27uZiVsFx754xDlr hYBXuCqc+FsK29uirx0LDSqpqhb9MT3s1I9miXhiXxYDPowpkebQpoXffTAnlDKrNNws +DAgWaFaNIl+UtwjMdtqPQpT3mg66OyclU0KpNasA9AY6oxo2n6g8dabCRkEgZmIeGhj NYTsIbTmzRx4LKXVJtGFcJRBaFzOeN/TuWkwIkj0Kz5GPpMh3CVDb9ih8E9drgCUXm+g Q6yR+u08D3Ffl8wcBRPQ2tUAGkJghpy+lmzIf9VqAN56yMLYi0dmvt7KZPT0jEwO0nZR D6Tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733281455; x=1733886255; 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=HWQu++WV789z3AsgN8NSsS0Hi3vDX++q+Q3xLlXGnZo=; b=QH7K9594rhK1ltB8wBs6bSfx5tXlHHj+NEf3t4RM0wyaFLzzT3Svxl40yQ2m6QChOv rXZ7xbkPtC8Zbp3bEmA3wKq8rKyB6WFR85pGCQyiShk1C6nKZJkI7UWkKwl7lIHU4QMO lOYyOXWG3Csx4He8AWTU4JR4QTLXm4Hxz7vtOKYtaLM3/h7rCFGOQVFU+3mbLkyFChgr nsdchhiGs75PQGtc9zU4tA1THxSoiAC5G5UuZYBtzLNebaFEXJWAAKj3JyXE/tIG6QlL Qrd8VMbPR7MOCiXNm4jmpTNlOLXqTrM0mT9nrLar4VqwKoBXEtawcdFkN0cjOnhHc86r T3Tw== X-Gm-Message-State: AOJu0Yw1XPt6qTCkFOYxhAeSMYuGRYWPMwF3L07aRRZRXV2V2Vix7/2H w4coFxgbxlucE5c5YPXGvRRMNeyKC8yjyZpv5SFVCs24ba78MkVBmQTqqhdp6MQ= X-Gm-Gg: ASbGncsMLWRKaVTiBsYCHJLCoYrqxPtl1CeIwKWbUpsUHwVIgQD1xrO9AP24g/5XJPO hfWAC9ucjTyr8iORDdbkGkrnBQUJTcTBj+W7ndZ29TmQQyedu5qW4PlAcXVN5k8ekhnZiB85VUa n9yW6asxs7cUcTvJgLNaNr311KGLQfuYbHEOSiPJnG1+oF8PTOTdEUbVg77Yy7c0G+nSt2vnLF2 W+++hPfa308ww9bmkEMX1PA2Pk8VamV9mAVktOtObq8LwJ6PvomrMjLJspdBzpVYkm9sD/89g== X-Google-Smtp-Source: AGHT+IH+PNCwVRG0IWCaVGRnuxpaZXi5rGLhK4EKo7flgBv0Lzzslzhgym3vLHqFvWu/Ig8CC/YXNA== X-Received: by 2002:a05:600c:1d86:b0:434:9e17:18e4 with SMTP id 5b1f17b1804b1-434d0905301mr43782735e9.0.1733281455095; Tue, 03 Dec 2024 19:04:15 -0800 (PST) Received: from localhost (fwdproxy-cln-000.fbsv.net. [2a03:2880:31ff::face:b00c]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-434d52cbd5asm7555655e9.40.2024.12.03.19.04.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 19:04:14 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Eduard Zingerman , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , kernel-team@fb.com Subject: [PATCH bpf-next v6 7/7] selftests/bpf: Add IRQ save/restore tests Date: Tue, 3 Dec 2024 19:04:00 -0800 Message-ID: <20241204030400.208005-8-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241204030400.208005-1-memxor@gmail.com> References: <20241204030400.208005-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=13870; h=from:subject; bh=ehi26WvLtHDNqOg9c0fi4JyDWWJM2Ix5M04x2JY8xbA=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnT8RAYvrbONJqwMsuBYoSS+qi19jI1kCZNnhDwUpT BCp423iJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ0/EQAAKCRBM4MiGSL8RygkoEA CPNqqx8Fp4bSDACiPUOrMSXYMvxvl4E8WIYz5wF9U9GZ6gQUfYPMJXrTemf9citG+TvKm06xlWrkAs CMkmlrTZH4M+MJkqgyjQzzoF3NlMb502sdsZTKuSTcb9EJTiNYv6LFTMIhor/qRMBc/bQulS0Mg4fx Fw88AOtRJ7vNBnfaa7w+O4ooRl5wSO2DAu24SiKhzYnyflZmTuTUxs0nmCYhnxxOo//TENfvDrF0ha WBwR8NH3K/IMJpTePaYJyrznI3Os71SwX9ivINgsdolclN+xJtwwmlt1XdhyAPaV7toDLGVhvIaSBy pymQzRcoJ0DTuvmjONOsH1KD98GIjjnHm+LcghlnmKMqz/X5YVzI0QI8AqriEqCfi75ki2T7SoVNjc NIG7z0wmyY+4k1AVRWSS2WfYM1UFEc0PTkpeAHKwSVDuzj6kpvfjmsYS1aGec7TJR+OFvAWq1HhjF1 4gFuMOBZPmDw+B2H6yhWI4sfioaNHXC5oDDN3uyqamoI+rViZlTbF5ZilsHcbRS010eeAKAsNu6Qp+ Em6AKaNWJgrL+rePNU0B2ApbzwPMR8KBJI5LgDImNFjIdiRYpdAi5addoJ1HO4iobTph46jp7XGH1M rZqp34a0f42oSdik5iTVHu73+UXQXaTh9qlgJs3dT0C+ZirCgOaOJenyQ6Uw== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net Include tests that check for rejection in erroneous cases, like unbalanced IRQ-disabled counts, within and across subprogs, invalid IRQ flag state or input to kfuncs, behavior upon overwriting IRQ saved state on stack, interaction with sleepable kfuncs/helpers, global functions, and out of order restore. Include some success scenarios as well to demonstrate usage. #128/1 irq/irq_save_bad_arg:OK #128/2 irq/irq_restore_bad_arg:OK #128/3 irq/irq_restore_missing_2:OK #128/4 irq/irq_restore_missing_3:OK #128/5 irq/irq_restore_missing_3_minus_2:OK #128/6 irq/irq_restore_missing_1_subprog:OK #128/7 irq/irq_restore_missing_2_subprog:OK #128/8 irq/irq_restore_missing_3_subprog:OK #128/9 irq/irq_restore_missing_3_minus_2_subprog:OK #128/10 irq/irq_balance:OK #128/11 irq/irq_balance_n:OK #128/12 irq/irq_balance_subprog:OK #128/13 irq/irq_global_subprog:OK #128/14 irq/irq_restore_ooo:OK #128/15 irq/irq_restore_ooo_3:OK #128/16 irq/irq_restore_3_subprog:OK #128/17 irq/irq_restore_4_subprog:OK #128/18 irq/irq_restore_ooo_3_subprog:OK #128/19 irq/irq_restore_invalid:OK #128/20 irq/irq_save_invalid:OK #128/21 irq/irq_restore_iter:OK #128/22 irq/irq_save_iter:OK #128/23 irq/irq_flag_overwrite:OK #128/24 irq/irq_flag_overwrite_partial:OK #128/25 irq/irq_ooo_refs_array:OK #128/26 irq/irq_sleepable_helper:OK #128/27 irq/irq_sleepable_kfunc:OK #128 irq:OK Summary: 1/27 PASSED, 0 SKIPPED, 0 FAILED Acked-by: Eduard Zingerman Signed-off-by: Kumar Kartikeya Dwivedi --- .../selftests/bpf/prog_tests/verifier.c | 2 + tools/testing/selftests/bpf/progs/irq.c | 444 ++++++++++++++++++ 2 files changed, 446 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/irq.c diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c index d9f65adb456b..b1b4d69c407a 100644 --- a/tools/testing/selftests/bpf/prog_tests/verifier.c +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c @@ -98,6 +98,7 @@ #include "verifier_xdp_direct_packet_access.skel.h" #include "verifier_bits_iter.skel.h" #include "verifier_lsm.skel.h" +#include "irq.skel.h" #define MAX_ENTRIES 11 @@ -225,6 +226,7 @@ void test_verifier_xdp(void) { RUN(verifier_xdp); } void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); } void test_verifier_bits_iter(void) { RUN(verifier_bits_iter); } void test_verifier_lsm(void) { RUN(verifier_lsm); } +void test_irq(void) { RUN(irq); } void test_verifier_mtu(void) { diff --git a/tools/testing/selftests/bpf/progs/irq.c b/tools/testing/selftests/bpf/progs/irq.c new file mode 100644 index 000000000000..b0b53d980964 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/irq.c @@ -0,0 +1,444 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */ +#include +#include +#include "bpf_misc.h" +#include "bpf_experimental.h" + +unsigned long global_flags; + +extern void bpf_local_irq_save(unsigned long *) __weak __ksym; +extern void bpf_local_irq_restore(unsigned long *) __weak __ksym; +extern int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void *unsafe_ptr__ign, u64 flags) __weak __ksym; + +SEC("?tc") +__failure __msg("arg#0 doesn't point to an irq flag on stack") +int irq_save_bad_arg(struct __sk_buff *ctx) +{ + bpf_local_irq_save(&global_flags); + return 0; +} + +SEC("?tc") +__failure __msg("arg#0 doesn't point to an irq flag on stack") +int irq_restore_bad_arg(struct __sk_buff *ctx) +{ + bpf_local_irq_restore(&global_flags); + return 0; +} + +SEC("?tc") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_local_irq_save-ed region") +int irq_restore_missing_2(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + + bpf_local_irq_save(&flags1); + bpf_local_irq_save(&flags2); + return 0; +} + +SEC("?tc") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_local_irq_save-ed region") +int irq_restore_missing_3(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + + bpf_local_irq_save(&flags1); + bpf_local_irq_save(&flags2); + bpf_local_irq_save(&flags3); + return 0; +} + +SEC("?tc") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_local_irq_save-ed region") +int irq_restore_missing_3_minus_2(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + + bpf_local_irq_save(&flags1); + bpf_local_irq_save(&flags2); + bpf_local_irq_save(&flags3); + bpf_local_irq_restore(&flags3); + bpf_local_irq_restore(&flags2); + return 0; +} + +static __noinline void local_irq_save(unsigned long *flags) +{ + bpf_local_irq_save(flags); +} + +static __noinline void local_irq_restore(unsigned long *flags) +{ + bpf_local_irq_restore(flags); +} + +SEC("?tc") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_local_irq_save-ed region") +int irq_restore_missing_1_subprog(struct __sk_buff *ctx) +{ + unsigned long flags; + + local_irq_save(&flags); + return 0; +} + +SEC("?tc") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_local_irq_save-ed region") +int irq_restore_missing_2_subprog(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + + local_irq_save(&flags1); + local_irq_save(&flags2); + return 0; +} + +SEC("?tc") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_local_irq_save-ed region") +int irq_restore_missing_3_subprog(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + + local_irq_save(&flags1); + local_irq_save(&flags2); + local_irq_save(&flags3); + return 0; +} + +SEC("?tc") +__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_local_irq_save-ed region") +int irq_restore_missing_3_minus_2_subprog(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + + local_irq_save(&flags1); + local_irq_save(&flags2); + local_irq_save(&flags3); + local_irq_restore(&flags3); + local_irq_restore(&flags2); + return 0; +} + +SEC("?tc") +__success +int irq_balance(struct __sk_buff *ctx) +{ + unsigned long flags; + + local_irq_save(&flags); + local_irq_restore(&flags); + return 0; +} + +SEC("?tc") +__success +int irq_balance_n(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + + local_irq_save(&flags1); + local_irq_save(&flags2); + local_irq_save(&flags3); + local_irq_restore(&flags3); + local_irq_restore(&flags2); + local_irq_restore(&flags1); + return 0; +} + +static __noinline void local_irq_balance(void) +{ + unsigned long flags; + + local_irq_save(&flags); + local_irq_restore(&flags); +} + +static __noinline void local_irq_balance_n(void) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + + local_irq_save(&flags1); + local_irq_save(&flags2); + local_irq_save(&flags3); + local_irq_restore(&flags3); + local_irq_restore(&flags2); + local_irq_restore(&flags1); +} + +SEC("?tc") +__success +int irq_balance_subprog(struct __sk_buff *ctx) +{ + local_irq_balance(); + return 0; +} + +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid") +__failure __msg("sleepable helper bpf_copy_from_user#") +int irq_sleepable_helper(void *ctx) +{ + unsigned long flags; + u32 data; + + local_irq_save(&flags); + bpf_copy_from_user(&data, sizeof(data), NULL); + local_irq_restore(&flags); + return 0; +} + +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid") +__failure __msg("kernel func bpf_copy_from_user_str is sleepable within IRQ-disabled region") +int irq_sleepable_kfunc(void *ctx) +{ + unsigned long flags; + u32 data; + + local_irq_save(&flags); + bpf_copy_from_user_str(&data, sizeof(data), NULL, 0); + local_irq_restore(&flags); + return 0; +} + +int __noinline global_local_irq_balance(void) +{ + local_irq_balance_n(); + return 0; +} + +SEC("?tc") +__failure __msg("global function calls are not allowed with IRQs disabled") +int irq_global_subprog(struct __sk_buff *ctx) +{ + unsigned long flags; + + bpf_local_irq_save(&flags); + global_local_irq_balance(); + bpf_local_irq_restore(&flags); + return 0; +} + +SEC("?tc") +__failure __msg("cannot restore irq state out of order") +int irq_restore_ooo(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + + bpf_local_irq_save(&flags1); + bpf_local_irq_save(&flags2); + bpf_local_irq_restore(&flags1); + bpf_local_irq_restore(&flags2); + return 0; +} + +SEC("?tc") +__failure __msg("cannot restore irq state out of order") +int irq_restore_ooo_3(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + + bpf_local_irq_save(&flags1); + bpf_local_irq_save(&flags2); + bpf_local_irq_restore(&flags2); + bpf_local_irq_save(&flags3); + bpf_local_irq_restore(&flags1); + bpf_local_irq_restore(&flags3); + return 0; +} + +static __noinline void local_irq_save_3(unsigned long *flags1, unsigned long *flags2, + unsigned long *flags3) +{ + local_irq_save(flags1); + local_irq_save(flags2); + local_irq_save(flags3); +} + +SEC("?tc") +__success +int irq_restore_3_subprog(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + + local_irq_save_3(&flags1, &flags2, &flags3); + bpf_local_irq_restore(&flags3); + bpf_local_irq_restore(&flags2); + bpf_local_irq_restore(&flags1); + return 0; +} + +SEC("?tc") +__failure __msg("cannot restore irq state out of order") +int irq_restore_4_subprog(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + unsigned long flags4; + + local_irq_save_3(&flags1, &flags2, &flags3); + bpf_local_irq_restore(&flags3); + bpf_local_irq_save(&flags4); + bpf_local_irq_restore(&flags4); + bpf_local_irq_restore(&flags1); + return 0; +} + +SEC("?tc") +__failure __msg("cannot restore irq state out of order") +int irq_restore_ooo_3_subprog(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags2; + unsigned long flags3; + + local_irq_save_3(&flags1, &flags2, &flags3); + bpf_local_irq_restore(&flags3); + bpf_local_irq_restore(&flags2); + bpf_local_irq_save(&flags3); + bpf_local_irq_restore(&flags1); + return 0; +} + +SEC("?tc") +__failure __msg("expected an initialized") +int irq_restore_invalid(struct __sk_buff *ctx) +{ + unsigned long flags1; + unsigned long flags = 0xfaceb00c; + + bpf_local_irq_save(&flags1); + bpf_local_irq_restore(&flags); + return 0; +} + +SEC("?tc") +__failure __msg("expected uninitialized") +int irq_save_invalid(struct __sk_buff *ctx) +{ + unsigned long flags1; + + bpf_local_irq_save(&flags1); + bpf_local_irq_save(&flags1); + return 0; +} + +SEC("?tc") +__failure __msg("expected an initialized") +int irq_restore_iter(struct __sk_buff *ctx) +{ + struct bpf_iter_num it; + + bpf_iter_num_new(&it, 0, 42); + bpf_local_irq_restore((unsigned long *)&it); + return 0; +} + +SEC("?tc") +__failure __msg("Unreleased reference id=1") +int irq_save_iter(struct __sk_buff *ctx) +{ + struct bpf_iter_num it; + + /* Ensure same sized slot has st->ref_obj_id set, so we reject based on + * slot_type != STACK_IRQ_FLAG... + */ + _Static_assert(sizeof(it) == sizeof(unsigned long), "broken iterator size"); + + bpf_iter_num_new(&it, 0, 42); + bpf_local_irq_save((unsigned long *)&it); + bpf_local_irq_restore((unsigned long *)&it); + return 0; +} + +SEC("?tc") +__failure __msg("expected an initialized") +int irq_flag_overwrite(struct __sk_buff *ctx) +{ + unsigned long flags; + + bpf_local_irq_save(&flags); + flags = 0xdeadbeef; + bpf_local_irq_restore(&flags); + return 0; +} + +SEC("?tc") +__failure __msg("expected an initialized") +int irq_flag_overwrite_partial(struct __sk_buff *ctx) +{ + unsigned long flags; + + bpf_local_irq_save(&flags); + *(((char *)&flags) + 1) = 0xff; + bpf_local_irq_restore(&flags); + return 0; +} + +SEC("?tc") +__failure __msg("cannot restore irq state out of order") +int irq_ooo_refs_array(struct __sk_buff *ctx) +{ + unsigned long flags[4]; + struct { int i; } *p; + + /* refs=1 */ + bpf_local_irq_save(&flags[0]); + + /* refs=1,2 */ + p = bpf_obj_new(typeof(*p)); + if (!p) { + bpf_local_irq_restore(&flags[0]); + return 0; + } + + /* refs=1,2,3 */ + bpf_local_irq_save(&flags[1]); + + /* refs=1,2,3,4 */ + bpf_local_irq_save(&flags[2]); + + /* Now when we remove ref=2, the verifier must not break the ordering in + * the refs array between 1,3,4. With an older implementation, the + * verifier would swap the last element with the removed element, but to + * maintain the stack property we need to use memmove. + */ + bpf_obj_drop(p); + + /* Save and restore to reset active_irq_id to 3, as the ordering is now + * refs=1,4,3. When restoring the linear scan will find prev_id in order + * as 3 instead of 4. + */ + bpf_local_irq_save(&flags[3]); + bpf_local_irq_restore(&flags[3]); + + /* With the incorrect implementation, we can release flags[1], flags[2], + * and flags[0], i.e. in the wrong order. + */ + bpf_local_irq_restore(&flags[1]); + bpf_local_irq_restore(&flags[2]); + bpf_local_irq_restore(&flags[0]); + return 0; +} + +char _license[] SEC("license") = "GPL";