From patchwork Fri Dec 13 22:19:27 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: 13907973 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) (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 C9E4C185949 for ; Fri, 13 Dec 2024 22:19:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.68 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734128380; cv=none; b=sjn8vBSrqyC7E7hYEs6Hq7rXtH0sCuCAA9Ve+Nn+C/YxlagfvGrtaPveX2zr2WXYKH0heb0UTNz4uV3nTcsG+/9hGi7RwB3qviDodqr2Jc6JRvXhT48VVcxBMklJV+/zjpR+paRzJVHkrMhiQL+0iRhZ7aG1cTNAoTYP05wShKw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734128380; c=relaxed/simple; bh=KEv755sEVd85j34CZrVOcnTJNLkMRBeR4Nr/xGtMmT4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=REcsUAlgTJE3RD1eVTf6P4la/EBEfOdEoFGhw6/LW3idLAHTWmjuzYK83UHOBfyP+h5PrdNuQNdj6RvOwNeOhncCQ+B0OzAZ1XT6xioaE9cqMJwuW7jVlXinNa2Wq35yGPycjWBZyXq9CeL5OPX60HEBAXMukPMJPjhytRTOHs0= 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=h4dwhnbw; arc=none smtp.client-ip=209.85.221.68 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="h4dwhnbw" Received: by mail-wr1-f68.google.com with SMTP id ffacd0b85a97d-385df53e559so1812242f8f.3 for ; Fri, 13 Dec 2024 14:19:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1734128376; x=1734733176; 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=spCXdJpgRnBemD1PVbI6UIUIfhJ5rtRztRc8Zw0sToo=; b=h4dwhnbwPH41Tq9lOB5C+7atwgXRgzNABRvGYKTujegHc0Zx66UDuAKWKLeTC6bRIG OOSvKcHgXzUSelZmec+RWBoAsR46mKbyHzHOQa0Q5Mt8lWAsJNqj/cW+N01UVBJZtRnk C23AeuHf+6GmVseV88ukw0NvT9zq3UyCGfqozl1gtrKBjDblfBEUUOltou8HkyUlYKcv 2MUVu3c9a0+AMP37HSspEorg7mmX1LbGOh2ianWXFZcXvN0VRW9KU274ncIwWaTBPvoG UGkV88RZ37Avj+MhL0n8IhYxlt2Q8CNcfW5LFwClvxFk6EWrBWt+vG/k19BsXxm0xaPn jZEA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734128376; x=1734733176; 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=spCXdJpgRnBemD1PVbI6UIUIfhJ5rtRztRc8Zw0sToo=; b=SV0/NuFc/BlhSC/FtI6agwgGvVzEKRtLCJjo9ru613Yqy6/H0dH1oZcFIVBH4IJaXo 39qG51dAdGiQj51OiuxWXfJOKtPtSgqe0KCVkTH/BOAfjkwVwhqj3DQhXTGawfitAazy TuOSUuntOGNHRjsgjiFeN1xuTWoLsB1YHrYJPmiO6q0ELhR3zb9Be3m8frNE1R6bSPuW gaoczmF900xDxFVeXerdY3jxxDt0MfVANWCjG0vato6/KMd15PKJyIV+RcEL6W6aO2gp j4WjMZlhTawFI2RSE9BL2oPeO51fc3prlbTy+3TTd9H2WKmc0Vp49ELhXrDqX1jg5UYI 0fkg== X-Gm-Message-State: AOJu0YypnGH7pwvlF19HBtOSqR7S00MbqxphIZgRM9A2WdzADED60myE BK08GtjLNduqOWL2ZhEhHcEyJup4tYDaBaGDNEmBBaU1C1kDTlwXXRWb9erohVUfGA== X-Gm-Gg: ASbGncv4nY1zVP/ykTa4yH2W9EdA9cQXMwjYLGiuXFzTIZlCqjwkzFyiO3ZJw/HQpIM xPQ4OTUi4NFxXz1nt0fDzevvp8MxBmEZ3JiA/DquK4B5mafjXjw2wf3znaxehFhyGKCxs5iBf1v xCjiVFxWqMGkzOgtshSZOpHlel4r+jGuLu0RGewJ5xsC4WFsReN92PXP+Y7rdMqWJUDbquU3csE KZ848NlymS4k3UvqeLvlYb/Cz4r49gbv+etkP3mLKwx+eCVu1qZGpBvsHmzt6Fo10SJe2meud1E zjfn X-Google-Smtp-Source: AGHT+IHty1oSxL3qSZllaGoL0SGowqGfkBo4X3+lTrCY0nd/iHPBogcDbKopZ2vKPV/hi/9DYdINIQ== X-Received: by 2002:a05:6000:144a:b0:385:f996:1bb9 with SMTP id ffacd0b85a97d-3888e0ae56dmr2978801f8f.23.1734128376137; Fri, 13 Dec 2024 14:19:36 -0800 (PST) Received: from localhost (fwdproxy-cln-000.fbsv.net. [2a03:2880:31ff::face:b00c]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4363602cc19sm5922425e9.13.2024.12.13.14.19.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Dec 2024 14:19:34 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Manu Bretelle , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Eduard Zingerman , Jiri Olsa , Juri Lelli , kernel-team@fb.com Subject: [PATCH bpf v3 1/3] bpf: Revert "bpf: Mark raw_tp arguments with PTR_MAYBE_NULL" Date: Fri, 13 Dec 2024 14:19:27 -0800 Message-ID: <20241213221929.3495062-2-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241213221929.3495062-1-memxor@gmail.com> References: <20241213221929.3495062-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=12835; h=from:subject; bh=KEv755sEVd85j34CZrVOcnTJNLkMRBeR4Nr/xGtMmT4=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnXK5lAac1/avgRyK3awTQWkJz/sZWTVpTL79hP84K y8a2oYeJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ1yuZQAKCRBM4MiGSL8Ryn1JD/ 9ddQKSiLry/HBAbhqKhaE3uMMk8sp62Xno6p2exM2wB78TGlwczTBr9UNmD0hzed+WwHcpBUHfnaok zCKBrjMf9aAO76SynpEAkpUxQ5wxwyWr5LX9hYn6ME6K+RiUFrcLuMCyD4Vuffz9ffKBjhA7gYqtvQ DRe7nIEgOGWIWDXy7TnIX7KEXSMlEiO0lym3KtvSSPmdkDSafukVSnL2zM9yEWj7RE1dpVWXmjoYdv R7sia2gXeI8bKCCGs0rCDXUABPFA8NWsel8xlOczK6L6Ety4bVkctpF2a2r9VNbjhiDPS/YWOPC5X4 QeBsOlBuL3OwLD42Bi+QLZy7KV+crbVb60eMCJMUvqvcj0BtEBMoUyfcDchq9KGMwMTUne8RmOssaa iIJfEE0oI4gU7mH08Z6Jh7rT7TskRUqefM+KdXru9zhOXXINvTs9k6kwT4qcaKFZpUH+rCxEmd2R48 Vtz1GvM9J3M/I8HlbCgs2rC4vyqBQ9f1QnGnbA5AzU0KA0O1D0oaHwOltcGE3dUWYosfYhj81t2qJx WjSA9neSPiqRx+EydElxu5yrsHc4xSs9TOHbYI/hUVvDQVFNYWlrAQApDZA2GCX7f2AZfBcF9pGLs5 lpr+9RIRG3FOR/uvOpwX/KHOc/WmFiz1+HNI4RE/xaZ//UsDEhLbgOfmThiA== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net This patch reverts commit cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL"). The patch was well-intended and meant to be as a stop-gap fixing branch prediction when the pointer may actually be NULL at runtime. Eventually, it was supposed to be replaced by an automated script or compiler pass detecting possibly NULL arguments and marking them accordingly. However, it caused two main issues observed for production programs and failed to preserve backwards compatibility. First, programs relied on the verifier not exploring == NULL branch when pointer is not NULL, thus they started failing with a 'dereference of scalar' error. Next, allowing raw_tp arguments to be modified surfaced the warning in the verifier that warns against reg->off when PTR_MAYBE_NULL is set. More information, context, and discusson on both problems is available in [0]. Overall, this approach had several shortcomings, and the fixes would further complicate the verifier's logic, and the entire masking scheme would have to be removed eventually anyway. Hence, revert the patch in preparation of a better fix avoiding these issues to replace this commit. [0]: https://lore.kernel.org/bpf/20241206161053.809580-1-memxor@gmail.com Reported-by: Manu Bretelle Fixes: cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL") Signed-off-by: Kumar Kartikeya Dwivedi --- include/linux/bpf.h | 6 -- kernel/bpf/btf.c | 5 +- kernel/bpf/verifier.c | 79 ++----------------- .../bpf/progs/test_tp_btf_nullable.c | 6 +- 4 files changed, 9 insertions(+), 87 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 805040813f5d..6e63dd3443b9 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -3514,10 +3514,4 @@ static inline bool bpf_is_subprog(const struct bpf_prog *prog) return prog->aux->func_idx != 0; } -static inline bool bpf_prog_is_raw_tp(const struct bpf_prog *prog) -{ - return prog->type == BPF_PROG_TYPE_TRACING && - prog->expected_attach_type == BPF_TRACE_RAW_TP; -} - #endif /* _LINUX_BPF_H */ diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index a63a03582f02..c4aa304028ce 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6594,10 +6594,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, if (prog_args_trusted(prog)) info->reg_type |= PTR_TRUSTED; - /* Raw tracepoint arguments always get marked as maybe NULL */ - if (bpf_prog_is_raw_tp(prog)) - info->reg_type |= PTR_MAYBE_NULL; - else if (btf_param_match_suffix(btf, &args[arg], "__nullable")) + if (btf_param_match_suffix(btf, &args[arg], "__nullable")) info->reg_type |= PTR_MAYBE_NULL; if (tgt_prog) { diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 5e541339b2f6..f7f892a52a37 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -420,25 +420,6 @@ static struct btf_record *reg_btf_record(const struct bpf_reg_state *reg) return rec; } -static bool mask_raw_tp_reg_cond(const struct bpf_verifier_env *env, struct bpf_reg_state *reg) { - return reg->type == (PTR_TO_BTF_ID | PTR_TRUSTED | PTR_MAYBE_NULL) && - bpf_prog_is_raw_tp(env->prog) && !reg->ref_obj_id; -} - -static bool mask_raw_tp_reg(const struct bpf_verifier_env *env, struct bpf_reg_state *reg) -{ - if (!mask_raw_tp_reg_cond(env, reg)) - return false; - reg->type &= ~PTR_MAYBE_NULL; - return true; -} - -static void unmask_raw_tp_reg(struct bpf_reg_state *reg, bool result) -{ - if (result) - reg->type |= PTR_MAYBE_NULL; -} - static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog) { struct bpf_func_info_aux *aux = env->prog->aux->func_info_aux; @@ -6801,7 +6782,6 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, const char *field_name = NULL; enum bpf_type_flag flag = 0; u32 btf_id = 0; - bool mask; int ret; if (!env->allow_ptr_leaks) { @@ -6873,21 +6853,7 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, if (ret < 0) return ret; - /* For raw_tp progs, we allow dereference of PTR_MAYBE_NULL - * trusted PTR_TO_BTF_ID, these are the ones that are possibly - * arguments to the raw_tp. Since internal checks in for trusted - * reg in check_ptr_to_btf_access would consider PTR_MAYBE_NULL - * modifier as problematic, mask it out temporarily for the - * check. Don't apply this to pointers with ref_obj_id > 0, as - * those won't be raw_tp args. - * - * We may end up applying this relaxation to other trusted - * PTR_TO_BTF_ID with maybe null flag, since we cannot - * distinguish PTR_MAYBE_NULL tagged for arguments vs normal - * tagging, but that should expand allowed behavior, and not - * cause regression for existing behavior. - */ - mask = mask_raw_tp_reg(env, reg); + if (ret != PTR_TO_BTF_ID) { /* just mark; */ @@ -6948,13 +6914,8 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, clear_trusted_flags(&flag); } - if (atype == BPF_READ && value_regno >= 0) { + if (atype == BPF_READ && value_regno >= 0) mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id, flag); - /* We've assigned a new type to regno, so don't undo masking. */ - if (regno == value_regno) - mask = false; - } - unmask_raw_tp_reg(reg, mask); return 0; } @@ -7329,7 +7290,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn if (!err && t == BPF_READ && value_regno >= 0) mark_reg_unknown(env, regs, value_regno); } else if (base_type(reg->type) == PTR_TO_BTF_ID && - (mask_raw_tp_reg_cond(env, reg) || !type_may_be_null(reg->type))) { + !type_may_be_null(reg->type)) { err = check_ptr_to_btf_access(env, regs, regno, off, size, t, value_regno); } else if (reg->type == CONST_PTR_TO_MAP) { @@ -9032,7 +8993,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, enum bpf_reg_type type = reg->type; u32 *arg_btf_id = NULL; int err = 0; - bool mask; if (arg_type == ARG_DONTCARE) return 0; @@ -9073,11 +9033,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, base_type(arg_type) == ARG_PTR_TO_SPIN_LOCK) arg_btf_id = fn->arg_btf_id[arg]; - mask = mask_raw_tp_reg(env, reg); err = check_reg_type(env, regno, arg_type, arg_btf_id, meta); + if (err) + return err; - err = err ?: check_func_arg_reg_off(env, reg, regno, arg_type); - unmask_raw_tp_reg(reg, mask); + err = check_func_arg_reg_off(env, reg, regno, arg_type); if (err) return err; @@ -9872,17 +9832,14 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, return ret; } else if (base_type(arg->arg_type) == ARG_PTR_TO_BTF_ID) { struct bpf_call_arg_meta meta; - bool mask; int err; if (register_is_null(reg) && type_may_be_null(arg->arg_type)) continue; memset(&meta, 0, sizeof(meta)); /* leave func_id as zero */ - mask = mask_raw_tp_reg(env, reg); err = check_reg_type(env, regno, arg->arg_type, &arg->btf_id, &meta); err = err ?: check_func_arg_reg_off(env, reg, regno, arg->arg_type); - unmask_raw_tp_reg(reg, mask); if (err) return err; } else { @@ -12205,7 +12162,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ enum bpf_arg_type arg_type = ARG_DONTCARE; u32 regno = i + 1, ref_id, type_size; bool is_ret_buf_sz = false; - bool mask = false; int kf_arg_type; t = btf_type_skip_modifiers(btf, args[i].type, NULL); @@ -12264,15 +12220,12 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ return -EINVAL; } - mask = mask_raw_tp_reg(env, reg); if ((is_kfunc_trusted_args(meta) || is_kfunc_rcu(meta)) && (register_is_null(reg) || type_may_be_null(reg->type)) && !is_kfunc_arg_nullable(meta->btf, &args[i])) { verbose(env, "Possibly NULL pointer passed to trusted arg%d\n", i); - unmask_raw_tp_reg(reg, mask); return -EACCES; } - unmask_raw_tp_reg(reg, mask); if (reg->ref_obj_id) { if (is_kfunc_release(meta) && meta->ref_obj_id) { @@ -12330,24 +12283,16 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ if (!is_kfunc_trusted_args(meta) && !is_kfunc_rcu(meta)) break; - /* Allow passing maybe NULL raw_tp arguments to - * kfuncs for compatibility. Don't apply this to - * arguments with ref_obj_id > 0. - */ - mask = mask_raw_tp_reg(env, reg); if (!is_trusted_reg(reg)) { if (!is_kfunc_rcu(meta)) { verbose(env, "R%d must be referenced or trusted\n", regno); - unmask_raw_tp_reg(reg, mask); return -EINVAL; } if (!is_rcu_reg(reg)) { verbose(env, "R%d must be a rcu pointer\n", regno); - unmask_raw_tp_reg(reg, mask); return -EINVAL; } } - unmask_raw_tp_reg(reg, mask); fallthrough; case KF_ARG_PTR_TO_CTX: case KF_ARG_PTR_TO_DYNPTR: @@ -12370,9 +12315,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ if (is_kfunc_release(meta) && reg->ref_obj_id) arg_type |= OBJ_RELEASE; - mask = mask_raw_tp_reg(env, reg); ret = check_func_arg_reg_off(env, reg, regno, arg_type); - unmask_raw_tp_reg(reg, mask); if (ret < 0) return ret; @@ -12549,7 +12492,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ ref_tname = btf_name_by_offset(btf, ref_t->name_off); fallthrough; case KF_ARG_PTR_TO_BTF_ID: - mask = mask_raw_tp_reg(env, reg); /* Only base_type is checked, further checks are done here */ if ((base_type(reg->type) != PTR_TO_BTF_ID || (bpf_type_has_unsafe_modifiers(reg->type) && !is_rcu_reg(reg))) && @@ -12558,11 +12500,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ verbose(env, "expected %s or socket\n", reg_type_str(env, base_type(reg->type) | (type_flag(reg->type) & BPF_REG_TRUSTED_MODIFIERS))); - unmask_raw_tp_reg(reg, mask); return -EINVAL; } ret = process_kf_arg_ptr_to_btf_id(env, reg, ref_t, ref_tname, ref_id, meta, i); - unmask_raw_tp_reg(reg, mask); if (ret < 0) return ret; break; @@ -13535,7 +13475,7 @@ static int sanitize_check_bounds(struct bpf_verifier_env *env, */ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, struct bpf_insn *insn, - struct bpf_reg_state *ptr_reg, + const struct bpf_reg_state *ptr_reg, const struct bpf_reg_state *off_reg) { struct bpf_verifier_state *vstate = env->cur_state; @@ -13549,7 +13489,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, struct bpf_sanitize_info info = {}; u8 opcode = BPF_OP(insn->code); u32 dst = insn->dst_reg; - bool mask; int ret; dst_reg = ®s[dst]; @@ -13576,14 +13515,11 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, return -EACCES; } - mask = mask_raw_tp_reg(env, ptr_reg); if (ptr_reg->type & PTR_MAYBE_NULL) { verbose(env, "R%d pointer arithmetic on %s prohibited, null-check it first\n", dst, reg_type_str(env, ptr_reg->type)); - unmask_raw_tp_reg(ptr_reg, mask); return -EACCES; } - unmask_raw_tp_reg(ptr_reg, mask); switch (base_type(ptr_reg->type)) { case PTR_TO_CTX: @@ -20126,7 +20062,6 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) * for this case. */ case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED: - case PTR_TO_BTF_ID | PTR_TRUSTED | PTR_MAYBE_NULL: if (type == BPF_READ) { if (BPF_MODE(insn->code) == BPF_MEM) insn->code = BPF_LDX | BPF_PROBE_MEM | diff --git a/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c b/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c index 5aaf2b065f86..bba3e37f749b 100644 --- a/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c +++ b/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c @@ -7,11 +7,7 @@ #include "bpf_misc.h" SEC("tp_btf/bpf_testmod_test_nullable_bare") -/* This used to be a failure test, but raw_tp nullable arguments can now - * directly be dereferenced, whether they have nullable annotation or not, - * and don't need to be explicitly checked. - */ -__success +__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'") int BPF_PROG(handle_tp_btf_nullable_bare1, struct bpf_testmod_test_read_ctx *nullable_ctx) { return nullable_ctx->len; From patchwork Fri Dec 13 22:19:28 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: 13907974 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 7AADE1A7AC7 for ; Fri, 13 Dec 2024 22:19:40 +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=1734128382; cv=none; b=EwTO4DYYvkk177FqWMp5mNTeAn3dY9N1KobSjnpBx8gGxioEONmoHXMZQs2mdOoc8ZKUEoiotrCPSEzjGRefBFtYHu8QtSt/h+2k9TvWLCJ/CA+gvXVJxhIg0GE3bEYcaNUDCH/TFtUhgZzPFS6APRFlVZf5EmE0CfeOb1hnGWM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734128382; c=relaxed/simple; bh=TOktLQI5dAYkR6Hyn62aVd3TlNwXf+iaDDCkArzju5w=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ilB+bGqJcoKbgEowMqD+ztotNCChfyyL61JLRC+cR37cu+mfALECQztPSXDzjHn9BsZaPrtzTy4J2SMvAD7UZ0TgWwt1J15cHIHAw8n3aL3/2DEdLXHyRyeLeHBDlNdtWgWUXoiag5gywIYtrw/HIWpfuRaJtm9aCMggRoJYjgw= 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=MnAeP8IM; 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="MnAeP8IM" Received: by mail-wm1-f67.google.com with SMTP id 5b1f17b1804b1-436202dd730so16341415e9.2 for ; Fri, 13 Dec 2024 14:19:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1734128378; x=1734733178; 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=2RVSSmUqL++a+EY7gKKS9MMxfzWGDgoy19gSXP/x8TE=; b=MnAeP8IMBr2+xAQVNgPN3uShgabERm7KaurznG3EW/k0r76BtBIzZsoc6MK6ty0HBl 2rsBA4lhseYAMcGiS8/Xc8IPz2r69nf1Os5m5RCiQ5yeIdR8DBzWu0MFl+/X6CdKwo1a souNAUx2fxVy7xsw4enLXXr6T/hb//uXDwoyX1TH4nCGFb0CHWcELyhNfhVbhvzLQuTK qqdzQdk0q9lnjToi+ChkVrRY2yVhdd7rUT6IkkjWrXvMKlLL02160fE6N9Wcyy0hHM3Z UspKS/gZqg12ryUU0wW1mAvQGm9tNnYth7kXtmizXDwIakBaaYNYDL7HlLVOU/K7plLb Mhiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734128378; x=1734733178; 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=2RVSSmUqL++a+EY7gKKS9MMxfzWGDgoy19gSXP/x8TE=; b=M7uoOHKas/YuUBkzbgwD1tOH19hDioorcmoOTET4f1L1Kcw+C8BEaWS0ig4idhFQ3Y wGpZ9aPKjqHp+KBgJ4Zjo9ZeRzg2sb4Q3h0KXR1+PzXerj8VXv7lEXBVuap8SzuA3Zys oDj+rLtddPf+WvBowdv2Vo5ZZuXMH5R7xkL38aGZ2SUbjCR77jLqoOS/qy83AGJ9yFkV avNsrN5dLH7DeP9U/ZIBUZ23eqLQDW9uIvc43KQQDaeSvP663DZypLQbMl7mDNk/lyOh ubs6QCS2hdxHao7cryB3Ze5mQ9+ndkUIbT8v53uVuvaiZkVGO+tnHjsJmJMY63Vn6usG MicA== X-Gm-Message-State: AOJu0YxrqX8bDFbhRqDb45jgIZ4GINJRXvFownMgRh1iwyttzy05oQ0d Wqn8l1IBqx7EU05jNK9hjVmuhqkusJb5nfcmqiR7+JsOwV7/YRXjPfgU6PssOLkJ7A== X-Gm-Gg: ASbGncsca9brYvuEJ6jlqC1UkjbQGyNazoOSXat1SG8WJYWEOww0+RtBw5Vy8yxJj3O QEKaks1h96VM5tg3FGnh1OA+y78HzV/O9hb3LiJ2LcSpke3tVzXrykb2btrgeGDRJWH1iUSjmM6 e3XrRmEgxnJZbHp4SURBsBXUG3jDFmT8eICMWL3GIJAdlbTOjpMSQoyOCF4VLvKIEJ5y9gZbQ35 LqJyFTup3fLl41yl7O88n45SHofWroZFeo4Xmhy6UUthkjOaqiSIFIchCrou6ast1bC7zmExgMN zL6DFi0= X-Google-Smtp-Source: AGHT+IFl47QStVKCNMLjvadnZP41lyUzcg5qCcsXaswT43EKi3RkcxtgCnDY7seEmfZ7Tk8weNMvmg== X-Received: by 2002:a5d:5f82:0:b0:385:fd31:ca34 with SMTP id ffacd0b85a97d-3888e0c06bbmr4072003f8f.54.1734128377763; Fri, 13 Dec 2024 14:19:37 -0800 (PST) Received: from localhost (fwdproxy-cln-014.fbsv.net. [2a03:2880:31ff:e::face:b00c]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-388c80165c5sm688738f8f.25.2024.12.13.14.19.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Dec 2024 14:19:36 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Juri Lelli , Manu Bretelle , Eduard Zingerman , Jiri Olsa , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , kernel-team@fb.com Subject: [PATCH bpf v3 2/3] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL Date: Fri, 13 Dec 2024 14:19:28 -0800 Message-ID: <20241213221929.3495062-3-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241213221929.3495062-1-memxor@gmail.com> References: <20241213221929.3495062-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=10747; h=from:subject; bh=TOktLQI5dAYkR6Hyn62aVd3TlNwXf+iaDDCkArzju5w=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnXK5lmnu19gVHJHqykjZOtgKaxwveQhbTv/vlCp4O 1j6KlYWJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ1yuZQAKCRBM4MiGSL8RygLHEA CACOTMmUWtMvA9ZOYDumbY5RTRXqcxQRqgXnCEi1f+4162lmLYP8pZP1ATuEWXrYZDL3FgwYxPen63 myb6Z8HCHTOPwqrYb85wi9md9sioCz2zQ+C+aZZVU6KKpCHB1R8k+grb1cdxohlQVfqsB/X/mY9UbE pv3U+rRu4kH/7WW+kA3qwy7Z7TuKX6j5WkblAx4VkOtxVdMNR5tvKWXgbKW3uGR+zxAYOvq9FZ2yLk aJAcRJ3Ox0q5Qr3ePy8RtXHMH9T21tAvgFDw6xQAZxJEE9XOB0DkAfNJ82bTLdCEpHxCvp3IsWEA9e 7YfnxP64EdH2HEfMa+kQlA0a0W7idTownYLn1CPn1G+bBSzN+XCBUHu6IlCKehZCoZXSJWbPuEyKHn 2VSjO5IzyFslWd8ldaKroik6yst0I87zR2C5eIG2wtd7rPr0jlYCZoWLDny2+4SxKYU4YdkcCcnfGf VWMHIPHESxg8GRjEaoA9wuV3SgcKni24EEdk3nE0FgUuohl0+q21yPXd6SFn7Nr9CJDzpeHrnRrKSG Dt2fruVxhDI8jKDJM8qnC/Yf4GgOJ59cxrKErrpdquy6Oqvgnq7qkpxndpSY8IdHc/KoxzXCP/xRBr MReWcfu0JwgqPEbULABNr+Xmnc5nJ65/cfw1ZAucAzQB2msKAt4BOmg61MDA== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net Arguments to a raw tracepoint are tagged as trusted, which carries the semantics that the pointer will be non-NULL. However, in certain cases, a raw tracepoint argument may end up being NULL. More context about this issue is available in [0]. Thus, there is a discrepancy between the reality, that raw_tp arguments can actually be NULL, and the verifier's knowledge, that they are never NULL, causing explicit NULL check branch to be dead code eliminated. A previous attempt [1], i.e. the second fixed commit, was made to simulate symbolic execution as if in most accesses, the argument is a non-NULL raw_tp, except for conditional jumps. This tried to suppress branch prediction while preserving compatibility, but surfaced issues with production programs that were difficult to solve without increasing verifier complexity. A more complete discussion of issues and fixes is available at [2]. Fix this by maintaining an explicit list of tracepoints where the arguments are known to be NULL, and mark the positional arguments as PTR_MAYBE_NULL. Additionally, capture the tracepoints where arguments are known to be ERR_PTR, and mark these arguments as scalar values to prevent potential dereference. Each hex digit is used to encode NULL-ness (0x1) or ERR_PTR-ness (0x2), shifted by the zero-indexed argument number x 4. This can be represented as follows: 1st arg: 0x1 2nd arg: 0x10 3rd arg: 0x100 ... and so on (likewise for ERR_PTR case). In the future, an automated pass will be used to produce such a list, or insert __nullable annotations automatically for tracepoints. Each compilation unit will be analyzed and results will be collated to find whether a tracepoint pointer is definitely not null, maybe null, or an unknown state where verifier conservatively marks it PTR_MAYBE_NULL. A proof of concept of this tool from Eduard is available at [3]. Note that in case we don't find a specification in the raw_tp_null_args array and the tracepoint belongs to a kernel module, we will conservatively mark the arguments as PTR_MAYBE_NULL. This is because unlike for in-tree modules, out-of-tree module tracepoints may pass NULL freely to the tracepoint. We don't protect against such tracepoints passing ERR_PTR (which is uncommon anyway), lest we mark all such arguments as SCALAR_VALUE. While we are it, let's adjust the test raw_tp_null to not perform dereference of the skb->mark, as that won't be allowed anymore, and make it more robust by using inline assembly to test the dead code elimination behavior, which should still stay the same. [0]: https://lore.kernel.org/bpf/ZrCZS6nisraEqehw@jlelli-thinkpadt14gen4.remote.csb [1]: https://lore.kernel.org/all/20241104171959.2938862-1-memxor@gmail.com [2]: https://lore.kernel.org/bpf/20241206161053.809580-1-memxor@gmail.com [3]: https://github.com/eddyz87/llvm-project/tree/nullness-for-tracepoint-params Reported-by: Juri Lelli # original bug Reported-by: Manu Bretelle # bugs in masking fix Fixes: 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs") Fixes: cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL") Reviewed-by: Eduard Zingerman Co-developed-by: Jiri Olsa Signed-off-by: Jiri Olsa Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/btf.c | 138 ++++++++++++++++++ .../testing/selftests/bpf/progs/raw_tp_null.c | 19 ++- 2 files changed, 147 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index c4aa304028ce..e5a5f023cedd 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6439,6 +6439,101 @@ int btf_ctx_arg_offset(const struct btf *btf, const struct btf_type *func_proto, return off; } +struct bpf_raw_tp_null_args { + const char *func; + u64 mask; +}; + +static const struct bpf_raw_tp_null_args raw_tp_null_args[] = { + /* sched */ + { "sched_pi_setprio", 0x10 }, + /* ... from sched_numa_pair_template event class */ + { "sched_stick_numa", 0x100 }, + { "sched_swap_numa", 0x100 }, + /* afs */ + { "afs_make_fs_call", 0x10 }, + { "afs_make_fs_calli", 0x10 }, + { "afs_make_fs_call1", 0x10 }, + { "afs_make_fs_call2", 0x10 }, + { "afs_protocol_error", 0x1 }, + { "afs_flock_ev", 0x10 }, + /* cachefiles */ + { "cachefiles_lookup", 0x1 | 0x200 }, + { "cachefiles_unlink", 0x1 }, + { "cachefiles_rename", 0x1 }, + { "cachefiles_prep_read", 0x1 }, + { "cachefiles_mark_active", 0x1 }, + { "cachefiles_mark_failed", 0x1 }, + { "cachefiles_mark_inactive", 0x1 }, + { "cachefiles_vfs_error", 0x1 }, + { "cachefiles_io_error", 0x1 }, + { "cachefiles_ondemand_open", 0x1 }, + { "cachefiles_ondemand_copen", 0x1 }, + { "cachefiles_ondemand_close", 0x1 }, + { "cachefiles_ondemand_read", 0x1 }, + { "cachefiles_ondemand_cread", 0x1 }, + { "cachefiles_ondemand_fd_write", 0x1 }, + { "cachefiles_ondemand_fd_release", 0x1 }, + /* ext4, from ext4__mballoc event class */ + { "ext4_mballoc_discard", 0x10 }, + { "ext4_mballoc_free", 0x10 }, + /* fib */ + { "fib_table_lookup", 0x100 }, + /* filelock */ + /* ... from filelock_lock event class */ + { "posix_lock_inode", 0x10 }, + { "fcntl_setlk", 0x10 }, + { "locks_remove_posix", 0x10 }, + { "flock_lock_inode", 0x10 }, + /* ... from filelock_lease event class */ + { "break_lease_noblock", 0x10 }, + { "break_lease_block", 0x10 }, + { "break_lease_unblock", 0x10 }, + { "generic_delete_lease", 0x10 }, + { "time_out_leases", 0x10 }, + /* host1x */ + { "host1x_cdma_push_gather", 0x10000 }, + /* huge_memory */ + { "mm_khugepaged_scan_pmd", 0x10 }, + { "mm_collapse_huge_page_isolate", 0x1 }, + { "mm_khugepaged_scan_file", 0x10 }, + { "mm_khugepaged_collapse_file", 0x10 }, + /* kmem */ + { "mm_page_alloc", 0x1 }, + { "mm_page_pcpu_drain", 0x1 }, + /* .. from mm_page event class */ + { "mm_page_alloc_zone_locked", 0x1 }, + /* netfs */ + { "netfs_failure", 0x10 }, + /* power */ + { "device_pm_callback_start", 0x10 }, + /* qdisc */ + { "qdisc_dequeue", 0x1000 }, + /* rxrpc */ + { "rxrpc_recvdata", 0x1 }, + { "rxrpc_resend", 0x10 }, + /* sunrpc */ + { "xs_stream_read_data", 0x1 }, + /* ... from xprt_cong_event event class */ + { "xprt_reserve_cong", 0x10 }, + { "xprt_release_cong", 0x10 }, + { "xprt_get_cong", 0x10 }, + { "xprt_put_cong", 0x10 }, + /* tcp */ + { "tcp_send_reset", 0x11 }, + /* tegra_apb_dma */ + { "tegra_dma_tx_status", 0x100 }, + /* timer_migration */ + { "tmigr_update_events", 0x1 }, + /* writeback, from writeback_folio_template event class */ + { "writeback_dirty_folio", 0x10 }, + { "folio_wait_writeback", 0x10 }, + /* rdma */ + { "mr_integ_alloc", 0x2000 }, + /* bpf_testmod */ + { "bpf_testmod_test_read", 0x0 }, +}; + bool btf_ctx_access(int off, int size, enum bpf_access_type type, const struct bpf_prog *prog, struct bpf_insn_access_aux *info) @@ -6449,6 +6544,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, const char *tname = prog->aux->attach_func_name; struct bpf_verifier_log *log = info->log; const struct btf_param *args; + bool ptr_err_raw_tp = false; const char *tag_value; u32 nr_args, arg; int i, ret; @@ -6597,6 +6693,39 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, if (btf_param_match_suffix(btf, &args[arg], "__nullable")) info->reg_type |= PTR_MAYBE_NULL; + if (prog->expected_attach_type == BPF_TRACE_RAW_TP) { + struct btf *btf = prog->aux->attach_btf; + const struct btf_type *t; + const char *tname; + + /* BTF lookups cannot fail, return false on error */ + t = btf_type_by_id(btf, prog->aux->attach_btf_id); + if (!t) + return false; + tname = btf_name_by_offset(btf, t->name_off); + if (!tname) + return false; + /* Checked by bpf_check_attach_target */ + tname += sizeof("btf_trace_") - 1; + for (i = 0; i < ARRAY_SIZE(raw_tp_null_args); i++) { + /* Is this a func with potential NULL args? */ + if (strcmp(tname, raw_tp_null_args[i].func)) + continue; + if (raw_tp_null_args[i].mask & (0x1 << (arg * 4))) + info->reg_type |= PTR_MAYBE_NULL; + /* Is the current arg IS_ERR? */ + if (raw_tp_null_args[i].mask & (0x2 << (arg * 4))) + ptr_err_raw_tp = true; + break; + } + /* If we don't know NULL-ness specification and the tracepoint + * is coming from a loadable module, be conservative and mark + * argument as PTR_MAYBE_NULL. + */ + if (i == ARRAY_SIZE(raw_tp_null_args) && btf_is_module(btf)) + info->reg_type |= PTR_MAYBE_NULL; + } + if (tgt_prog) { enum bpf_prog_type tgt_type; @@ -6641,6 +6770,15 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, bpf_log(log, "func '%s' arg%d has btf_id %d type %s '%s'\n", tname, arg, info->btf_id, btf_type_str(t), __btf_name_by_offset(btf, t->name_off)); + + /* Perform all checks on the validity of type for this argument, but if + * we know it can be IS_ERR at runtime, scrub pointer type and mark as + * scalar. + */ + if (ptr_err_raw_tp) { + bpf_log(log, "marking pointer arg%d as scalar as it may encode error", arg); + info->reg_type = SCALAR_VALUE; + } return true; } EXPORT_SYMBOL_GPL(btf_ctx_access); diff --git a/tools/testing/selftests/bpf/progs/raw_tp_null.c b/tools/testing/selftests/bpf/progs/raw_tp_null.c index 457f34c151e3..5927054b6dd9 100644 --- a/tools/testing/selftests/bpf/progs/raw_tp_null.c +++ b/tools/testing/selftests/bpf/progs/raw_tp_null.c @@ -3,6 +3,7 @@ #include #include +#include "bpf_misc.h" char _license[] SEC("license") = "GPL"; @@ -17,16 +18,14 @@ int BPF_PROG(test_raw_tp_null, struct sk_buff *skb) if (task->pid != tid) return 0; - i = i + skb->mark + 1; - /* The compiler may move the NULL check before this deref, which causes - * the load to fail as deref of scalar. Prevent that by using a barrier. + /* If dead code elimination kicks in, the increment +=2 will be + * removed. For raw_tp programs attaching to tracepoints in kernel + * modules, we mark input arguments as PTR_MAYBE_NULL, so branch + * prediction should never kick in. */ - barrier(); - /* If dead code elimination kicks in, the increment below will - * be removed. For raw_tp programs, we mark input arguments as - * PTR_MAYBE_NULL, so branch prediction should never kick in. - */ - if (!skb) - i += 2; + asm volatile ("%[i] += 1; if %[ctx] != 0 goto +1; %[i] += 2;" + : [i]"+r"(i) + : [ctx]"r"(skb) + : "memory"); return 0; } From patchwork Fri Dec 13 22:19:29 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: 13907975 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 E12AA185949 for ; Fri, 13 Dec 2024 22:19:41 +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=1734128383; cv=none; b=hLCSp0jxvmXO+o91bEemjtAR60ehgqExLNg8+OwvcVcOEkoGmpbbF59T8Re6i8veQJ+p9f+DRdqJiaTfp6XpJMJWFqfLRrB/Ha5BmYBqnbuhyNKT4h5gR78j2BKWuKTUNqizVHsgLdbaIvFPSddNFS+uLLZRZN8d5DqvxrBQdhs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734128383; c=relaxed/simple; bh=vsRKSkz/fZOFZmIfl7f9CCgSIMhv+zZmOd+PgYxJzDg=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=UCoEslXHKnmxYmZNrjQsjHYcnjdoNKXVD3uQCRzeop70JKiLtRR/GxlisKx4JmR8i9P6dntwqhCKy9AE/Uj/s5gyZqfSoTfADem0wCC5smxXqDhc64g91wuy8MUy6tnrFB5nFy0bm9June5wyhFBym4Ig4DL9UiavyL8Ja39I8o= 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=SomIOZlu; 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="SomIOZlu" Received: by mail-wr1-f65.google.com with SMTP id ffacd0b85a97d-3862d161947so1019278f8f.3 for ; Fri, 13 Dec 2024 14:19:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1734128379; x=1734733179; 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=j/xdfUQsKZfCf1D4VPdEjPDkdQjmt7uCtafMZN5T7vY=; b=SomIOZluaYr+nMUz7+Nd7SPyIHuZ1iojQfp2t8Hl/jm4uNCQeAE7NssRHXwK47hzpz yUZeEP4r3k40yV9KbbcqvCS3JII/FYZrIMChgakEsEEzHHjuPUu1hfbAZfTNzVuN7/Os vxx0WK9S0oqN2w6IioCKGcChh4hgAB4glsZjDGJodcSdICZAFl39twzKYn7IOMzgEY54 27NgdnEpDgP9YYXnOMznAeFgOTeWbgdY+Cr04dBS4+JLLCT1FY7hICVq7nmOQaNbr/DO U4R9s2HVkcgfoOOSrJ/HhTS17ClRtnjRXOjOOTgHBjevx33QvNFfv8z4jaHdi9hI85SR 9B1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734128379; x=1734733179; 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=j/xdfUQsKZfCf1D4VPdEjPDkdQjmt7uCtafMZN5T7vY=; b=D1M3qtaExxnYjvV+VGwbZPCv5iU2Qtcqss5TGdVbuRXq3ET8lGijGnihVnzOiI9Zjv yxDMzS0uYo1+jJ8cMOcTMhqgBnZ347o42SBK2dEFzyVjc/mGWjOpi5xxj+qMEb2CLUl9 6Ql7G7sV0OwJKHMLQoxtrHTEkz4EM/R0YOFf2PzvnPPIJISp3AIn+N6Fpdp6X4okHf/4 /dE0W2hsJCCIAp7AYrvbU/wlZ4S6DsJsq4ZFOV8mLdpxIqT9NFBKaVXkaHE0CsEUnjvO y72z8jhqZi4nOsj6PObmmVvnCRilXTwn7R59Tn6aZ/nQA/Yku/XaPGpNloG1O+keMaQY 0rsA== X-Gm-Message-State: AOJu0YwsFxgNG1YUv7qIMUVoS1V6bxjdeBWYHdeT3fEv4zDF088opZa4 I6kWgOPpGJOOMrV2arImW4NfpLBEuJBjbW9nBwqRX5tnVPEsaNN2eC0bszv1wCLVnA== X-Gm-Gg: ASbGncu+OSB2upO3wOwHOdC90YSWBH4xpl/iKREz3WMOrzvnHF90uzRh138i+XvsGYE VCOljSmYFcdC7N1eji9jrLEN1vcTNnKARD2UdNGngiX+p5ehmwGRlO+JWKYCoYbYSBZU+jHpVoq VA954gn7DvOpyLf/er5EtlY3t15YV6p5XZfYBE54VaIjrMl9tMpLuIKPAG3xtcz7hL/qBWUwyPa QrgPz+Tye88t95o0KbG96dfU0EYzAPxVS2nPw0DuanXG4llvM6yH9zve/EcMQH5WTnL6MMIxNq3 VXmo+09r X-Google-Smtp-Source: AGHT+IForGseAj7xBAzNzdqeCK55zAg1Hew9Q+kNtkV4XfRKCPkZiSXBw+b81MfRa8Yo9KWSuae5LQ== X-Received: by 2002:a05:6000:4709:b0:386:3803:bbd8 with SMTP id ffacd0b85a97d-3888e0c4d35mr3368674f8f.59.1734128379404; Fri, 13 Dec 2024 14:19:39 -0800 (PST) Received: from localhost (fwdproxy-cln-039.fbsv.net. [2a03:2880:31ff:27::face:b00c]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-388c801626asm690656f8f.29.2024.12.13.14.19.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Dec 2024 14:19:38 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Eduard Zingerman , Manu Bretelle , Jiri Olsa , Juri Lelli , kernel-team@fb.com Subject: [PATCH bpf v3 3/3] selftests/bpf: Add tests for raw_tp NULL args Date: Fri, 13 Dec 2024 14:19:29 -0800 Message-ID: <20241213221929.3495062-4-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241213221929.3495062-1-memxor@gmail.com> References: <20241213221929.3495062-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=2301; h=from:subject; bh=vsRKSkz/fZOFZmIfl7f9CCgSIMhv+zZmOd+PgYxJzDg=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnXK5lZKmTsj99EWTjLG1rmpgQTQk6XSjfFdqe8PQb tfxJShKJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZ1yuZQAKCRBM4MiGSL8RyoSFD/ 471ubGxUCUfNQbGbCwQjFdBgcF7duepdT+DGeQNi836aWhdCBV/3dwJE8XkiMZPKH606pn72YZICMx Dy5ggd19bY1dQMJMIIrS2LDvBKoSnzpZKP0ambuGtlHi4EuWGhIKChkPYJ47Ma7dHiVxCnV/Zcfasa jzYyzAdfAkXL8DWKL3VEaX8hGq7b5J7LAAvVEHOY0F0j6UGWMxvsds2A+FUyvXY6W8JJvjaz8xhsU3 gW/2hFYhjUsEAvy5MK3rxD+W17z04rNFa1rHLexnq4vxkyLl+acOq4vthymoVrEZ7p045qJMjIQN0k EusdVAXvDzCl1LyOSL8SCR9LKOKr1FdycyFuX2FD3wJrd+UAAzGGKmmzqQwpsYYDvCbvH5pfNn6YW1 B4+pGh3LhvanEiFh3gyWQE+9seXeuVSFl/P0NH/q9Q3ul7GGUjt3nAn8K+LwpFkRIRZ+nL0+3xwBlL Q3r6olRDbAjaEYp5BN0ncJ4+J5PcdSdSox8b0UIoXhtjLky7D3BUSgXwof8T6uyxLIm7M0EOX2NPnd fbctk+cXh1jtjv8kPdFMU/j+iAyDN34RF867wY8NtTwfqOZ9l7XO+CBiYcWwhcVjq9T4xRjAFJzSYK h9XPX/mbWrp8vi+xS+oeuoX1ODryhfJ0z16X+9PJ+GrfqQwixBtn2bZvARqg== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net Add tests to ensure that arguments are correctly marked based on their specified positions, and whether they get marked correctly as maybe null. For modules, all tracepoint parameters should be marked PTR_MAYBE_NULL by default. Signed-off-by: Kumar Kartikeya Dwivedi --- .../selftests/bpf/prog_tests/raw_tp_null.c | 3 +++ .../selftests/bpf/progs/raw_tp_null_fail.c | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/raw_tp_null_fail.c diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c index 6fa19449297e..43676a9922dc 100644 --- a/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c +++ b/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c @@ -3,11 +3,14 @@ #include #include "raw_tp_null.skel.h" +#include "raw_tp_null_fail.skel.h" void test_raw_tp_null(void) { struct raw_tp_null *skel; + RUN_TESTS(raw_tp_null_fail); + skel = raw_tp_null__open_and_load(); if (!ASSERT_OK_PTR(skel, "raw_tp_null__open_and_load")) return; diff --git a/tools/testing/selftests/bpf/progs/raw_tp_null_fail.c b/tools/testing/selftests/bpf/progs/raw_tp_null_fail.c new file mode 100644 index 000000000000..38d669957bf1 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/raw_tp_null_fail.c @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */ + +#include +#include +#include "bpf_misc.h" + +char _license[] SEC("license") = "GPL"; + +/* Ensure module parameter has PTR_MAYBE_NULL */ +SEC("tp_btf/bpf_testmod_test_raw_tp_null") +__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'") +int test_raw_tp_null_bpf_testmod_test_raw_tp_null_arg_1(void *ctx) { + asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all); + return 0; +} + +/* Check NULL marking */ +SEC("tp_btf/sched_pi_setprio") +__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'") +int test_raw_tp_null_sched_pi_setprio_arg_2(void *ctx) { + asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all); + return 0; +}