From patchwork Fri Jan 20 07:03:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109272 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4FB0C25B50 for ; Fri, 20 Jan 2023 07:04:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229453AbjATHEF (ORCPT ); Fri, 20 Jan 2023 02:04:05 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47862 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229437AbjATHEF (ORCPT ); Fri, 20 Jan 2023 02:04:05 -0500 Received: from mail-pg1-x543.google.com (mail-pg1-x543.google.com [IPv6:2607:f8b0:4864:20::543]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 43481683F8 for ; Thu, 19 Jan 2023 23:04:03 -0800 (PST) Received: by mail-pg1-x543.google.com with SMTP id 36so3422205pgp.10 for ; Thu, 19 Jan 2023 23:04:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=VaLkm+F26DdGfFgA0j+DUYDbXVbQ5FuBOQ58rlSnxCU=; b=CDPYdIkDNbtOh52S6seOMbfSHkCUj9SoWMug3lglu1a9V+ZhixqHw4shLqV2TjBJWm 4RyW2Tku2b+Q8YTXxmSlYhLvNOs+oXCFgwbkHSbl6w2v0WL3eHX8J3aPcbDpgzg/7Wzp L3o6euzjFFHpQZizOPaPve43VncUx2X43BfrmwiMP6n6qtGtqEZC2ui78O3X3eIX23Z+ ehgtIzZNiAXLgtfw4hk8ciJ4UFAXxNJAyKyl+anwaBy2G20/iurwG1tAWHmnD55+D2cT ofJHR7PyJfQbUFJeGHla6F50zX8EsgpwRGfAC9ja2/752uOsOZ21CrieheQRtKPNFY5M 3HOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=VaLkm+F26DdGfFgA0j+DUYDbXVbQ5FuBOQ58rlSnxCU=; b=lWPlyIBJPxgClf+zkme6KcKpYqTAtbJkGNP+angLrvfBfg/tXedyOCgRo6sTnLtieK wKXeUKsKQnxvpiYFKzdf3pKM6OZmV0S+xS8VX7x4uUWNgX9M8S8JBMHsI98CV3MSy0Cb kFtu6UxWBG8/09bvXawXhx5T1PMJOk1R2hZl9dNsCDYp7SqwoAcd7le6hci2sg3J7LCQ HAENx1kt3jmW6siuMJ8JtV79nc7h7Ezi+xEINnYfPZ8Vpxg8bKItPxEHw4z/Gjy0z3eX wlYyh5khZG8ptAH1RHS7TyrDtiJNJpCfsr6HoOibfd9nN+I8x22MAQpdlLuck+fwq4A4 z1LQ== X-Gm-Message-State: AFqh2ko46lgH3bFu0oQVPGT/mskJOElZKjW1B2orDs/tcgytoBCBEfUf 0rkgQOqAV9xMb4FHyEeIdwe7kR2gsSM= X-Google-Smtp-Source: AMrXdXv9255mmLSN3ArlqQAem19kx5w8zYBbLTK/KUBevFw5h2cPMnTVZfM74V9rzoXkPrEymaeeIA== X-Received: by 2002:aa7:96f7:0:b0:580:c223:90e9 with SMTP id i23-20020aa796f7000000b00580c22390e9mr15921725pfq.6.1674198242378; Thu, 19 Jan 2023 23:04:02 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id a6-20020aa79706000000b0058d9e915891sm9330402pfg.57.2023.01.19.23.04.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:01 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Eduard Zingerman , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v4 01/12] bpf: Fix state pruning for STACK_DYNPTR stack slots Date: Fri, 20 Jan 2023 12:33:44 +0530 Message-Id: <20230120070355.1983560-2-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9951; i=memxor@gmail.com; h=from:subject; bh=NYNbdfps9+kYMJ335GtKo6Xc0a43rfZyUOa/oudRbNc=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzLL83VYouCKa4i4kZhtgc+y/4A2p1iM42Byfm9 YpAPpFSJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8ywAKCRBM4MiGSL8RysUxD/ 48/vyP7/KB3g69npSdMaXOxec9SSj/6Tt4pL1dWI/Re7roadzapP6ztLyxPwtTVZvwZdd+uuLcdw1Q HOpx5nec79RjTlNvF7u1WdQ9sosfRZLSaZLLcsDWwQ1Fyw/aQaR5geCkxW8VS/K/EayRyRPhBWTqTx n4TYzYXo+biYZyKYJgCS08VrwMmMTJ0x6gTUNqgc0lTN2gf/NkRJVJPb0W2jmUFuJhEdNSmnhhwmJL cg2geJQfcSoXYdHYfKeup8Oq1s/k0diZx6++Ti3ceUV4XCYD7FTU8oavUxPIlU2ZCe4Zy8Y7MZD1tH ETAmoN6YtrgSEMevbCTdXK7kcqlWeF+9LYHvnNDI6ZsfJp6AOSu8xNibACWGALFusDP80a3XShKOY8 np65+gSkpK4Y4n3AJqYTPkjwTq2XOREodItsuCyc8Spw//qSV9cGq0ieMVxfliyyyhxg0yHLk177i1 F1DwyVqhHSY4Daggo5gOZVYzqSj2NST2NjNGEbJD/jENuKHizekH1diomXOFCKGDYP31O8YA+3QOEu TG9zlu2+kCWo1uobF2cnf000PYdlNzf4/ankqvW3eiwMbgE3jUQNh37xqvgSZ6DFX0ViOIZ/g7QKKB F9o6DhM71pKeMA8g1h6qLJOCuPwdAbuvg32qGT3RtQVfh0Bgu2Yr2ynBY5hQ== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The root of the problem is missing liveness marking for STACK_DYNPTR slots. This leads to all kinds of problems inside stacksafe. The verifier by default inside stacksafe ignores spilled_ptr in stack slots which do not have REG_LIVE_READ marks. Since this is being checked in the 'old' explored state, it must have already done clean_live_states for this old bpf_func_state. Hence, it won't be receiving any more liveness marks from to be explored insns (it has received REG_LIVE_DONE marking from liveness point of view). What this means is that verifier considers that it's safe to not compare the stack slot if was never read by children states. While liveness marks are usually propagated correctly following the parentage chain for spilled registers (SCALAR_VALUE and PTR_* types), the same is not the case for STACK_DYNPTR. clean_live_states hence simply rewrites these stack slots to the type STACK_INVALID since it sees no REG_LIVE_READ marks. The end result is that we will never see STACK_DYNPTR slots in explored state. Even if verifier was conservatively matching !REG_LIVE_READ slots, very next check continuing the stacksafe loop on seeing STACK_INVALID would again prevent further checks. Now as long as verifier stores an explored state which we can compare to when reaching a pruning point, we can abuse this bug to make verifier prune search for obviously unsafe paths using STACK_DYNPTR slots thinking they are never used hence safe. Doing this in unprivileged mode is a bit challenging. add_new_state is only set when seeing BPF_F_TEST_STATE_FREQ (which requires privileges) or when jmps_processed difference is >= 2 and insn_processed difference is >= 8. So coming up with the unprivileged case requires a little more work, but it is still totally possible. The test case being discussed below triggers the heuristic even in unprivileged mode. However, it no longer works since commit 8addbfc7b308 ("bpf: Gate dynptr API behind CAP_BPF"). Let's try to study the test step by step. Consider the following program (C style BPF ASM): 0 r0 = 0; 1 r6 = &ringbuf_map; 3 r1 = r6; 4 r2 = 8; 5 r3 = 0; 6 r4 = r10; 7 r4 -= -16; 8 call bpf_ringbuf_reserve_dynptr; 9 if r0 == 0 goto pc+1; 10 goto pc+1; 11 *(r10 - 16) = 0xeB9F; 12 r1 = r10; 13 r1 -= -16; 14 r2 = 0; 15 call bpf_ringbuf_discard_dynptr; 16 r0 = 0; 17 exit; We know that insn 12 will be a pruning point, hence if we force add_new_state for it, it will first verify the following path as safe in straight line exploration: 0 1 3 4 5 6 7 8 9 -> 10 -> (12) 13 14 15 16 17 Then, when we arrive at insn 12 from the following path: 0 1 3 4 5 6 7 8 9 -> 11 (12) We will find a state that has been verified as safe already at insn 12. Since register state is same at this point, regsafe will pass. Next, in stacksafe, for spi = 0 and spi = 1 (location of our dynptr) is skipped seeing !REG_LIVE_READ. The rest matches, so stacksafe returns true. Next, refsafe is also true as reference state is unchanged in both states. The states are considered equivalent and search is pruned. Hence, we are able to construct a dynptr with arbitrary contents and use the dynptr API to operate on this arbitrary pointer and arbitrary size + offset. To fix this, first define a mark_dynptr_read function that propagates liveness marks whenever a valid initialized dynptr is accessed by dynptr helpers. REG_LIVE_WRITTEN is marked whenever we initialize an uninitialized dynptr. This is done in mark_stack_slots_dynptr. It allows screening off mark_reg_read and not propagating marks upwards from that point. This ensures that we either set REG_LIVE_READ64 on both dynptr slots, or none, so clean_live_states either sets both slots to STACK_INVALID or none of them. This is the invariant the checks inside stacksafe rely on. Next, do a complete comparison of both stack slots whenever they have STACK_DYNPTR. Compare the dynptr type stored in the spilled_ptr, and also whether both form the same first_slot. Only then is the later path safe. Fixes: 97e03f521050 ("bpf: Add verifier support for dynptrs") Acked-by: Eduard Zingerman Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 88 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index ca7db2ce70b9..39d8ee38c338 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -781,6 +781,9 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ state->stack[spi - 1].spilled_ptr.ref_obj_id = id; } + state->stack[spi].spilled_ptr.live |= REG_LIVE_WRITTEN; + state->stack[spi - 1].spilled_ptr.live |= REG_LIVE_WRITTEN; + return 0; } @@ -805,6 +808,31 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re __mark_reg_not_init(env, &state->stack[spi].spilled_ptr); __mark_reg_not_init(env, &state->stack[spi - 1].spilled_ptr); + + /* Why do we need to set REG_LIVE_WRITTEN for STACK_INVALID slot? + * + * While we don't allow reading STACK_INVALID, it is still possible to + * do <8 byte writes marking some but not all slots as STACK_MISC. Then, + * helpers or insns can do partial read of that part without failing, + * but check_stack_range_initialized, check_stack_read_var_off, and + * check_stack_read_fixed_off will do mark_reg_read for all 8-bytes of + * the slot conservatively. Hence we need to prevent those liveness + * marking walks. + * + * This was not a problem before because STACK_INVALID is only set by + * default (where the default reg state has its reg->parent as NULL), or + * in clean_live_states after REG_LIVE_DONE (at which point + * mark_reg_read won't walk reg->parent chain), but not randomly during + * verifier state exploration (like we did above). Hence, for our case + * parentage chain will still be live (i.e. reg->parent may be + * non-NULL), while earlier reg->parent was NULL, so we need + * REG_LIVE_WRITTEN to screen off read marker propagation when it is + * done later on reads or by mark_dynptr_read as well to unnecessary + * mark registers in verifier state. + */ + state->stack[spi].spilled_ptr.live |= REG_LIVE_WRITTEN; + state->stack[spi - 1].spilled_ptr.live |= REG_LIVE_WRITTEN; + return 0; } @@ -2390,6 +2418,30 @@ 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) +{ + struct bpf_func_state *state = func(env, reg); + int spi, ret; + + /* 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 + * check_kfunc_call. + */ + if (reg->type == CONST_PTR_TO_DYNPTR) + return 0; + spi = get_spi(reg->off); + /* Caller ensures dynptr is valid and initialized, which means spi is in + * 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); +} + /* 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. @@ -5977,6 +6029,8 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, meta->uninit_dynptr_regno = regno; } else /* MEM_RDONLY and None case from above */ { + int err; + /* For the reg->type == PTR_TO_STACK case, bpf_dynptr is never const */ if (reg->type == CONST_PTR_TO_DYNPTR && !(arg_type & MEM_RDONLY)) { verbose(env, "cannot pass pointer to const bpf_dynptr, the helper mutates it\n"); @@ -6010,6 +6064,10 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, err_extra, regno); return -EINVAL; } + + err = mark_dynptr_read(env, reg); + if (err) + return err; } return 0; } @@ -13215,10 +13273,9 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old, return false; if (i % BPF_REG_SIZE != BPF_REG_SIZE - 1) continue; - if (!is_spilled_reg(&old->stack[spi])) - continue; - if (!regsafe(env, &old->stack[spi].spilled_ptr, - &cur->stack[spi].spilled_ptr, idmap)) + /* Both old and cur are having same slot_type */ + switch (old->stack[spi].slot_type[BPF_REG_SIZE - 1]) { + case STACK_SPILL: /* when explored and current stack slot are both storing * spilled registers, check that stored pointers types * are the same as well. @@ -13229,7 +13286,30 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old, * such verifier states are not equivalent. * return false to continue verification of this path */ + if (!regsafe(env, &old->stack[spi].spilled_ptr, + &cur->stack[spi].spilled_ptr, idmap)) + return false; + break; + case STACK_DYNPTR: + { + const struct bpf_reg_state *old_reg, *cur_reg; + + old_reg = &old->stack[spi].spilled_ptr; + cur_reg = &cur->stack[spi].spilled_ptr; + if (old_reg->dynptr.type != cur_reg->dynptr.type || + old_reg->dynptr.first_slot != cur_reg->dynptr.first_slot || + !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: + continue; + /* Ensure that new unhandled slot types return false by default */ + default: return false; + } } return true; } From patchwork Fri Jan 20 07:03:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109273 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF192C05027 for ; Fri, 20 Jan 2023 07:04:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229721AbjATHEI (ORCPT ); Fri, 20 Jan 2023 02:04:08 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47892 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229437AbjATHEI (ORCPT ); Fri, 20 Jan 2023 02:04:08 -0500 Received: from mail-pl1-x644.google.com (mail-pl1-x644.google.com [IPv6:2607:f8b0:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C8E41530D7 for ; Thu, 19 Jan 2023 23:04:06 -0800 (PST) Received: by mail-pl1-x644.google.com with SMTP id v23so4602079plo.1 for ; Thu, 19 Jan 2023 23:04:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=tbfmok1bo7nEVSKUA385ZnbR9z6ipfMuo+sDIvn1aHs=; b=Y01TAT+fsEZCUp0QP066V2t97AUQa+L9o3hed4/bOFogBGI1B8wxs0izCfUK7buSkH Be8uZGTdCRsGOesYTRJCy7kDrUBf2xWM9Uq9QoRD+B3n7jYq6CR/q4RbSikl3ORQ7zdW ca6xKu7kIJ/+cy9QZcIYI9JFgbpdNok/IoIZAXn+j9HgsZDGU93Zwh/xlpjVDz6coqUN 7ELRKOAuFuBi4SKPwyJg0hAzNFi6hKboCQYh4q2ywzOF7b48vmnrAwX7r7NLPyMi6QR7 /wFM4iKrgOolEHOZI79k/LYtZMtOUWRlRuElvQ67J+/66i27l89u4YVX4ltVslLyIxw4 Fcdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=tbfmok1bo7nEVSKUA385ZnbR9z6ipfMuo+sDIvn1aHs=; b=KhWkDLsFstHBUfWv0G1mGr2Y0vMEm6A8fbwQNF8ryiN8J6POvUzB4W0K6yBi9wVzLz i1qb3Ag718ErPqpWX1goEtphTP6XOKwIPHMMXuZGw0/P2yJAgim94qth2YM+J78haeqq cHAnaFSdVaciv4d+enDLH/OXfzKnJfn3XhG0V7c71w7Sol2WLHcA1/X3+p7V9Qyr1Xsv nVUXyMVGcHHwJUXKAs95XjO/FcYDsGicdL/GCXz1yLMfX+LTJ9kdKZIlN+CNoRJI/ePi LSrCsUa2z3A1G7ujb9c2/MhsXCBmb8q0Dt5N1DsmyjuGTT09zbwSU3LLREZ6rLewjDpQ zFEw== X-Gm-Message-State: AFqh2koVHkGZBl7CfwcGeWeu0k3zJclWeFl0yfKX3fc+Y9m1KUFQv37+ 1Zqd3jR/xY2ETe5gylb5s9G2XoKst0s= X-Google-Smtp-Source: AMrXdXt1Tf7co94fsdBz3vipvUTPOq8UQpaYUdP77A96FSWeDkQ0obruzTuVVEt5YtmuZvLyP6DShg== X-Received: by 2002:a17:903:2448:b0:192:7845:e0cc with SMTP id l8-20020a170903244800b001927845e0ccmr51920031pls.68.1674198245984; Thu, 19 Jan 2023 23:04:05 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id y7-20020a1709027c8700b00194afb5a3ebsm5825494pll.21.2023.01.19.23.04.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:05 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Joanne Koong , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 02/12] bpf: Fix missing var_off check for ARG_PTR_TO_DYNPTR Date: Fri, 20 Jan 2023 12:33:45 +0530 Message-Id: <20230120070355.1983560-3-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9553; i=memxor@gmail.com; h=from:subject; bh=2s1pBDiiXTIV1BB1rUUx54yULFT49RyHEefJ0HCSxfo=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzLl2cE3exe3tS2y0dU5xdJr1Vgd0s+vbl8nS4L rp+xG4SJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8ywAKCRBM4MiGSL8RyhEmD/ 4/tVINO6aN/REFg+2Z/nfDzGJieIGOijMSHvzDYU7S/hdkUlz0nln5VwbrfqnUANCEBfFHJSXMm9Lm +9l+DicbC+uydPi/Vhu+Ye9aP3MQp1YHyrOlYVGSHMorxm0WwMme9dxWpW/yxtSf6WBQ/3tsOl7FMq PVTpJr8nGsuM8Tgia3xb0/vw5fh2EkTLWs+W9QdrlyGFkI0jFsTnhIJzoFxdSw8e63Doz//0ykPdm6 NnLftu2mH02oOD4trJgs+hml2tA4Yqvgq2xjmD7MChsb+s7IxYWsqw0rjoNTkxLXwgJjBqa25fhYq9 oir25Jku2hKg6Ivs8y/+yqLrdc1Y9EVFHZs9DaOTq6WP2Vm2EiofknoQxkKW3PyVFWxD0em0NOzGoF wgaD/g4YRNb+IYVZ0ivAeLYAYt2I6zEwJZ46GbqyqYy09a1/pCiNPODcdz3fPyQpEghpzpPr7irzj7 JtekBdRxl8xqJmO/3+Yg/0KC/o3kUJsirKXudXg3cMMretuqJWjl88GGiBbv1NKN9u4M+JT6wKpner XoNgDLBmEUxVe9FA+c7qyFPbRnFn6991rGfIZ7BKhnfSdDgGjZzvQ2SJoICRMvFzGIWo8HBtTHCHUm lSGH0JHTrvbRxCEnj0/+zL7miBEsG2PF6zfyW4zwLg6RvKWBZwZQgLszE51w== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Currently, the dynptr function is not checking the variable offset part of PTR_TO_STACK that it needs to check. The fixed offset is considered when computing the stack pointer index, but if the variable offset was not a constant (such that it could not be accumulated in reg->off), we will end up a discrepency where runtime pointer does not point to the actual stack slot we mark as STACK_DYNPTR. It is impossible to precisely track dynptr state when variable offset is not constant, hence, just like bpf_timer, kptr, bpf_spin_lock, etc. simply reject the case where reg->var_off is not constant. Then, consider both reg->off and reg->var_off.value when computing the stack pointer index. A new helper dynptr_get_spi is introduced to hide over these details since the dynptr needs to be located in multiple places outside the process_dynptr_func checks, hence once we know it's a PTR_TO_STACK, we need to enforce these checks in all places. Note that it is disallowed for unprivileged users to have a non-constant var_off, so this problem should only be possible to trigger from programs having CAP_PERFMON. However, its effects can vary. Without the fix, it is possible to replace the contents of the dynptr arbitrarily by making verifier mark different stack slots than actual location and then doing writes to the actual stack address of dynptr at runtime. Fixes: 97e03f521050 ("bpf: Add verifier support for dynptrs") Acked-by: Joanne Koong Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 84 +++++++++++++++---- .../bpf/prog_tests/kfunc_dynptr_param.c | 2 +- .../testing/selftests/bpf/progs/dynptr_fail.c | 4 +- 3 files changed, 69 insertions(+), 21 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 39d8ee38c338..76afdbea425a 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -638,11 +638,34 @@ static void print_liveness(struct bpf_verifier_env *env, verbose(env, "D"); } -static int get_spi(s32 off) +static int __get_spi(s32 off) { return (-off - 1) / BPF_REG_SIZE; } +static int dynptr_get_spi(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +{ + int off, spi; + + if (!tnum_is_const(reg->var_off)) { + verbose(env, "dynptr has to be at a constant offset\n"); + return -EINVAL; + } + + off = reg->off + reg->var_off.value; + if (off % BPF_REG_SIZE) { + verbose(env, "cannot pass in dynptr at an offset=%d\n", off); + return -EINVAL; + } + + spi = __get_spi(off); + if (spi < 1) { + verbose(env, "cannot pass in dynptr at an offset=%d\n", off); + return -EINVAL; + } + return spi; +} + static bool is_spi_bounds_valid(struct bpf_func_state *state, int spi, int nr_slots) { int allocated_slots = state->allocated_stack / BPF_REG_SIZE; @@ -754,7 +777,9 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ enum bpf_dynptr_type type; int spi, i, id; - spi = get_spi(reg->off); + spi = dynptr_get_spi(env, reg); + if (spi < 0) + return spi; if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) return -EINVAL; @@ -792,7 +817,9 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re struct bpf_func_state *state = func(env, reg); int spi, i; - spi = get_spi(reg->off); + spi = dynptr_get_spi(env, reg); + if (spi < 0) + return spi; if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) return -EINVAL; @@ -844,7 +871,11 @@ static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_ if (reg->type == CONST_PTR_TO_DYNPTR) return false; - spi = get_spi(reg->off); + spi = dynptr_get_spi(env, reg); + if (spi < 0) + return false; + + /* We will do check_mem_access to check and update stack bounds later */ if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) return true; @@ -860,14 +891,15 @@ static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_ static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { struct bpf_func_state *state = func(env, reg); - int spi; - int i; + int spi, i; /* This already represents first slot of initialized bpf_dynptr */ if (reg->type == CONST_PTR_TO_DYNPTR) return true; - spi = get_spi(reg->off); + spi = dynptr_get_spi(env, reg); + if (spi < 0) + return false; if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS) || !state->stack[spi].spilled_ptr.dynptr.first_slot) return false; @@ -896,7 +928,9 @@ static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg if (reg->type == CONST_PTR_TO_DYNPTR) { return reg->dynptr.type == dynptr_type; } else { - spi = get_spi(reg->off); + spi = dynptr_get_spi(env, reg); + if (spi < 0) + return false; return state->stack[spi].spilled_ptr.dynptr.type == dynptr_type; } } @@ -2429,7 +2463,9 @@ static int mark_dynptr_read(struct bpf_verifier_env *env, struct bpf_reg_state * */ if (reg->type == CONST_PTR_TO_DYNPTR) return 0; - spi = get_spi(reg->off); + spi = dynptr_get_spi(env, reg); + if (spi < 0) + return spi; /* Caller ensures dynptr is valid and initialized, which means spi is in * bounds and spi is the first dynptr slot. Simply mark stack slot as * read. @@ -5992,12 +6028,15 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, } /* CONST_PTR_TO_DYNPTR already has fixed and var_off as 0 due to * check_func_arg_reg_off's logic. We only need to check offset - * alignment for PTR_TO_STACK. + * and its alignment for PTR_TO_STACK. */ - if (reg->type == PTR_TO_STACK && (reg->off % BPF_REG_SIZE)) { - verbose(env, "cannot pass in dynptr at an offset=%d\n", reg->off); - return -EINVAL; + if (reg->type == PTR_TO_STACK) { + int err = dynptr_get_spi(env, reg); + + if (err < 0) + return err; } + /* MEM_UNINIT - Points to memory that is an appropriate candidate for * constructing a mutable bpf_dynptr object. * @@ -6405,15 +6444,16 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, } } -static u32 dynptr_ref_obj_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +static int dynptr_ref_obj_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { struct bpf_func_state *state = func(env, reg); int spi; if (reg->type == CONST_PTR_TO_DYNPTR) return reg->ref_obj_id; - - spi = get_spi(reg->off); + spi = dynptr_get_spi(env, reg); + if (spi < 0) + return spi; return state->stack[spi].spilled_ptr.ref_obj_id; } @@ -6487,7 +6527,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, * PTR_TO_STACK. */ if (reg->type == PTR_TO_STACK) { - spi = get_spi(reg->off); + spi = dynptr_get_spi(env, reg); + if (spi < 0) + return spi; if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS) || !state->stack[spi].spilled_ptr.ref_obj_id) { verbose(env, "arg %d is an unacquired reference\n", regno); @@ -7977,13 +8019,19 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { if (arg_type_is_dynptr(fn->arg_type[i])) { struct bpf_reg_state *reg = ®s[BPF_REG_1 + i]; + int ref_obj_id; if (meta.ref_obj_id) { verbose(env, "verifier internal error: meta.ref_obj_id already set\n"); return -EFAULT; } - meta.ref_obj_id = dynptr_ref_obj_id(env, reg); + ref_obj_id = dynptr_ref_obj_id(env, reg); + if (ref_obj_id < 0) { + verbose(env, "verifier internal error: failed to obtain dynptr ref_obj_id\n"); + return ref_obj_id; + } + meta.ref_obj_id = ref_obj_id; break; } } diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c b/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c index a9229260a6ce..72800b1e8395 100644 --- a/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c @@ -18,7 +18,7 @@ static struct { const char *expected_verifier_err_msg; int expected_runtime_err; } kfunc_dynptr_tests[] = { - {"not_valid_dynptr", "Expected an initialized dynptr as arg #1", 0}, + {"not_valid_dynptr", "cannot pass in dynptr at an offset=-8", 0}, {"not_ptr_to_stack", "arg#0 expected pointer to stack or dynptr_ptr", 0}, {"dynptr_data_null", NULL, -EBADMSG}, }; diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index 78debc1b3820..02d57b95cf6e 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -382,7 +382,7 @@ int invalid_helper1(void *ctx) /* A dynptr can't be passed into a helper function at a non-zero offset */ SEC("?raw_tp") -__failure __msg("Expected an initialized dynptr as arg #3") +__failure __msg("cannot pass in dynptr at an offset=-8") int invalid_helper2(void *ctx) { struct bpf_dynptr ptr; @@ -584,7 +584,7 @@ int invalid_read4(void *ctx) /* Initializing a dynptr on an offset should fail */ SEC("?raw_tp") -__failure __msg("invalid write to stack") +__failure __msg("cannot pass in dynptr at an offset=0") int invalid_offset(void *ctx) { struct bpf_dynptr ptr; From patchwork Fri Jan 20 07:03:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109274 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EE20DC25B4E for ; Fri, 20 Jan 2023 07:04:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229437AbjATHEN (ORCPT ); Fri, 20 Jan 2023 02:04:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47948 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229713AbjATHEM (ORCPT ); Fri, 20 Jan 2023 02:04:12 -0500 Received: from mail-pf1-x442.google.com (mail-pf1-x442.google.com [IPv6:2607:f8b0:4864:20::442]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8A774241D9 for ; Thu, 19 Jan 2023 23:04:10 -0800 (PST) Received: by mail-pf1-x442.google.com with SMTP id c26so3292787pfp.10 for ; Thu, 19 Jan 2023 23:04:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=2H/54ZqUIjU3VYoXRIgXxyEiI8YMEx+pUQ0BaawvGCY=; b=HhN3OrSSlscayczrtewUgp4FWDQwGakQf1b1rZlGPaZd4oyXjjSjy0KupaMU6Wvr7N No2Kf4mF1Wqct+Af0RyQMwzunMu4741N+w3CHXNV6yZAJYKH27sKQVty6xI5IDAodwNs KDtVIYAcsicwIYEIYV9Q2WZGcT/M4SRGn6+rtKU/DEDjlmO7ZUpxJKmEI9nf1ss041ve /cdqPOx9jx5q0JRAMV31ZFtMqlAFNBR4rrjKYgkB3jQbZmJJSlZk5NXA24wK5Ava1V9h 4wZURlz6kdmFnh6EuOmR/LUF45MvocPwhCIyQpBbJmOnHHHWv6zpsKAKe1QcAk8PFh8o ap2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=2H/54ZqUIjU3VYoXRIgXxyEiI8YMEx+pUQ0BaawvGCY=; b=ihr1buSvKChEpUh7rkHZQAaeC8hHxeW+DpN+NQhwKMzZSVNBpYTWQT9guogjaxlqUU 9BD4zAVu8SoiNUNfC2dwTO+STJmNQEuyH93WClUM18huJG0bjQQz0dkZW+8sQTAbvKeG auc59MEFEiX2/Yttlmbari1k1NQacmyP9JxPd56f69rUj1ci0ZUHNGDnYcnuvSxXf68P DNwoxELWLXf0qwqsOfhZtMgQbRFoM8lE8Bc0dKbBNGikSnc4Xi7KtkbT9sZcBsVgCD7c p+X72XCCNWrzyEzAnMQy1m1ad0RyBKYMGLXfry86hBQIlCI+6aqx04FZUCEat+vmHI4N DWeQ== X-Gm-Message-State: AFqh2kpTlIRNzjNJUDClunlOneCsJSLgQ6HmQOx172PwB0Dfcs78ly9s h4fsTaipsa4ih13yQazgimmBXZkVrC4= X-Google-Smtp-Source: AMrXdXsyBZ84sY2XHrYOPLeQpydbCDyb8QwetGnAgCWtQyEHgjDQe6265uPfnmoQgoj+5mglcXWMrw== X-Received: by 2002:a05:6a00:340a:b0:58d:aae8:1c2 with SMTP id cn10-20020a056a00340a00b0058daae801c2mr15911814pfb.8.1674198249706; Thu, 19 Jan 2023 23:04:09 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id z20-20020aa79594000000b0058de2c315e6sm4792438pfj.158.2023.01.19.23.04.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:09 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 03/12] bpf: Fix partial dynptr stack slot reads/writes Date: Fri, 20 Jan 2023 12:33:46 +0530 Message-Id: <20230120070355.1983560-4-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=8936; i=memxor@gmail.com; h=from:subject; bh=88XPzRu2Z9EO4iSWHshfPh1pJZo++IffnWh/36lilyY=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzLDrwrMkcLcpOoo/SrtdbpZSaCSGvVeLRH4RUd 8HAwUXWJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8ywAKCRBM4MiGSL8Ryuf5EA CcNaodh7bWhoYTOBAau7AxkvTn6LpGXzKMBikvnbm75MHz/gW3iVeDfsvcWndFOdyM+hXku/DcHO5i VD5sOUey1XJen0t5nTKo7GbBnyvaCCOWpo3TH9kF/q5i5mS2U7qikYbhH+9fMKPIhxqwHLEa2mPVi9 9/MyI7yMnKne97YnkEs2ATLHpLRdbKE3fcHcrxDDsIwZcUwRuAzmm7lznCHU/Lg465DvAjFgjEwJ8B sS9NS23G8dXoPo3uyt1JPNDZx4DtcPvE+XLTpKi2XXiD1wNSGsjC55o8sfgznTcR8EIjuH05HgHKgV lgM50zyLDNI3r2eQekW6WkS4+sBze/a65EWUD2UQtxje2T3K/ipnU83LWtrmUc+VuuLd9pNw6n6yVF MeRAdcoPVB1wVRLR4PCyd69MS9rVXYgJBRbwrLBkoptqwHIQvUFP5ObYIUosokXZEjNCgm91ABYCwO WT0nKjNDDT1uW28RMhNNu4P0a0w63IW+KshNBpPoUyAs7eEExPRbFp2KphCs3hTROMRLXlFazKTSZB RlbkzXBbyApNUWYt/NF6P+yv0C8HYLpQ3CLvU/KtbFXaWDfSwPYN2yIhPswOgnv9GdEJbKEPaf7uaN SybxWIo0X9EcOruo/NoSE1fRn20fojAsFAU4y3aLfIDs+14qpVPE8XGQn7SQ== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Currently, while reads are disallowed for dynptr stack slots, writes are not. Reads don't work from both direct access and helpers, while writes do work in both cases, but have the effect of overwriting the slot_type. While this is fine, handling for a few edge cases is missing. Firstly, a user can overwrite the stack slots of dynptr partially. Consider the following layout: spi: [d][d][?] 2 1 0 First slot is at spi 2, second at spi 1. Now, do a write of 1 to 8 bytes for spi 1. This will essentially either write STACK_MISC for all slot_types or STACK_MISC and STACK_ZERO (in case of size < BPF_REG_SIZE partial write of zeroes). The end result is that slot is scrubbed. Now, the layout is: spi: [d][m][?] 2 1 0 Suppose if user initializes spi = 1 as dynptr. We get: spi: [d][d][d] 2 1 0 But this time, both spi 2 and spi 1 have first_slot = true. Now, when passing spi 2 to dynptr helper, it will consider it as initialized as it does not check whether second slot has first_slot == false. And spi 1 should already work as normal. This effectively replaced size + offset of first dynptr, hence allowing invalid OOB reads and writes. Make a few changes to protect against this: When writing to PTR_TO_STACK using BPF insns, when we touch spi of a STACK_DYNPTR type, mark both first and second slot (regardless of which slot we touch) as STACK_INVALID. Reads are already prevented. Second, prevent writing to stack memory from helpers if the range may contain any STACK_DYNPTR slots. Reads are already prevented. For helpers, we cannot allow it to destroy dynptrs from the writes as depending on arguments, helper may take uninit_mem and dynptr both at the same time. This would mean that helper may write to uninit_mem before it reads the dynptr, which would be bad. PTR_TO_MEM: [?????dd] Depending on the code inside the helper, it may end up overwriting the dynptr contents first and then read those as the dynptr argument. Verifier would only simulate destruction when it does byte by byte access simulation in check_helper_call for meta.access_size, and fail to catch this case, as it happens after argument checks. The same would need to be done for any other non-trivial objects created on the stack in the future, such as bpf_list_head on stack, or bpf_rb_root on stack. A common misunderstanding in the current code is that MEM_UNINIT means writes, but note that writes may also be performed even without MEM_UNINIT in case of helpers, in that case the code after handling meta && meta->raw_mode will complain when it sees STACK_DYNPTR. So that invalid read case also covers writes to potential STACK_DYNPTR slots. The only loophole was in case of meta->raw_mode which simulated writes through instructions which could overwrite them. A future series sequenced after this will focus on the clean up of helper access checks and bugs around that. Fixes: 97e03f521050 ("bpf: Add verifier support for dynptrs") Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 88 +++++++++++++++++++ .../testing/selftests/bpf/progs/dynptr_fail.c | 6 +- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 76afdbea425a..5c7f29ca94ec 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -769,6 +769,8 @@ static void mark_dynptr_cb_reg(struct bpf_reg_state *reg, __mark_dynptr_reg(reg, type, true); } +static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, + struct bpf_func_state *state, int spi); static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg, enum bpf_arg_type arg_type, int insn_idx) @@ -863,6 +865,55 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re return 0; } +static void __mark_reg_unknown(const struct bpf_verifier_env *env, + struct bpf_reg_state *reg); + +static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, + struct bpf_func_state *state, int spi) +{ + int i; + + /* We always ensure that STACK_DYNPTR is never set partially, + * hence just checking for slot_type[0] is enough. This is + * different for STACK_SPILL, where it may be only set for + * 1 byte, so code has to use is_spilled_reg. + */ + if (state->stack[spi].slot_type[0] != STACK_DYNPTR) + return 0; + + /* Reposition spi to first slot */ + if (!state->stack[spi].spilled_ptr.dynptr.first_slot) + spi = spi + 1; + + if (dynptr_type_refcounted(state->stack[spi].spilled_ptr.dynptr.type)) { + verbose(env, "cannot overwrite referenced dynptr\n"); + return -EINVAL; + } + + mark_stack_slot_scratched(env, spi); + mark_stack_slot_scratched(env, spi - 1); + + /* Writing partially to one dynptr stack slot destroys both. */ + for (i = 0; i < BPF_REG_SIZE; i++) { + state->stack[spi].slot_type[i] = STACK_INVALID; + state->stack[spi - 1].slot_type[i] = STACK_INVALID; + } + + /* TODO: Invalidate any slices associated with this dynptr */ + + /* Do not release reference state, we are destroying dynptr on stack, + * not using some helper to release it. Just reset register. + */ + __mark_reg_not_init(env, &state->stack[spi].spilled_ptr); + __mark_reg_not_init(env, &state->stack[spi - 1].spilled_ptr); + + /* Same reason as unmark_stack_slots_dynptr above */ + state->stack[spi].spilled_ptr.live |= REG_LIVE_WRITTEN; + state->stack[spi - 1].spilled_ptr.live |= REG_LIVE_WRITTEN; + + return 0; +} + static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { struct bpf_func_state *state = func(env, reg); @@ -3391,6 +3442,10 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env, env->insn_aux_data[insn_idx].sanitize_stack_spill = true; } + err = destroy_if_dynptr_stack_slot(env, state, spi); + if (err) + return err; + mark_stack_slot_scratched(env, spi); if (reg && !(off % BPF_REG_SIZE) && register_is_bounded(reg) && !register_is_null(reg) && env->bpf_capable) { @@ -3504,6 +3559,14 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env, if (err) return err; + for (i = min_off; i < max_off; i++) { + int spi; + + spi = __get_spi(i); + err = destroy_if_dynptr_stack_slot(env, state, spi); + if (err) + return err; + } /* Variable offset writes destroy any spilled pointers in range. */ for (i = min_off; i < max_off; i++) { @@ -5531,6 +5594,31 @@ static int check_stack_range_initialized( } if (meta && meta->raw_mode) { + /* Ensure we won't be overwriting dynptrs when simulating byte + * by byte access in check_helper_call using meta.access_size. + * This would be a problem if we have a helper in the future + * which takes: + * + * helper(uninit_mem, len, dynptr) + * + * Now, uninint_mem may overlap with dynptr pointer. Hence, it + * may end up writing to dynptr itself when touching memory from + * arg 1. This can be relaxed on a case by case basis for known + * safe cases, but reject due to the possibilitiy of aliasing by + * default. + */ + for (i = min_off; i < max_off + access_size; i++) { + int stack_off = -i - 1; + + spi = __get_spi(i); + /* raw_mode may write past allocated_stack */ + if (state->allocated_stack <= stack_off) + continue; + if (state->stack[spi].slot_type[stack_off % BPF_REG_SIZE] == STACK_DYNPTR) { + verbose(env, "potential write to dynptr at off=%d disallowed\n", i); + return -EACCES; + } + } meta->access_size = access_size; meta->regno = regno; return 0; diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index 02d57b95cf6e..9dc3f23a8270 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -420,7 +420,7 @@ int invalid_write1(void *ctx) * offset */ SEC("?raw_tp") -__failure __msg("Expected an initialized dynptr as arg #3") +__failure __msg("cannot overwrite referenced dynptr") int invalid_write2(void *ctx) { struct bpf_dynptr ptr; @@ -444,7 +444,7 @@ int invalid_write2(void *ctx) * non-const offset */ SEC("?raw_tp") -__failure __msg("Expected an initialized dynptr as arg #1") +__failure __msg("cannot overwrite referenced dynptr") int invalid_write3(void *ctx) { struct bpf_dynptr ptr; @@ -476,7 +476,7 @@ static int invalid_write4_callback(__u32 index, void *data) * be invalidated as a dynptr */ SEC("?raw_tp") -__failure __msg("arg 1 is an unacquired reference") +__failure __msg("cannot overwrite referenced dynptr") int invalid_write4(void *ctx) { struct bpf_dynptr ptr; From patchwork Fri Jan 20 07:03:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109275 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 503F2C05027 for ; Fri, 20 Jan 2023 07:04:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229713AbjATHEQ (ORCPT ); Fri, 20 Jan 2023 02:04:16 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48010 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229816AbjATHEP (ORCPT ); Fri, 20 Jan 2023 02:04:15 -0500 Received: from mail-pj1-x1044.google.com (mail-pj1-x1044.google.com [IPv6:2607:f8b0:4864:20::1044]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 66DB211659 for ; Thu, 19 Jan 2023 23:04:14 -0800 (PST) Received: by mail-pj1-x1044.google.com with SMTP id n20-20020a17090aab9400b00229ca6a4636so7339838pjq.0 for ; Thu, 19 Jan 2023 23:04:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=LOz80cJ3IZFOxxyLue64vsI8jEYRpgXfAo5uR9M08kA=; b=p65gq/FfWMdSTJZ6sYY28CDi6neOCczLrYgfob1r6dUq5fxIOfJKwEigybo6SQ31s9 FjTBAbS9DLB89opQdLhcBEcKlUD9DcRNjlhjPvqY1FeMTDUWm7hKLkcZ8ypl+V1/whP4 UWtt1vxFM9Q4It88RFcMnW7f4gB5A3ay8JVuj8tZyW2LvRyShtjF2X3Y1NIbTqaDk+Qp wiW7xSz8BWkuH9B4TpueEM3bl2/Fma2+iStf/aRDL9AiFIQ6RGhLKcoQoaQidVxJTlN+ HJLpDE8PPvHdwohM8XYE2zjKfXkwLgTv/m7nM+dx6wruI6pbeW63JsHyhulRxGqfBwmZ H9cg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=LOz80cJ3IZFOxxyLue64vsI8jEYRpgXfAo5uR9M08kA=; b=meOKYIAmO19pMNGtcVcFO3mKr618WIYrIH+GOvgRDAR7bA0WzarPmrElPpMbu4f0vZ 2iPeaSVX9pjU6F1hp1OyPS/EpTH86Vf2f8F4CSZtyzBF6ANVvXkU5/yH6IeZb/exxi/y BUMxdTFbnGkv6XLprDrNYIUJ2IPOPNoXXdGxb9ER/SYAoq5JNQGX4KIW9fGxZtefzVv+ oHHtD1j1nXJDSk3pa/A/LRLD+5pmVgNRZ2iI7mfL4VjS7wR0iCaQu3Lfzn6xq3RZQnmC vCURC5yQMgO2XLjIlAi+dLvQfC5+F1mMNvvkh0tLluLdiQWMyx/VpVVVCfOTQ6WA2zft eg6w== X-Gm-Message-State: AFqh2krEApXPI1WQpHIZBa5dzkqrDhGamSKA906SvE62zU0cDGqtQsjX sFhaOvvpl3gFvDGit+e46eXZEYOM890= X-Google-Smtp-Source: AMrXdXsO/ChUPR9sGeSqRskXFx9VPC7VYCaU74hb9ZlfbsQJ0aAE5HOMcNnXuTP5IFtruLuSF+qjJA== X-Received: by 2002:a17:902:aa05:b0:193:335a:989e with SMTP id be5-20020a170902aa0500b00193335a989emr14500402plb.25.1674198253613; Thu, 19 Jan 2023 23:04:13 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id w15-20020a170902a70f00b00186b86ed450sm10508120plq.156.2023.01.19.23.04.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:13 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 04/12] bpf: Invalidate slices on destruction of dynptrs on stack Date: Fri, 20 Jan 2023 12:33:47 +0530 Message-Id: <20230120070355.1983560-5-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9330; i=memxor@gmail.com; h=from:subject; bh=OL9gEtpOpPO43wWplrijA6mPPBf0/K4Hd5BOyzsMLCU=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzL73eOalfR0quAX8XXX6QYGDZfPEkqepJIzmC5 8h5tvXqJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8ywAKCRBM4MiGSL8Ryvt/D/ 9ONKRIjtvFqEJq8sgAikcDr4h1/tNem9N5N+7xuaCTTnqsWjlOj70AyZ6c968vLrbajxkalxiqQESm pe411Q3P8sV+qxsdz8UYXRVAmTZpyauHCqLOGAkQpgOroUDMS5Ok0kasGBU6EQkMNyV5SHBI0dts1e N48BL+c5y3VaRSBicYtwOaP1q7fKr++5sgFKRsO79VvbSVYDtrn2PO0pIYStCxollbqP6hhn5ftjq0 Xs7yc2rL+EWISQQjyCOn/2NQw5ys4KpFCH2cOGJOuhMxi1ThExXtSpM/Q0WyneHXROFwmafl8moZX6 u3nEx06NsPkPLxIjbhNwRBbILHcLomRg/YgGSJn/Bnhn89s3cOcBeKNBGISYDaTvcbGeGxuJqSkTNF QaIr2qMJyLBxyuhwb57eZnmdSfMnY8JKnJGsIXQPi6ygKVZbtm8Z2fr4ynkWcZMpWB64OIARQmLYmI NnjLbyO3VwTVG6fOUSYZDU6uZGrRKzSs9HetCGlUntO/frAQIyo7XNXBnpKcMBtXzrZy1bnpJqZoJP CX7VoIzjWVxRA2wyuA+WizRiv5TfGmecT0QP2oiL4P1vc1zDT7ecybOm+64uXEaBHwBLJt4B02TuQL Oif+6wWxG20H7knq6UU0iyBtmkdaooBle4In+ShWwmoPM2P61oT2REK+YV3g== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The previous commit implemented destroy_if_dynptr_stack_slot. It destroys the dynptr which given spi belongs to, but still doesn't invalidate the slices that belong to such a dynptr. While for the case of referenced dynptr, we don't allow their overwrite and return an error early, we still allow it and destroy the dynptr for unreferenced dynptr. To be able to enable precise and scoped invalidation of dynptr slices in this case, we must be able to associate the source dynptr of slices that have been obtained using bpf_dynptr_data. When doing destruction, only slices belonging to the dynptr being destructed should be invalidated, and nothing else. Currently, dynptr slices belonging to different dynptrs are indistinguishible. Hence, allocate a unique id to each dynptr (CONST_PTR_TO_DYNPTR and those on stack). This will be stored as part of reg->id. Whenever using bpf_dynptr_data, transfer this unique dynptr id to the returned PTR_TO_MEM_OR_NULL slice pointer, and store it in a new per-PTR_TO_MEM dynptr_id register state member. Finally, after establishing such a relationship between dynptrs and their slices, implement precise invalidation logic that only invalidates slices belong to the destroyed dynptr in destroy_if_dynptr_stack_slot. Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: Joanne Koong --- include/linux/bpf_verifier.h | 5 +- kernel/bpf/verifier.c | 74 ++++++++++++++++--- .../testing/selftests/bpf/progs/dynptr_fail.c | 4 +- 3 files changed, 68 insertions(+), 15 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 127058cfec47..aa83de1fe755 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -70,7 +70,10 @@ struct bpf_reg_state { u32 btf_id; }; - u32 mem_size; /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */ + struct { /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */ + u32 mem_size; + u32 dynptr_id; /* for dynptr slices */ + }; /* For dynptr stack slots */ struct { diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 5c7f29ca94ec..01cb802776fd 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -255,6 +255,7 @@ struct bpf_call_arg_meta { int mem_size; u64 msize_max_value; int ref_obj_id; + int dynptr_id; int map_uid; int func_id; struct btf *btf; @@ -750,23 +751,27 @@ static bool dynptr_type_refcounted(enum bpf_dynptr_type type) static void __mark_dynptr_reg(struct bpf_reg_state *reg, enum bpf_dynptr_type type, - bool first_slot); + bool first_slot, int dynptr_id); static void __mark_reg_not_init(const struct bpf_verifier_env *env, struct bpf_reg_state *reg); -static void mark_dynptr_stack_regs(struct bpf_reg_state *sreg1, +static void mark_dynptr_stack_regs(struct bpf_verifier_env *env, + struct bpf_reg_state *sreg1, struct bpf_reg_state *sreg2, enum bpf_dynptr_type type) { - __mark_dynptr_reg(sreg1, type, true); - __mark_dynptr_reg(sreg2, type, false); + int id = ++env->id_gen; + + __mark_dynptr_reg(sreg1, type, true, id); + __mark_dynptr_reg(sreg2, type, false, id); } -static void mark_dynptr_cb_reg(struct bpf_reg_state *reg, +static void mark_dynptr_cb_reg(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, enum bpf_dynptr_type type) { - __mark_dynptr_reg(reg, type, true); + __mark_dynptr_reg(reg, type, true, ++env->id_gen); } static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, @@ -795,7 +800,7 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ if (type == BPF_DYNPTR_TYPE_INVALID) return -EINVAL; - mark_dynptr_stack_regs(&state->stack[spi].spilled_ptr, + mark_dynptr_stack_regs(env, &state->stack[spi].spilled_ptr, &state->stack[spi - 1].spilled_ptr, type); if (dynptr_type_refcounted(type)) { @@ -871,7 +876,9 @@ static void __mark_reg_unknown(const struct bpf_verifier_env *env, static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, struct bpf_func_state *state, int spi) { - int i; + struct bpf_func_state *fstate; + struct bpf_reg_state *dreg; + int i, dynptr_id; /* We always ensure that STACK_DYNPTR is never set partially, * hence just checking for slot_type[0] is enough. This is @@ -899,7 +906,19 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, state->stack[spi - 1].slot_type[i] = STACK_INVALID; } - /* TODO: Invalidate any slices associated with this dynptr */ + dynptr_id = state->stack[spi].spilled_ptr.id; + /* Invalidate any slices associated with this dynptr */ + bpf_for_each_reg_in_vstate(env->cur_state, fstate, dreg, ({ + /* Dynptr slices are only PTR_TO_MEM_OR_NULL and PTR_TO_MEM */ + if (dreg->type != (PTR_TO_MEM | PTR_MAYBE_NULL) && dreg->type != PTR_TO_MEM) + continue; + if (dreg->dynptr_id == dynptr_id) { + if (!env->allow_ptr_leaks) + __mark_reg_not_init(env, dreg); + else + __mark_reg_unknown(env, dreg); + } + })); /* Do not release reference state, we are destroying dynptr on stack, * not using some helper to release it. Just reset register. @@ -1562,7 +1581,7 @@ static void mark_reg_known_zero(struct bpf_verifier_env *env, } static void __mark_dynptr_reg(struct bpf_reg_state *reg, enum bpf_dynptr_type type, - bool first_slot) + bool first_slot, int dynptr_id) { /* reg->type has no meaning for STACK_DYNPTR, but when we set reg for * callback arguments, it does need to be CONST_PTR_TO_DYNPTR, so simply @@ -1570,6 +1589,8 @@ static void __mark_dynptr_reg(struct bpf_reg_state *reg, enum bpf_dynptr_type ty */ __mark_reg_known_zero(reg); reg->type = CONST_PTR_TO_DYNPTR; + /* Give each dynptr a unique id to uniquely associate slices to it. */ + reg->id = dynptr_id; reg->dynptr.type = type; reg->dynptr.first_slot = first_slot; } @@ -6532,6 +6553,19 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, } } +static int dynptr_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +{ + struct bpf_func_state *state = func(env, reg); + int spi; + + if (reg->type == CONST_PTR_TO_DYNPTR) + return reg->id; + spi = dynptr_get_spi(env, reg); + if (spi < 0) + return spi; + return state->stack[spi].spilled_ptr.id; +} + static int dynptr_ref_obj_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { struct bpf_func_state *state = func(env, reg); @@ -7601,7 +7635,7 @@ static int set_user_ringbuf_callback_state(struct bpf_verifier_env *env, * callback_fn(const struct bpf_dynptr_t* dynptr, void *callback_ctx); */ __mark_reg_not_init(env, &callee->regs[BPF_REG_0]); - mark_dynptr_cb_reg(&callee->regs[BPF_REG_1], BPF_DYNPTR_TYPE_LOCAL); + mark_dynptr_cb_reg(env, &callee->regs[BPF_REG_1], BPF_DYNPTR_TYPE_LOCAL); callee->regs[BPF_REG_2] = caller->regs[BPF_REG_3]; /* unused */ @@ -8107,18 +8141,31 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { if (arg_type_is_dynptr(fn->arg_type[i])) { struct bpf_reg_state *reg = ®s[BPF_REG_1 + i]; - int ref_obj_id; + int id, ref_obj_id; + + if (meta.dynptr_id) { + verbose(env, "verifier internal error: meta.dynptr_id already set\n"); + return -EFAULT; + } if (meta.ref_obj_id) { verbose(env, "verifier internal error: meta.ref_obj_id already set\n"); return -EFAULT; } + id = dynptr_id(env, reg); + if (id < 0) { + verbose(env, "verifier internal error: failed to obtain dynptr id\n"); + return id; + } + ref_obj_id = dynptr_ref_obj_id(env, reg); if (ref_obj_id < 0) { verbose(env, "verifier internal error: failed to obtain dynptr ref_obj_id\n"); return ref_obj_id; } + + meta.dynptr_id = id; meta.ref_obj_id = ref_obj_id; break; } @@ -8275,6 +8322,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn return -EFAULT; } + if (is_dynptr_ref_function(func_id)) + regs[BPF_REG_0].dynptr_id = meta.dynptr_id; + if (is_ptr_cast_function(func_id) || is_dynptr_ref_function(func_id)) { /* For release_reference() */ regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id; diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index 9dc3f23a8270..e43000c63c66 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -67,7 +67,7 @@ static int get_map_val_dynptr(struct bpf_dynptr *ptr) * bpf_ringbuf_submit/discard_dynptr call */ SEC("?raw_tp") -__failure __msg("Unreleased reference id=1") +__failure __msg("Unreleased reference id=2") int ringbuf_missing_release1(void *ctx) { struct bpf_dynptr ptr; @@ -80,7 +80,7 @@ int ringbuf_missing_release1(void *ctx) } SEC("?raw_tp") -__failure __msg("Unreleased reference id=2") +__failure __msg("Unreleased reference id=4") int ringbuf_missing_release2(void *ctx) { struct bpf_dynptr ptr1, ptr2; From patchwork Fri Jan 20 07:03:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109276 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5EB7CC25B4E for ; Fri, 20 Jan 2023 07:04:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229845AbjATHET (ORCPT ); Fri, 20 Jan 2023 02:04:19 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48062 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229814AbjATHES (ORCPT ); Fri, 20 Jan 2023 02:04:18 -0500 Received: from mail-pj1-x1044.google.com (mail-pj1-x1044.google.com [IPv6:2607:f8b0:4864:20::1044]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F05FD40FB for ; Thu, 19 Jan 2023 23:04:17 -0800 (PST) Received: by mail-pj1-x1044.google.com with SMTP id z1-20020a17090a66c100b00226f05b9595so4076302pjl.0 for ; Thu, 19 Jan 2023 23:04:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=tJTE+2IiThjMO3PC0iuCb5HXJeu+oAk3QS1io4LW83g=; b=A7s4MhGg+4do/NyS56v6JrVz0LzTivVOUmV1UQ0Ba96E4TxbVA6lgYSHRi2zVTLaIe yI/B3K6RkD0jEW1H5Z/Rppmce7JbnEe4XfhKEBODd5WhmRtw833pzlVQK2GTW/FpbHnU YpVJJk4OgdrJTmDAu/lCftnmsj9uDg9qkYP5GOGBGciiej5xVMvhB/24sZHuZc+0LS1Y 2ypJLe1yyfd4ZQ4c9rnnTG+5FFP/IMoNb0YxoJJO2QYtT8gyiXxKWSdDmmt6hmXmcRqa t0X+wHX3MF0ChcrWWc9+9SNeBj0SrO1E2cLAxcq4/ZJ1XDGgRa9+IhL5OOEkSUqjReyJ jDig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=tJTE+2IiThjMO3PC0iuCb5HXJeu+oAk3QS1io4LW83g=; b=YvSxGg+PzTpgUKg/qUePJbqXAC1j8Mc1QMJMIi7mKNKZ8L5H4bF9NWIsK4NMXx1/eS KWmRDVDqVIYrS3/OeSVNBVA7xhDY43n2hl1qePekNIytiQIdCH5GvK3oYUZvVTNuuNAr XaFb5M4zYnff5N/SHo2h72a9ZZY9QQRb8wLqBEMrbpF81KM+gbL76qp5jRbDCB1/YCU9 vvvZ6lTXcMk+W0y6s8I5YIErp1Vfaevc/M/hIxxF5CGhCXhaJ4x0BH1gjeGGZqmZwIwk lZMsVYjxFP/K12OhfD63hvMuSquTWbF2BPw3E7ANyfKM0LF0wjBF8jPTMqNJL0bL9aYD gNIQ== X-Gm-Message-State: AFqh2krb01gkhFzimzlG6bTYXduxIzouxzw8dAvhfRNXEFJVFsyD9AUs Ui3BWNdY7TFe5pbjECk2bxdF2qj8oaI= X-Google-Smtp-Source: AMrXdXt/IXcjM5b/aAyyZ40wJM498xSBaLg8yjEx0PBZCEjsYC69sjp/4IbSlkKhbSRMc+YN8ZFULQ== X-Received: by 2002:a17:90b:1206:b0:229:188a:c0e7 with SMTP id gl6-20020a17090b120600b00229188ac0e7mr13801813pjb.49.1674198257290; Thu, 19 Jan 2023 23:04:17 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id gp6-20020a17090adf0600b00219186abd7csm766152pjb.16.2023.01.19.23.04.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:16 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Joanne Koong , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 05/12] bpf: Allow reinitializing unreferenced dynptr stack slots Date: Fri, 20 Jan 2023 12:33:48 +0530 Message-Id: <20230120070355.1983560-6-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3888; i=memxor@gmail.com; h=from:subject; bh=YkhcRHw6M8R1uNyUytIog/IKy6jL6P3mgH0kEKFhQB8=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzLJqykp549TuD8qfi+l8J7mju9PTDOmDmMoZB7 eluhbseJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8ywAKCRBM4MiGSL8RyoYxD/ 9aqXjUHfv3Ewq2IQuoLFusL8z9DK1rNpN0O9+Fq/lVB594vNcwDiLO1pv2xfDpr6qC8HgpTo73fbip g70arQIo5wrFQFAqhjIZW+RDdk9kBN0p6unV9R/q6oqm3+SWJSe9J4jm+8DLyx61oh7+bkSifSVCYl V84sY/9DWVklgMaM9fCcph9zFH26rcelQLOg+6cqVDGFxEWyRwCSlb7SLcmN8TDltxuU8SXUthgUVk jTCZsCWhkjp9r/f//LJrSDy76TT+79lNWcXtdT6LmNBLNNfAv43duEfIXltBuaRxa810UPl5dU1rEx Bt3/RGvEouZbNMb1+JpG7c4446GN+6kx5BPF+xY27aOR6eqHqc8HYbpSWE1skdK5yp+yXcn1dE0Xl6 O+hW7wBfmJTzQQ+MAUc3vsrhnCshkBXGKJvuADaZ9SBZV1Eu2ZpCXBheaTZK3YHwzlpyQzibCFAyDB BNvlVAAIIq50vTejkX/eMWJBNQ8/Jjfh3r69mfIIU6tlCae8pDnZGt1skyN4BZPHteGLFLhCE9g0bo F/Peh0XbDTDr9ryEFVgRpD6OM8oCSLL3YYRhtpmAgcFEbKlEx3h3xvxh/2wPCNx0OM6LRmyJarY/Mi m0O0VkrrObGMWpYmzMyleTUVz/vGlOrKH7DO78eInrMtkBzqK88I0yxDIV9g== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Consider a program like below: void prog(void) { { struct bpf_dynptr ptr; bpf_dynptr_from_mem(...); } ... { struct bpf_dynptr ptr; bpf_dynptr_from_mem(...); } } Here, the C compiler based on lifetime rules in the C standard would be well within in its rights to share stack storage for dynptr 'ptr' as their lifetimes do not overlap in the two distinct scopes. Currently, such an example would be rejected by the verifier, but this is too strict. Instead, we should allow reinitializing over dynptr stack slots and forget information about the old dynptr object. The destroy_if_dynptr_stack_slot function already makes necessary checks to avoid overwriting referenced dynptr slots. This is done to present a better error message instead of forgetting dynptr information on stack and preserving reference state, leading to an inevitable but undecipherable error at the end about an unreleased reference which has to be associated back to its allocating call instruction to make any sense to the user. Acked-by: Joanne Koong Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 01cb802776fd..e5745b696bfe 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -782,7 +782,7 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ { struct bpf_func_state *state = func(env, reg); enum bpf_dynptr_type type; - int spi, i, id; + int spi, i, id, err; spi = dynptr_get_spi(env, reg); if (spi < 0) @@ -791,6 +791,22 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) return -EINVAL; + /* We cannot assume both spi and spi - 1 belong to the same dynptr, + * hence we need to call destroy_if_dynptr_stack_slot twice for both, + * to ensure that for the following example: + * [d1][d1][d2][d2] + * spi 3 2 1 0 + * So marking spi = 2 should lead to destruction of both d1 and d2. In + * case they do belong to same dynptr, second call won't see slot_type + * as STACK_DYNPTR and will simply skip destruction. + */ + err = destroy_if_dynptr_stack_slot(env, state, spi); + if (err) + return err; + err = destroy_if_dynptr_stack_slot(env, state, spi - 1); + if (err) + return err; + for (i = 0; i < BPF_REG_SIZE; i++) { state->stack[spi].slot_type[i] = STACK_DYNPTR; state->stack[spi - 1].slot_type[i] = STACK_DYNPTR; @@ -936,7 +952,7 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { struct bpf_func_state *state = func(env, reg); - int spi, i; + int spi; if (reg->type == CONST_PTR_TO_DYNPTR) return false; @@ -949,12 +965,14 @@ static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_ if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) return true; - for (i = 0; i < BPF_REG_SIZE; i++) { - if (state->stack[spi].slot_type[i] == STACK_DYNPTR || - state->stack[spi - 1].slot_type[i] == STACK_DYNPTR) - return false; - } - + /* We allow overwriting existing unreferenced STACK_DYNPTR slots, see + * mark_stack_slots_dynptr which calls destroy_if_dynptr_stack_slot to + * ensure dynptr objects at the slots we are touching are completely + * destructed before we reinitialize them for a new one. For referenced + * ones, destroy_if_dynptr_stack_slot returns an error early instead of + * delaying it until the end where the user will get "Unreleased + * reference" error. + */ return true; } From patchwork Fri Jan 20 07:03:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109277 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E2C9C05027 for ; Fri, 20 Jan 2023 07:04:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229844AbjATHEX (ORCPT ); Fri, 20 Jan 2023 02:04:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48114 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229816AbjATHEW (ORCPT ); Fri, 20 Jan 2023 02:04:22 -0500 Received: from mail-pg1-x544.google.com (mail-pg1-x544.google.com [IPv6:2607:f8b0:4864:20::544]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5C594530D7 for ; Thu, 19 Jan 2023 23:04:21 -0800 (PST) Received: by mail-pg1-x544.google.com with SMTP id g68so3419709pgc.11 for ; Thu, 19 Jan 2023 23:04:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=w6atmUnu6yUDSAXWR/NCWLDY2ibxR/H8Klr3dPFutJk=; b=n+rdGyRfKbjwmU2UsGYd7odVlcBa0/xk7brZ/+PoxV/yYDjf9ipieEPFxiScsBwW8u HBPAKhpYIYfvAKT8x07Zjizi2Xly5AHmFH8Vc2Vob1geVZ7UXfxxofUHjnUl7NT9TE3h IdZ64gIyGBKQzJmSfDkA/KWloPrjIql4a14/zm7YB5npjUrKobrsn6u9hUqVTFIMr6BP e8NVe1PqN9X/Ffo7E49HwZX34NYd5zb9X+7R7yJoKudbSc3cBLLK65pePDM++TgkIC0a DYLJsEaO866vtdO8DRPhymRRsk2AM31H5IfdrQ1b1VSc23mD8cQLNLIOfuVuQ2K9q3Nd ebew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=w6atmUnu6yUDSAXWR/NCWLDY2ibxR/H8Klr3dPFutJk=; b=lerAevmQRox8tu3/WoOL3xQKlJU2eYutjNCBhsOUm6V1UqmPZfTSLdPn9Ed0YW9JEH jHd7ADDzwq2ZCmp8hvhc8vwL84tlpako9mIwD6eF9is3XYcgm5zTMHfSNJRjXg1ZjMxp LrO4+PsWOU9U3yia1gR1kSkGiqXm1e2RUL5j9PSmbEkJXC3nJYu3fNw7j9q88PqiFK/C dTPjEd9UWWB7cfrwc3yEKOuJFxMbwpmYX2F+pRTiA++WxnFlg8F3+WGWAvRYogcbEHE0 Jl/b9Q1DsOSGOFQkNTLRiRHUO6o3NpL6JXkgWbh5bcXZ+Qd2XOCbeYusg/iTiMq0QnFf Tcvg== X-Gm-Message-State: AFqh2kojjx/bu4L0SpDfPsJj+BF2NsbG0bV7X/GVOwkSruqWgS9tRPig lQ/Lbekx8M5FVRzAfqSzIugP995TDaU= X-Google-Smtp-Source: AMrXdXsl9VuIzTi1YDP9AOr3n22ZsWJIJjzwbfZ/VtjeaksWjFq/zU0R/aeLalaMsPVqstCi4BjEKQ== X-Received: by 2002:a05:6a00:4519:b0:58d:f047:53b7 with SMTP id cw25-20020a056a00451900b0058df04753b7mr7736844pfb.3.1674198260616; Thu, 19 Jan 2023 23:04:20 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id t25-20020a62d159000000b0056bd1bf4243sm10329149pfl.53.2023.01.19.23.04.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:20 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Joanne Koong , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 06/12] bpf: Combine dynptr_get_spi and is_spi_bounds_valid Date: Fri, 20 Jan 2023 12:33:49 +0530 Message-Id: <20230120070355.1983560-7-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=6180; i=memxor@gmail.com; h=from:subject; bh=Io8R4nMkdTkL0qipRPdCVbaGYoI8/FQxzBNP3ACEZXc=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzLM4sVtlsxGoBslFWC6lc7p/kxompbmt4+GNFv n46VPXCJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8ywAKCRBM4MiGSL8RynftEA C5s735WzUfN6oPcAztDYN9ZVxGYUXnwLToTI5XT4WfeVQvZ++gTPTJnjE3Mtf/jk9oOuIKqoZ8qkeE fBHGlyQLXol/XAvk63rh2fkXnH6CgNZpHyWS6avcPehYrWpKV1Kxgi+hiWJ13c5W5BZps58cX/ALPK BNwaJt2IejOFqlBb0KRQ1NH/ekCW75rbx1SWMBtDMPduSvKvC/yJNMVhqBjH4Za+q4/115i/LsfxC2 VZ08yZW0uDnQdsnGhgZJ1DhtkQJPgZTkQ+md+KmovxOHGwKgt0KIkbBUnUKbxKfs5wvKFXU+WmvpOi VWDl+zHs51s23cxCDW0DZa71BBRPmqT+fG9CCsWN/ruO5sxuzf5HQKcTN3oNnhWBzxodWX96xNkwbA WPlsRcBHXHfYSt3WeCJz10oVUEdyO1y5/4jGTN/RwWD6P8qTqUpO/nMF0Ns+7CqkSPG4fX3EXRnwbw G+dd2FXVQKeU2U2l+sKnEQo/HsH7uqyHOwcFvQetYvDzeUgN0qgygtodKftBGPjit1U8puKRK6s5lN uEkrTJIOeQQb4bdU0bqFAh6tMxPltTOmwXESOd3uZ/WH5jMhiMscIbXQYE07dEPt8p4J+L11ORviV7 6aYwZwL7NGaW5nn1TJOUVr6BS9+aSjlvweZA+XDPRQDocQOnyIkK39c3iyTQ== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Currently, a check on spi resides in dynptr_get_spi, while others checking its validity for being within the allocated stack slots happens in is_spi_bounds_valid. Almost always barring a couple of cases (where being beyond allocated stack slots is not an error as stack slots need to be populated), both are used together to make checks. Hence, subsume the is_spi_bounds_valid check in dynptr_get_spi, and return -ERANGE to specially distinguish the case where spi is valid but not within allocated slots in the stack state. The is_spi_bounds_valid function is still kept around as it is a generic helper that will be useful for other objects on stack similar to dynptr in the future. Suggested-by: Joanne Koong Acked-by: Joanne Koong Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 75 +++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 42 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index e5745b696bfe..29cbb3ef35e2 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -644,6 +644,28 @@ static int __get_spi(s32 off) return (-off - 1) / BPF_REG_SIZE; } +static struct bpf_func_state *func(struct bpf_verifier_env *env, + const struct bpf_reg_state *reg) +{ + struct bpf_verifier_state *cur = env->cur_state; + + return cur->frame[reg->frameno]; +} + +static bool is_spi_bounds_valid(struct bpf_func_state *state, int spi, int nr_slots) +{ + int allocated_slots = state->allocated_stack / BPF_REG_SIZE; + + /* We need to check that slots between [spi - nr_slots + 1, spi] are + * within [0, allocated_stack). + * + * Please note that the spi grows downwards. For example, a dynptr + * takes the size of two stack slots; the first slot will be at + * spi and the second slot will be at spi - 1. + */ + return spi - nr_slots + 1 >= 0 && spi < allocated_slots; +} + static int dynptr_get_spi(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { int off, spi; @@ -664,29 +686,10 @@ static int dynptr_get_spi(struct bpf_verifier_env *env, struct bpf_reg_state *re verbose(env, "cannot pass in dynptr at an offset=%d\n", off); return -EINVAL; } - return spi; -} - -static bool is_spi_bounds_valid(struct bpf_func_state *state, int spi, int nr_slots) -{ - int allocated_slots = state->allocated_stack / BPF_REG_SIZE; - /* We need to check that slots between [spi - nr_slots + 1, spi] are - * within [0, allocated_stack). - * - * Please note that the spi grows downwards. For example, a dynptr - * takes the size of two stack slots; the first slot will be at - * spi and the second slot will be at spi - 1. - */ - return spi - nr_slots + 1 >= 0 && spi < allocated_slots; -} - -static struct bpf_func_state *func(struct bpf_verifier_env *env, - const struct bpf_reg_state *reg) -{ - struct bpf_verifier_state *cur = env->cur_state; - - return cur->frame[reg->frameno]; + if (!is_spi_bounds_valid(func(env, reg), spi, BPF_DYNPTR_NR_SLOTS)) + return -ERANGE; + return spi; } static const char *kernel_type_name(const struct btf* btf, u32 id) @@ -788,9 +791,6 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ if (spi < 0) return spi; - if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) - return -EINVAL; - /* We cannot assume both spi and spi - 1 belong to the same dynptr, * hence we need to call destroy_if_dynptr_stack_slot twice for both, * to ensure that for the following example: @@ -844,9 +844,6 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re if (spi < 0) return spi; - if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) - return -EINVAL; - for (i = 0; i < BPF_REG_SIZE; i++) { state->stack[spi].slot_type[i] = STACK_INVALID; state->stack[spi - 1].slot_type[i] = STACK_INVALID; @@ -951,20 +948,18 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { - struct bpf_func_state *state = func(env, reg); int spi; if (reg->type == CONST_PTR_TO_DYNPTR) return false; spi = dynptr_get_spi(env, reg); + /* 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. + */ if (spi < 0) - return false; - - /* We will do check_mem_access to check and update stack bounds later */ - if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) - return true; - + return spi == -ERANGE; /* We allow overwriting existing unreferenced STACK_DYNPTR slots, see * mark_stack_slots_dynptr which calls destroy_if_dynptr_stack_slot to * ensure dynptr objects at the slots we are touching are completely @@ -988,8 +983,7 @@ static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, struct bpf_re spi = dynptr_get_spi(env, reg); if (spi < 0) return false; - if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS) || - !state->stack[spi].spilled_ptr.dynptr.first_slot) + if (!state->stack[spi].spilled_ptr.dynptr.first_slot) return false; for (i = 0; i < BPF_REG_SIZE; i++) { @@ -6160,7 +6154,7 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, if (reg->type == PTR_TO_STACK) { int err = dynptr_get_spi(env, reg); - if (err < 0) + if (err < 0 && err != -ERANGE) return err; } @@ -6668,10 +6662,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, */ if (reg->type == PTR_TO_STACK) { spi = dynptr_get_spi(env, reg); - if (spi < 0) - return spi; - if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS) || - !state->stack[spi].spilled_ptr.ref_obj_id) { + if (spi < 0 || !state->stack[spi].spilled_ptr.ref_obj_id) { verbose(env, "arg %d is an unacquired reference\n", regno); return -EINVAL; } From patchwork Fri Jan 20 07:03:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109278 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67CFBC25B4E for ; Fri, 20 Jan 2023 07:04:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229816AbjATHE0 (ORCPT ); Fri, 20 Jan 2023 02:04:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48154 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229814AbjATHE0 (ORCPT ); Fri, 20 Jan 2023 02:04:26 -0500 Received: from mail-pj1-x1044.google.com (mail-pj1-x1044.google.com [IPv6:2607:f8b0:4864:20::1044]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4274D530D7 for ; Thu, 19 Jan 2023 23:04:25 -0800 (PST) Received: by mail-pj1-x1044.google.com with SMTP id lp10so1324287pjb.4 for ; Thu, 19 Jan 2023 23:04:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=Tmo0le0cUJOBzWSdrlYEJJL3gLctaWM5TmJR0sXVqHw=; b=XtOhahdlROSujGFdGp0pfY38UYK0Ces0ozOf6MIHPQlYxxhQsNvOEfl0q9LStn9uA+ UqEH7P+ooAZBrcNYwcwbeYkY0H8ZiUVmvpTui2dw2BCpgkC/yg0vKQn5xSqyr7nK4dg8 lX4riTEI3ILk4bIw2nRIOMUgPynBHaXJ26Qa5TY/goHrgz+Phq69tCVg+0lqFEsHoNEd jwc767t6jEMVDsOYDoNetUIXucfAR7xzwEAV+LiryWgSVwh0fAQxd8pmrAxKqsfGxrdv Lds9Fg2/lX+1BcToa6KTh6kXEk4PLX69fRU8ejk5sg8UPzJQZMJ94xeWi5Zi85kG4ttE +/Ng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=Tmo0le0cUJOBzWSdrlYEJJL3gLctaWM5TmJR0sXVqHw=; b=VmB66OWr7MKEECKqb6zqTxBcAvaW0j97SqGTB1XWynRKQFDlD5JZavJ1r4UKKvl14y Xb8QUPh9uK+0lFpxQbFO3/XnfHKRC4AAwPEt6nHBa9jR7oSzRHoDg+V8l4h4izz6rz+I BpTKYZzIP+aTBUVbHKNxs9pBTLpZNiyQY0XbuT1tmeGVHwVw7a+62J+uGTGsysIhz+ke 7eHCvpkJFmpX06MajZeA8b7A0SsyZHM9outMUvpC9eOA4kH/uMJcppRa3ZqtUlFnj+h4 NVN9Dt9azOGIm8T0/Bx+nuENya+i2HVYYiQXqjyZPoBSvNeNSdypJdTUVZWIbq/Xw4aH gKuQ== X-Gm-Message-State: AFqh2kqJs6vR3ubQ3d048k/uqxpGbbhhvMcJleUJxvEYURu+ldUaW1P8 LS0Af6yyyAEtBJIDHNKoTiITj2mMhH0= X-Google-Smtp-Source: AMrXdXt2sJOY8RzHuwr6xg7oL4JMSi8NrlwdHfOcrzXhPdhbxk8qpX5vGQoCkM6QjTaMM2vTGY6Mjg== X-Received: by 2002:a05:6a20:2d12:b0:b8:5a85:6332 with SMTP id g18-20020a056a202d1200b000b85a856332mr16555131pzl.7.1674198264504; Thu, 19 Jan 2023 23:04:24 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id d21-20020a630e15000000b0047781f8ac17sm21676070pgl.77.2023.01.19.23.04.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:24 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Joanne Koong , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 07/12] bpf: Avoid recomputing spi in process_dynptr_func Date: Fri, 20 Jan 2023 12:33:50 +0530 Message-Id: <20230120070355.1983560-8-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3698; i=memxor@gmail.com; h=from:subject; bh=n4U1dXvfeNH1zRNe/co4rVPmlxHdSUzypJJY0IRJzjw=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzL1RvlX6RfReOLAFbMWC9wK/gvWsxIfRy4IvoV dOq3G5OJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8ywAKCRBM4MiGSL8Ryq0oD/ 9mkzu6cCVgXiBlhPGOymPfo9EFP7yJcdupRfv4YRBiYK3xFAo9QXGdvYdMXkVFb+yXmAI3xnk1eYFx xHICWScpikgh7ILscN526bbXW4o7WIQwr9dfuLVwV2eT1RBqJefF5GZe7YDm+NSlYcL9LJf6qFMJM/ aDpKKC+Cpd8v0pwbWg0UebUWwXd/IzVuSYYt58xF6+ZYHaRBcOUgj+20guOp41PdlvJv7doawhVv3d JN2sb/soYUnIStDQ2Y3JMPJAbOCCbS6KtKn5w2A7i4eqL9gyq71GD6gZb79NUQN/pFIMehh4tvWUpn WU5YV+dSxJdc8E9sVLC/RpSoveF7qN5h/6pWln5l0UF/e7zwkZmYWzXzGWUmQvUo1Ldx2IvtY/ubYQ Q7dm9dzf8cL7JnJ1e39ClYU3p5g4+UNeYvMyuqSuzStvB8kwpN4notqv6CN9lLYX/Zp+5DRxjGZSv+ DvXlb2wURChhZ9K8E1SYIA1uDIQemkx7bo7acPNPRmvsE/gwqE3RqxJmmNnK5IPxSBmCutQlc0614w Y+JeN8uRp3nD46AKFn1W8hamDl/C9pSfHHtG9FLf/DO2maizwxPhXQNMHGOORNa8n5N0IPBm9+McM7 EziQncQ+6pt8CdDWpt7qvriXWZDuJPYLTVsqca4L4tz4jST+wWUf437n03qQ== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Currently, process_dynptr_func first calls dynptr_get_spi and then is_dynptr_reg_valid_init and is_dynptr_reg_valid_uninit have to call it again to obtain the spi value. Instead of doing this twice, reuse the already obtained value (which is by default 0, and is only set for PTR_TO_STACK, and only used in that case in aforementioned functions). The input value for these two functions will either be -ERANGE or >= 1, and can either be permitted or rejected based on the respective check. Suggested-by: Joanne Koong Acked-by: Joanne Koong Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 29cbb3ef35e2..ecf7fed7881c 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -946,14 +946,12 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, return 0; } -static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg, + int spi) { - int spi; - if (reg->type == CONST_PTR_TO_DYNPTR) return false; - spi = dynptr_get_spi(env, reg); /* 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. @@ -971,16 +969,16 @@ static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_ return true; } -static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg, + int spi) { struct bpf_func_state *state = func(env, reg); - int spi, i; + int i; /* This already represents first slot of initialized bpf_dynptr */ if (reg->type == CONST_PTR_TO_DYNPTR) return true; - spi = dynptr_get_spi(env, reg); if (spi < 0) return false; if (!state->stack[spi].spilled_ptr.dynptr.first_slot) @@ -6139,6 +6137,7 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, enum bpf_arg_type arg_type, struct bpf_call_arg_meta *meta) { struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; + int spi = 0; /* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to an * ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*): @@ -6152,10 +6151,9 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, * and its alignment for PTR_TO_STACK. */ if (reg->type == PTR_TO_STACK) { - int err = dynptr_get_spi(env, reg); - - if (err < 0 && err != -ERANGE) - return err; + spi = dynptr_get_spi(env, reg); + if (spi < 0 && spi != -ERANGE) + return spi; } /* MEM_UNINIT - Points to memory that is an appropriate candidate for @@ -6174,7 +6172,7 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, * to. */ if (arg_type & MEM_UNINIT) { - if (!is_dynptr_reg_valid_uninit(env, reg)) { + if (!is_dynptr_reg_valid_uninit(env, reg, spi)) { verbose(env, "Dynptr has to be an uninitialized dynptr\n"); return -EINVAL; } @@ -6197,7 +6195,7 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, return -EINVAL; } - if (!is_dynptr_reg_valid_init(env, reg)) { + if (!is_dynptr_reg_valid_init(env, reg, spi)) { verbose(env, "Expected an initialized dynptr as arg #%d\n", regno); From patchwork Fri Jan 20 07:03:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109279 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 55653C25B4E for ; Fri, 20 Jan 2023 07:04:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229846AbjATHEc (ORCPT ); Fri, 20 Jan 2023 02:04:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48226 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229771AbjATHEc (ORCPT ); Fri, 20 Jan 2023 02:04:32 -0500 Received: from mail-pj1-x1044.google.com (mail-pj1-x1044.google.com [IPv6:2607:f8b0:4864:20::1044]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1702C66CC0 for ; Thu, 19 Jan 2023 23:04:29 -0800 (PST) Received: by mail-pj1-x1044.google.com with SMTP id z4-20020a17090a170400b00226d331390cso4023514pjd.5 for ; Thu, 19 Jan 2023 23:04:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=Zs5n2Q1YRbL+FZCik99wIL5O/1FCJ2BZ9GlI4NG+zGU=; b=nRR5tKBXlw+xafwDMdwemdBfma1IBjjJ/NzIRyU1T0DUZ409MW7mMJTsJUasFM7JG7 6EjEHGUlGcbJDuIP+MsksyD1FOUm8FUwzUjE/Qh1Nd/xnplHTzjPoEpQ3Nqkcgq4Mcqh 5w2ovOubfPyr8+t1JUlQCqM2SPr36ALOnhgMI58+O9LWyM0yprMtwVRhpaHJHosh71CD hvC3srqGU7jyqMEg0AxAt3vlV+rkqZY0H/FnfXVnpfdvAMWeLCwqGeC14wrOdvLrN53+ y36ppkxX3s/KG/mBPJRzsmvjLqbTXRespppS6OXw6mWIVcYPDjQ/O7wG3m7q/yviBTX+ ZvDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=Zs5n2Q1YRbL+FZCik99wIL5O/1FCJ2BZ9GlI4NG+zGU=; b=wKGCyJYn2xtS5Sg1Dr3wSAInAFG/IUt6uozxLRqUz6ZB3c3fPyeGJekvc+YSMnBmem BbvGnJaFs3fPDRDg3BvzX5a087xEQxq0iebvxcOXPZHO7pRDr84tSbWMhVhOor/OdJRC IjIglQ1+i++4LczVDV3IXvTCdTEv4aliE4umGu1EiXmqlshu9OJXHt9rv2p56slybDXx K5+Nm1WFgur2RFG1wcgqRChEzSWaci10aU4oLIFoUHV/Mq2mwLlAvzN/SMfUAanj4LkO uIm2ELxJS5qUjGAiM5mW4DWq0aQkMHgW92MwNSPYGi8OfTS7LGocqjifBMM0wHjz90jB fUAw== X-Gm-Message-State: AFqh2kqf3GyFgznAqwBoPiQzzwwHq71ckH5ck/Qg5EXwCwkiuQ0JdfWi bP37fNOK03ixSshPSg1Xf38DYjCTZTo= X-Google-Smtp-Source: AMrXdXu79TbOC7iHL9lGwXqyWhrcPBYe5/paLAkMsuBP6B6+UJwYxeLMof/f5szLGmRKRUurHP0XJA== X-Received: by 2002:a17:903:1c6:b0:195:e590:c7c6 with SMTP id e6-20020a17090301c600b00195e590c7c6mr1364477plh.22.1674198268290; Thu, 19 Jan 2023 23:04:28 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id e3-20020a17090301c300b00172cb8b97a8sm12487522plh.5.2023.01.19.23.04.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:27 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Andrii Nakryiko , Yonghong Song , Eduard Zingerman , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v4 08/12] selftests/bpf: convenience macro for use with 'asm volatile' blocks Date: Fri, 20 Jan 2023 12:33:51 +0530 Message-Id: <20230120070355.1983560-9-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1699; i=memxor@gmail.com; h=from:subject; bh=SNRMXDcMW1zluX6a3W/WYa1RbpYDzrfdzhpCt6yThTA=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzLs5JRMTd9QT6uBQgwWA2rgY8rE3jweRYNnMEY kA+RJ3KJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8ywAKCRBM4MiGSL8RyhcAD/ 48GFDqIMM3+H6w3L6v0biyh5kVkqymLHEJd962rUtTS/9oe5Fsv+cMVK4GA10HWyHNKZhJIso0cFK1 AKRZKA55RkHf429kzvOIfjt7iNMTNxOud45tbPnQNI/B+rOR3e5xIKLVS5ctDzNrbjbAw8l/Q1ISCH TuxWg3VZIlZNtcKB9pWxu8RlmcakIVEZqvbtS1JIbi/YPaAQDMVEYTsi+httVQq3skwdoS87D7KWe1 9Gid2vWgQ16NNKw6BN1X3Xiy1J+dSlf6qFqzmFzdJ+RfZBjLX90s6uTMWvziE5Oqn2Ahk564BeOgWf /fVb9ismDG+M+JKRktunXfTZtPomzYRoV7JKbZvVvzAh9v2Gu0ELKJW+Ypot4gU5RUotT/Kr4ZbzlL JcfaM4BIk5Ilcvt65c+tAOaRPXGtDkcCYMZTyvL83cBmp+/ymmCrrdYo1DByGEO6cdJxnVIxw+weUL naBmkxqIQ/kG1E2DJSziN/Ofqa6mZzAeF+6R6fUhQapWTZ3tqoSGa8QugLrFNIfICPs0w+PJGTM7Nw esaafTA7IaX3q/HCDIMhvFxiuAX8kk2B4g4RjhURX7/5Bj/Mzq8NqbUp9aDfIUKn3GA9tSnuiEI5+x Qov8J3NoZLfwLiyC+/q2XReOe8AqjQPVavoAQx5l1vyxtLoWUu28dDUD3lbQ== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net From: Eduard Zingerman A set of macros useful for writing naked BPF functions using inline assembly. E.g. as follows: struct map_struct { ... } map SEC(".maps"); SEC(...) __naked int foo_test(void) { asm volatile( "r0 = 0;" "*(u64*)(r10 - 8) = r0;" "r1 = %[map] ll;" "r2 = r10;" "r2 += -8;" "call %[bpf_map_lookup_elem];" "r0 = 0;" "exit;" : : __imm(bpf_map_lookup_elem), __imm_addr(map) : __clobber_all); } Acked-by: Andrii Nakryiko Acked-by: Yonghong Song Signed-off-by: Eduard Zingerman [ Kartikeya: Add acks, include __clobber_common from Andrii ] Signed-off-by: Kumar Kartikeya Dwivedi --- tools/testing/selftests/bpf/progs/bpf_misc.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h index 4a01ea9113bf..2d7b89b447b2 100644 --- a/tools/testing/selftests/bpf/progs/bpf_misc.h +++ b/tools/testing/selftests/bpf/progs/bpf_misc.h @@ -7,6 +7,13 @@ #define __success __attribute__((btf_decl_tag("comment:test_expect_success"))) #define __log_level(lvl) __attribute__((btf_decl_tag("comment:test_log_level="#lvl))) +/* Convenience macro for use with 'asm volatile' blocks */ +#define __naked __attribute__((naked)) +#define __clobber_all "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "r8", "r9", "memory" +#define __clobber_common "r0", "r1", "r2", "r3", "r4", "r5", "memory" +#define __imm(name) [name]"i"(name) +#define __imm_addr(name) [name]"i"(&name) + #if defined(__TARGET_ARCH_x86) #define SYSCALL_WRAPPER 1 #define SYS_PREFIX "__x64_" From patchwork Fri Jan 20 07:03:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109280 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F077BC25B50 for ; Fri, 20 Jan 2023 07:04:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229771AbjATHEe (ORCPT ); Fri, 20 Jan 2023 02:04:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48246 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229814AbjATHEe (ORCPT ); Fri, 20 Jan 2023 02:04:34 -0500 Received: from mail-pl1-x643.google.com (mail-pl1-x643.google.com [IPv6:2607:f8b0:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C195911659 for ; Thu, 19 Jan 2023 23:04:32 -0800 (PST) Received: by mail-pl1-x643.google.com with SMTP id c6so4588614pls.4 for ; Thu, 19 Jan 2023 23:04:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=JjuxCRFRYw9wjpCtAG75vdMCkmNxJZQw7fQxwyidGTs=; b=lNhqDH3VpR66ivmIRjRVaUyTZ7UbJVf4xGwfH+EfeAoEmJHgJepscRzis2T6zi/V95 2hgkFQHjlaMxGriJpaGM3i5K4ko7j2AonndnodjijbXdHzmqA6IVPur+4PfCdlr1F2Ng 59YWMVmyW9H0wn7s1rFKq6QATCrwmXiIjLz4rs2im60Ku35jccTNujFxphMTfUp5esKi YNmUPLvhgmwTDpjDO/iRph2z3yDMaXPehQby9aPi1mQhTcb9DXJDBQSUmOmWt3bhng8T It8IlLqQzvAhUqfp142KFKhzAS/kaMo36oaQpTZTlHaHLKZt82BHfUglMABgrUd9Kgw2 Uw9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=JjuxCRFRYw9wjpCtAG75vdMCkmNxJZQw7fQxwyidGTs=; b=z+ueeLnl5xuL2BJHmxXgUXMWvBHb61takgHu8AawKIaZWv50RAS9/+AJaY4tLBh8e8 gw4nW+C/b+9J/IPA6MK1AgskHx8PGRREqH/WP8lr6ilVtEUpyEglofRbO+pe3RDK1OMr 74qz++XrhTAifV0vzWBc/jaPqV9pUqy4OkSBoNt+INdIbkLSUgHNrMxX4HcdrJ5NaFZW KLXFy/23laXwFAd058Dt9ekOKLIYekGZt1gRaR19sWvNcim8EtX+lzxe48lHQW+MPynz NcMDN0vJtDTv/muEyF7WFaGj+AmHSB8tL65RhI4+uxLvb2/hLgdPaxojO4uiFqUzMjS8 UoBg== X-Gm-Message-State: AFqh2krQSzjpSz1QCJjH9SKSvX71wopChSWwCRaziRseh9CNoD6tK+1j rmgM8P2XnJefdreWUKtMN/AqwqB17E8= X-Google-Smtp-Source: AMrXdXt8UOvGMcbmx4T2sV5pvAx989xZk5KdwQ8c5EDEJYEpvvpmRA/mkrd3ceIowBOBAvLirPVMXw== X-Received: by 2002:a17:902:f708:b0:189:c4a9:c5e8 with SMTP id h8-20020a170902f70800b00189c4a9c5e8mr15749750plo.45.1674198272060; Thu, 19 Jan 2023 23:04:32 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id j3-20020a170903028300b001886ff822ffsm2742638plr.186.2023.01.19.23.04.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:31 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 09/12] selftests/bpf: Add dynptr pruning tests Date: Fri, 20 Jan 2023 12:33:52 +0530 Message-Id: <20230120070355.1983560-10-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=4291; i=memxor@gmail.com; h=from:subject; bh=KOeUuf38swlcblzdJeZUFs0p+ftuZYTeMeza4RnQIH4=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzLYF+eUnuacr+8EXLHi1Ln5LCgo7dSC6XL0jKR Zsw+QEeJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8ywAKCRBM4MiGSL8RyrkGEA CHqvPg2zQBvDGw9ffYTjQhkNXCmYtjSW+HDyqJhp2j96dht14hJe0TkbSedFTrPfnTzp79NCw+qfDf PABGC0w/ec++6Lwt/i6Hg9Bms7/AxI/QXfHjvaHZqWIS+DGc+VzUMkkhv8AXzcvMKPxLSVy/ihWWPS 3YFFRMfaKJlRbWg+z60Rmo6M5TsnJ8KwXPcTI2kFr4qiJ5Ni0I20EmXG6a+ahj82YrBC1rPHRDENtt IpljnJRm2SpaH+rCziwTnIPgs3Q6w5A0PMLvdbBafFHAxmbdFOktwbGr9/XekGb81OQtWlpFmBMK7p VA34wSn6fgimFnWhsvqPT4IQ7SmQrrM8hcWNeDylecqq0oU/DMqetwh8ResOONbWHtdymsk2S6rzCo ZsTtpWyVAjZlr7T0w9X1eZ/H2BRG7thsb5DcQGi/FXcSCb++ILwO/lFFA+kdXgY9Z2n404r7OCPTJJ 84Zk0y5tDpgXPx26pSqWa8GvxhVdl53bQ72rbbJEw9R7B3salnbeixWSnvFCOb35Do1bJ494+VXuGC CMIW2ZY7BZpnxuIGw9pP9Lx2ymcCB283nFkkc2gnxFEnIJCVrqyDve3SW4JeW4pYACHxMePT39VEUX phtfzbtFsu9XitKjIus3dogeQrh9IuZ16I0rPWLcUzW8EI+UWGi1fVg2IJbQ== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Add verifier tests that verify the new pruning behavior for STACK_DYNPTR slots, and ensure that state equivalence takes into account changes to the old and current verifier state correctly. Also ensure that the stacksafe changes are actually enabling pruning in case states are equivalent from pruning PoV. Signed-off-by: Kumar Kartikeya Dwivedi --- .../testing/selftests/bpf/progs/dynptr_fail.c | 141 ++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index e43000c63c66..f1e047877279 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -35,6 +35,13 @@ struct { __type(value, __u32); } array_map3 SEC(".maps"); +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 1); + __type(key, __u32); + __type(value, __u64); +} array_map4 SEC(".maps"); + struct sample { int pid; long value; @@ -653,3 +660,137 @@ int dynptr_from_mem_invalid_api(void *ctx) return 0; } + +SEC("?tc") +__failure __msg("cannot overwrite referenced dynptr") __log_level(2) +int dynptr_pruning_overwrite(struct __sk_buff *ctx) +{ + asm volatile ( + "r9 = 0xeB9F; \ + r6 = %[ringbuf] ll; \ + r1 = r6; \ + r2 = 8; \ + r3 = 0; \ + r4 = r10; \ + r4 += -16; \ + call %[bpf_ringbuf_reserve_dynptr]; \ + if r0 == 0 goto pjmp1; \ + goto pjmp2; \ + pjmp1: \ + *(u64 *)(r10 - 16) = r9; \ + pjmp2: \ + r1 = r10; \ + r1 += -16; \ + r2 = 0; \ + call %[bpf_ringbuf_discard_dynptr]; " + : + : __imm(bpf_ringbuf_reserve_dynptr), + __imm(bpf_ringbuf_discard_dynptr), + __imm_addr(ringbuf) + : __clobber_all + ); + return 0; +} + +SEC("?tc") +__success __msg("12: safe") __log_level(2) +int dynptr_pruning_stacksafe(struct __sk_buff *ctx) +{ + asm volatile ( + "r9 = 0xeB9F; \ + r6 = %[ringbuf] ll; \ + r1 = r6; \ + r2 = 8; \ + r3 = 0; \ + r4 = r10; \ + r4 += -16; \ + call %[bpf_ringbuf_reserve_dynptr]; \ + if r0 == 0 goto stjmp1; \ + goto stjmp2; \ + stjmp1: \ + r9 = r9; \ + stjmp2: \ + r1 = r10; \ + r1 += -16; \ + r2 = 0; \ + call %[bpf_ringbuf_discard_dynptr]; " + : + : __imm(bpf_ringbuf_reserve_dynptr), + __imm(bpf_ringbuf_discard_dynptr), + __imm_addr(ringbuf) + : __clobber_all + ); + return 0; +} + +SEC("?tc") +__failure __msg("cannot overwrite referenced dynptr") __log_level(2) +int dynptr_pruning_type_confusion(struct __sk_buff *ctx) +{ + asm volatile ( + "r6 = %[array_map4] ll; \ + r7 = %[ringbuf] ll; \ + r1 = r6; \ + r2 = r10; \ + r2 += -8; \ + r9 = 0; \ + *(u64 *)(r2 + 0) = r9; \ + r3 = r10; \ + r3 += -24; \ + r9 = 0xeB9FeB9F; \ + *(u64 *)(r10 - 16) = r9; \ + *(u64 *)(r10 - 24) = r9; \ + r9 = 0; \ + r4 = 0; \ + r8 = r2; \ + call %[bpf_map_update_elem]; \ + r1 = r6; \ + r2 = r8; \ + call %[bpf_map_lookup_elem]; \ + if r0 != 0 goto tjmp1; \ + exit; \ + tjmp1: \ + r8 = r0; \ + r1 = r7; \ + r2 = 8; \ + r3 = 0; \ + r4 = r10; \ + r4 += -16; \ + r0 = *(u64 *)(r0 + 0); \ + call %[bpf_ringbuf_reserve_dynptr]; \ + if r0 == 0 goto tjmp2; \ + r8 = r8; \ + r8 = r8; \ + r8 = r8; \ + r8 = r8; \ + r8 = r8; \ + r8 = r8; \ + r8 = r8; \ + goto tjmp3; \ + tjmp2: \ + *(u64 *)(r10 - 8) = r9; \ + *(u64 *)(r10 - 16) = r9; \ + r1 = r8; \ + r1 += 8; \ + r2 = 0; \ + r3 = 0; \ + r4 = r10; \ + r4 += -16; \ + call %[bpf_dynptr_from_mem]; \ + tjmp3: \ + r1 = r10; \ + r1 += -16; \ + r2 = 0; \ + call %[bpf_ringbuf_discard_dynptr]; " + : + : __imm(bpf_map_update_elem), + __imm(bpf_map_lookup_elem), + __imm(bpf_ringbuf_reserve_dynptr), + __imm(bpf_dynptr_from_mem), + __imm(bpf_ringbuf_discard_dynptr), + __imm_addr(array_map4), + __imm_addr(ringbuf) + : __clobber_all + ); + return 0; +} From patchwork Fri Jan 20 07:03:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109281 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 93652C05027 for ; Fri, 20 Jan 2023 07:04:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229854AbjATHEh (ORCPT ); Fri, 20 Jan 2023 02:04:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48278 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229814AbjATHEg (ORCPT ); Fri, 20 Jan 2023 02:04:36 -0500 Received: from mail-pl1-x643.google.com (mail-pl1-x643.google.com [IPv6:2607:f8b0:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 40C1660CB1 for ; Thu, 19 Jan 2023 23:04:36 -0800 (PST) Received: by mail-pl1-x643.google.com with SMTP id c6so4588724pls.4 for ; Thu, 19 Jan 2023 23:04:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=LqKHgLPVOuLzH7ZCR7QAiED+nBVGYayNRgcRrmJ2ARk=; b=g+55hz6CKQlVjap1KIwT0fHrCka2aXGuekfL0aqagViDozBRRoFuWcffjARfzMURBE WqnZliHVk7QFLO+r2lo8MPMtkQp6cakW263c71iLfX0L1O8Ocms7m0FZDNC38Xg0tLMR Rkq5TXSPJc0iseyp+DedGxK14gxFpaig/0UgLZKfgsav/uxmPq8tebSCPxiEB86CG0Au TUNjonLOADuA5YxQV1hKSG8zkUB9qX46OfNbR2H3ue8QSbI4q3kPn7Kf+rjBJ0MXGKMs al3V13Ktia16FwXge5oap/BMgcZ0SzrtZFw7XyvEWTTqOD4/6qxY2/oV4cBoOAxYAuX9 i7qA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=LqKHgLPVOuLzH7ZCR7QAiED+nBVGYayNRgcRrmJ2ARk=; b=GipSUqos5sIa9JyJQ1mF7zdr6G5oVcCw0vNQ5iG2Sso3rpoTTmLoAik3dBpKIKznoS 6XPtbZ3u+Qa8hoRPwwpKq8EIiiXTDj5Exy6KoxlER3G/xLP7mcs5b8zEWUs/Mh993r6L IO0uMeYJGkHCnu+FUzQKYGEMiRJ3T8Gj9g1KlqXYVM4boVro/GuF3rN9rIVZ5igc/jsL hieKq0Y/PjMFdeyj7NdPxzG7GyD5UEqizhiqDMNirZ/Nnn6MU4aGk4yj+orlKvBAcNj5 muuUAHZlOWK+CH6nsPZHzHsulz0dKDZzPt7TIEahy1b97L5xHYscCwMPbuA3F23tLRT7 gJTQ== X-Gm-Message-State: AFqh2krPVlfzoxPR0XO6hfUGBqJYoz+85xGRHKwVmSfuXoOw2QrX24+c 66RubTVhyT3xp16Apf9mogLv74IfLXE= X-Google-Smtp-Source: AMrXdXuxQ5mUFHDzgyGAKN+14gpTZD9GE/HqGdGwPjGtMP5Mk7TgdlRhkAyaXBH7cak7VEhYll23gA== X-Received: by 2002:a17:902:b213:b0:194:5c63:3638 with SMTP id t19-20020a170902b21300b001945c633638mr13569307plr.61.1674198275450; Thu, 19 Jan 2023 23:04:35 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id q14-20020a17090311ce00b00189c536c72asm26287873plh.148.2023.01.19.23.04.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:35 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 10/12] selftests/bpf: Add dynptr var_off tests Date: Fri, 20 Jan 2023 12:33:53 +0530 Message-Id: <20230120070355.1983560-11-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1662; i=memxor@gmail.com; h=from:subject; bh=35zrMYiHjIev+NoG4HbRoMpjPSoBEo4hvE45xRAMx28=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzMPR7d9Dz+Hqxsm4lJqmrpIOh2VEe9eLixVhUX WICorMeJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8zAAKCRBM4MiGSL8Ryjr+D/ 42KWDPofMYcKxqPqlCxCe8QQnbsTlaUo928JgYm3TwLoZeW1U9u3/WKzLrmdrGH3RuwRtIukca8+W+ 0gSVHuh/tvImI2mWhwwofN4ItJpq+1grSNC9/sSjhl34RIgDl4F+T241/A7fTwqusASfRTWoC1T/Jj szU/NuwdT66x3Qevyrr+ZmEWTMW6nw9zB1GakEs81AEUXSsnczraWH0F2vH/gB4b0ur+ygCxowtO03 bE/DlMFpzYU4RomLTfI+G8Vj1ms1Fmslfgl9Wu/cxWiMOzWjjVYnKNLgWtHpLhVZK0aS4satRLKqpk POdRiAwhDu8bMaQZztWHaPcaG26KhJuF/nLr/rrF4Z3cc4eojD/3/FfHExHW4bR+8PqsylCi719LqK 99AQFtsfainAmaKavoSpC8UBX4v4HvdbeWkIFueJQU5ywqTMuAhF2+us2PbeVTYN4UoRa8ntbpDyf8 36cboYg65k4PK5FCbHMSBBCslv4VlHYvcH9mG8dHfS9Hn5dTsaYAAHT90aGNv/9BoRalO15FZWqW4k RLsv1Gm+P8sWXCUq7mD2c+qXwNx6E6KQMT4V8FS61dMFGv/+8r/caDhuJ+WO+H43ShXKgOwYR+er5h ZhNo7oIm7bZrIpirpjYkiPzX7x7dpcwg8uFO/37vc/H5xSdpXAsm2ekLByqw== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Ensure that variable offset is handled correctly, and verifier takes both fixed and variable part into account. Also ensures that only constant var_off is allowed. Signed-off-by: Kumar Kartikeya Dwivedi --- .../testing/selftests/bpf/progs/dynptr_fail.c | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index f1e047877279..2d899f2bebb0 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -794,3 +794,43 @@ int dynptr_pruning_type_confusion(struct __sk_buff *ctx) ); return 0; } + +SEC("?tc") +__failure __msg("dynptr has to be at a constant offset") __log_level(2) +int dynptr_var_off_overwrite(struct __sk_buff *ctx) +{ + asm volatile ( + "r9 = 16; \ + *(u32 *)(r10 - 4) = r9; \ + r8 = *(u32 *)(r10 - 4); \ + if r8 >= 0 goto vjmp1; \ + r0 = 1; \ + exit; \ + vjmp1: \ + if r8 <= 16 goto vjmp2; \ + r0 = 1; \ + exit; \ + vjmp2: \ + r8 &= 16; \ + r1 = %[ringbuf] ll; \ + r2 = 8; \ + r3 = 0; \ + r4 = r10; \ + r4 += -32; \ + r4 += r8; \ + call %[bpf_ringbuf_reserve_dynptr]; \ + r9 = 0xeB9F; \ + *(u64 *)(r10 - 16) = r9; \ + r1 = r10; \ + r1 += -32; \ + r1 += r8; \ + r2 = 0; \ + call %[bpf_ringbuf_discard_dynptr]; " + : + : __imm(bpf_ringbuf_reserve_dynptr), + __imm(bpf_ringbuf_discard_dynptr), + __imm_addr(ringbuf) + : __clobber_all + ); + return 0; +} From patchwork Fri Jan 20 07:03:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109282 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3B7B1C05027 for ; Fri, 20 Jan 2023 07:04:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229573AbjATHEl (ORCPT ); Fri, 20 Jan 2023 02:04:41 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48334 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229814AbjATHEk (ORCPT ); Fri, 20 Jan 2023 02:04:40 -0500 Received: from mail-pj1-x1044.google.com (mail-pj1-x1044.google.com [IPv6:2607:f8b0:4864:20::1044]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 880ED241D9 for ; Thu, 19 Jan 2023 23:04:39 -0800 (PST) Received: by mail-pj1-x1044.google.com with SMTP id dw9so4738980pjb.5 for ; Thu, 19 Jan 2023 23:04:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=yqzLD9MYr6KiuRpe9sBVIUYjbcoAjTXay8FSvvNcEzA=; b=gt/CfO6t5JjH8WUQDRYfLn1I7TT39faaIUUenEuJQNmjG9BL2PWBE0H9aNMCF0Gm5I nAmR35nyDyGMxjkFyFFxq+bL9sR/voBSe3TrKq4BMgpVVBMNmXe+f8V12oKfGSSoI06i GwoQWJ796thu/yrFNPdLLrudYybSuTX2H/dy6D4eL9fdHxCmkWHVFuE/n63iMzka/Pfb MbbPJS1fhuxVWD64Hk2xYQAJsTaKlSjxLneOsP0RjEJLb1zKlrmBq94RlQasRgkkMhg5 TQC+WFJgDJp0m6/TS7LwEFGA49MahNTdu3U5K4GOmlZBRAuYmu/2kzwF6zAPMOuk9bEm nnUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=yqzLD9MYr6KiuRpe9sBVIUYjbcoAjTXay8FSvvNcEzA=; b=JD6zEAaVGCl/+nSjh+GBSkipaN5kS97zLAz8toaH6nsUO0r/OIVznda3SlZ74GbREe 8y5D/MhgKHFaoJdGttSz/Ee4IiaiMHCX2R7JL4agUCZgUEVbW8X9cDG43UoWoXeut2dK 4YoeRQNx7TV7WDSl/4KalfTBByPQAbYfsOTtIG2eMQVXpnanS3ECbJ9BJw9hcT7iZPRP bMuXfBftnS6vR+r27vnNPMOyBm/gf0QeyR3Jbn/VOr/tGtSFY1oU/jYRysXRQryoZq4N PaEc2Aej1nIqryTKXE+6ibwAAHyh0EF+QefXhK2ztsSL/hUvtgkW9NqQbo6VnfBDCCtz xvfw== X-Gm-Message-State: AFqh2kovXz9UaTLy6oIPpn8VkQKLPvUxlBo8w6muPsjdf72nEx7h8xq0 0GdgOGAwv+IDCM4eWR9GuvuSfTQl6qE= X-Google-Smtp-Source: AMrXdXsLyhjnJh+jqTfhEbfWRM9YZ5TDPsHh8jRgF7xPHpBWUUBVpZPj3HbIq4l4vRXenOq879I7gw== X-Received: by 2002:a17:90a:4964:b0:223:a079:7354 with SMTP id c91-20020a17090a496400b00223a0797354mr14508867pjh.15.1674198278859; Thu, 19 Jan 2023 23:04:38 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id m1-20020a17090a3f8100b00218f9bd50c7sm715471pjc.50.2023.01.19.23.04.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:38 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 11/12] selftests/bpf: Add dynptr partial slot overwrite tests Date: Fri, 20 Jan 2023 12:33:54 +0530 Message-Id: <20230120070355.1983560-12-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2463; i=memxor@gmail.com; h=from:subject; bh=vb2M3d6AP1tJHi9lacIFMdvNjgTaEGGdyWxaqNz5OHI=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzMXtdYoVT+uua2rJKUZ75sLYaxpcQxs+Mg4Z+P 6ikII2eJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8zAAKCRBM4MiGSL8RysOwEA CzsW2KdhXJk+mUEiM7kB9GWRnJBQYx+WrEBj2hmRaHwsuqvD02VAh+C0SMt8O8HD9Udy5lzD2+hJba U5B0bAwciaUMqWYzJiV6wvUrzcFkm8sAof4lTkpnazxMTTwC0VeXhpLMm1WF7dHekUoCz+TNQ9d0iH rismWlQipzLWVSYCq1kqgy0ICxk/KcoVgsaS0LM5klj/EjuTlDahlqm5+XRixg16VeI2AJ/91XvZ1L FTK/Uu2D08n7OfcHqpO3lN72nXZy0LRb++zKUYzMbvhiYQQZl0N91oS5SEmw/5Y8N1jiz540j1zQrT m2T8fRheMvfBwtDVWgQyB0M+JZwt3InPCSAvhqhIBcvrW869J7HFo+7JQegLYEC9SGfEhr1ngz7gky CV9zCLktQ8oFDH0UdiVPDiBn68Wem9K2Zf0VppNfSQUveuiKHrHeOCgZAFP/qDa5o341na/+mQ+a8s t0CjJQlCESgOY2g8sGXFxhQJ5X4xyt/Em1S/ppxHoAhClhvIdT7lp1H4ZQXECbImD1GUQZDFI/OLcF VKYv+dJsLfJ0t8peGOtKqVVco2A3HjS8j3GaYyBeInG0FPpaFvinMQ9hAWWQbrWBRkwY6fxCnW/bOt 2rk54GryLMrW8K9bJJz4zZCyCzxqpK6NjIvCoyldgfryly4sm1LzH1gNn4jw== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Try creating a dynptr, then overwriting second slot with first slot of another dynptr. Then, the first slot of first dynptr should also be invalidated, but without our fix that does not happen. As a consequence, the unfixed case allows passing first dynptr (as the kernel check only checks for slot_type and then first_slot == true). Signed-off-by: Kumar Kartikeya Dwivedi --- .../testing/selftests/bpf/progs/dynptr_fail.c | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index 2d899f2bebb0..1cbec5468879 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -834,3 +834,69 @@ int dynptr_var_off_overwrite(struct __sk_buff *ctx) ); return 0; } + +SEC("?tc") +__failure __msg("cannot overwrite referenced dynptr") __log_level(2) +int dynptr_partial_slot_invalidate(struct __sk_buff *ctx) +{ + asm volatile ( + "r6 = %[ringbuf] ll; \ + r7 = %[array_map4] ll; \ + r1 = r7; \ + r2 = r10; \ + r2 += -8; \ + r9 = 0; \ + *(u64 *)(r2 + 0) = r9; \ + r3 = r2; \ + r4 = 0; \ + r8 = r2; \ + call %[bpf_map_update_elem]; \ + r1 = r7; \ + r2 = r8; \ + call %[bpf_map_lookup_elem]; \ + if r0 != 0 goto sjmp1; \ + exit; \ + sjmp1: \ + r7 = r0; \ + r1 = r6; \ + r2 = 8; \ + r3 = 0; \ + r4 = r10; \ + r4 += -24; \ + call %[bpf_ringbuf_reserve_dynptr]; \ + *(u64 *)(r10 - 16) = r9; \ + r1 = r7; \ + r2 = 8; \ + r3 = 0; \ + r4 = r10; \ + r4 += -16; \ + call %[bpf_dynptr_from_mem]; \ + r1 = r10; \ + r1 += -512; \ + r2 = 488; \ + r3 = r10; \ + r3 += -24; \ + r4 = 0; \ + r5 = 0; \ + call %[bpf_dynptr_read]; \ + r8 = 1; \ + if r0 != 0 goto sjmp2; \ + r8 = 0; \ + sjmp2: \ + r1 = r10; \ + r1 += -24; \ + r2 = 0; \ + call %[bpf_ringbuf_discard_dynptr]; " + : + : __imm(bpf_map_update_elem), + __imm(bpf_map_lookup_elem), + __imm(bpf_ringbuf_reserve_dynptr), + __imm(bpf_ringbuf_discard_dynptr), + __imm(bpf_dynptr_from_mem), + __imm(bpf_dynptr_read), + __imm_addr(ringbuf), + __imm_addr(array_map4) + : __clobber_all + ); + return 0; +} From patchwork Fri Jan 20 07:03:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13109283 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F2DA1C05027 for ; Fri, 20 Jan 2023 07:04:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229532AbjATHEp (ORCPT ); Fri, 20 Jan 2023 02:04:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48386 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229814AbjATHEo (ORCPT ); Fri, 20 Jan 2023 02:04:44 -0500 Received: from mail-pl1-x644.google.com (mail-pl1-x644.google.com [IPv6:2607:f8b0:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0962A530D7 for ; Thu, 19 Jan 2023 23:04:43 -0800 (PST) Received: by mail-pl1-x644.google.com with SMTP id d9so4559440pll.9 for ; Thu, 19 Jan 2023 23:04:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=YyTWOZilBFDAJAIdzvGkhUiQOCd5v7yklxBaXynN35o=; b=T1y1G6871uDzt9PPygMPduYST6f5C4V9yqZrJDfNs4SYywD2qY3NoMa5yS0NX4yNY2 p4w6EmjMcs7a6Y0ENSDAOY0oNs+E3lgUfT23JCnacoeIZ8SnwjlcFzQgN4j+PxAFALZq 7HO+ktUa5RHBtaDkcZNoqhIWn9pwcFAbGrIzCO4sHlBXfpr5I0iOOZrf0+f1YjcikdrN AAkV4W6OeEXqve/plCI4NvodrX/WfbAbZcgc9yTISC2pTrZUn3q47pcqEhAvB/pFFyuv 6ZAOvRhSQdyhsM5enKImUtK9qb7Ak5z/Yl2Ce1cDjfxOgzZ0NzxVsFOyqKW0o8h0r18W NZdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=YyTWOZilBFDAJAIdzvGkhUiQOCd5v7yklxBaXynN35o=; b=HPWWwAPo53MLW/Sv6sou/O07QOWy06YWeeKqLBMAyeuoyH2cZYYVRZb/DIoPqP59jI SZqW88m4vhKFJRktul2u8loQ9GzvOqwF+El98AQwLGEGhmasRLTe423erfF8d0C0/jae GnZDPXv1rXk+Nhn63ce48yQlipPT+tAf3nF6fljjxKbxGRCzKEyGfMzM2Y7Hok7Z7yFi Fw+Vrv2pY3A/KABCHhTT1obyALzPQlBs9qb2hm4GQt+W9OVbKy1KrGylBOA9EW+8crPb 7YJ4AyOERP4cND7iumGRfn5clT/D+5HEvY/d93qpl5+XF32iJoLV5upcCw+2C9oD/yJA CmNQ== X-Gm-Message-State: AFqh2krPUbH903bOjSk797DlDfaI9ZJ7v5baURlTBbjx6xuzK5nHzgE9 w+G9En8StHCwnbZ3CnluqzvXzzFG4nE= X-Google-Smtp-Source: AMrXdXvctzyr80C+xnoA7AxnmV3JFBhr1Tthefh4TvYkblcA3n5VJmIR58W3ZF+zCtPEkXm3XX9Qmg== X-Received: by 2002:a17:90a:f30e:b0:229:27a2:d80f with SMTP id ca14-20020a17090af30e00b0022927a2d80fmr14584940pjb.23.1674198282228; Thu, 19 Jan 2023 23:04:42 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id n7-20020a170902e54700b00194ab9a4febsm6309823plf.74.2023.01.19.23.04.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 23:04:41 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v4 12/12] selftests/bpf: Add dynptr helper tests Date: Fri, 20 Jan 2023 12:33:55 +0530 Message-Id: <20230120070355.1983560-13-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120070355.1983560-1-memxor@gmail.com> References: <20230120070355.1983560-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3628; i=memxor@gmail.com; h=from:subject; bh=KrTMop4yccSalaFbkhQLR70jCCGcHJeO+aSXiPpo0lA=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyjzMJ5kE1rcwnr3HwfkUejt1ifNTCMP0HJbgxP48 p5T38yeJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8o8zAAKCRBM4MiGSL8RylxLEA CpxqIv0gutmrt8bnDHFI1StkTS4+jClyK0gTAi6VLPH5BwlPUyHMZfe9/pafsUPwg3AJWHziqWhJ0e VM8jLzrRbyd9ol2IEUVJjgDoGEqtaX/UPWGV0V5vr8UX0ARa8YmL85SVlIDqyqKVRT2SOzeNiLIioK /c6sw/UI01Kn1YuIpI7UQw6bsGfnZoeNUhVHtA+lt9DaJwDenb88nLUNJ+0DuEtSYb+0KjUzcfporl 3MNJbdNtBLQopchNBGIpyZVdO8Ld81hsusU3t7W/xACwLi3yEyHqMPnE7s4M3vI9pTWVbRBUf4DBYQ +/2SLaWauIKKyCO4ghAgLRiztXEnaI3l22iIWfgtpO2bH1s2qxsksytSOWjV7P5UIPfszJ39MbOtpq k1gnfL9Zc/E1GhgYOYYErYzRKMGMw8bVsHbq6GjXdgnjMxg4YG01PAFAMaoc9Vux3JpRISciBcx+ke fR9ADycoC1UNRb6mITk6chIiZ+XzQavAqPR+xsRwQnp4qE7U/ywAmYLo4dgDK08ToCr4oS5TwCc4A6 Yv/2dzcM/1AIq9Qr/6GUAlhBEoW+m0tZG2YcKgYC+z04fosbg0qfmz5ehtK7CPdBOETH4W5pFsuq19 8LYf4RukK+HGF69WGG84+17vJVaNjhczfUB6iKkYQHirhPy3DH67QroMpV2Q== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net First test that we allow overwriting dynptr slots and reinitializing them in unreferenced case, and disallow overwriting for referenced case. Include tests to ensure slices obtained from destroyed dynptrs are being invalidated on their destruction. The destruction needs to be scoped, as in slices of dynptr A should not be invalidated when dynptr B is destroyed. Next, test that MEM_UNINIT doesn't allow writing dynptr stack slots. Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: Joanne Koong --- .../testing/selftests/bpf/progs/dynptr_fail.c | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index 1cbec5468879..c10abb98e47d 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -900,3 +900,132 @@ int dynptr_partial_slot_invalidate(struct __sk_buff *ctx) ); return 0; } + +SEC("?raw_tp") +__success +int dynptr_overwrite_unref(void *ctx) +{ + struct bpf_dynptr ptr; + + if (get_map_val_dynptr(&ptr)) + return 0; + if (get_map_val_dynptr(&ptr)) + return 0; + if (get_map_val_dynptr(&ptr)) + return 0; + + return 0; +} + +SEC("?raw_tp") +__failure __msg("R1 type=scalar expected=percpu_ptr_") +int dynptr_invalidate_slice_or_null(void *ctx) +{ + struct bpf_dynptr ptr; + __u8 *p; + + if (get_map_val_dynptr(&ptr)) + return 0; + + p = bpf_dynptr_data(&ptr, 0, 1); + *(__u8 *)&ptr = 0; + bpf_this_cpu_ptr(p); + return 0; +} + +SEC("?raw_tp") +__failure __msg("R7 invalid mem access 'scalar'") +int dynptr_invalidate_slice_failure(void *ctx) +{ + struct bpf_dynptr ptr1; + struct bpf_dynptr ptr2; + __u8 *p1, *p2; + + if (get_map_val_dynptr(&ptr1)) + return 0; + if (get_map_val_dynptr(&ptr2)) + return 0; + + p1 = bpf_dynptr_data(&ptr1, 0, 1); + if (!p1) + return 0; + p2 = bpf_dynptr_data(&ptr2, 0, 1); + if (!p2) + return 0; + + *(__u8 *)&ptr1 = 0; + return *p1; +} + +SEC("?raw_tp") +__success +int dynptr_invalidate_slice_success(void *ctx) +{ + struct bpf_dynptr ptr1; + struct bpf_dynptr ptr2; + __u8 *p1, *p2; + + if (get_map_val_dynptr(&ptr1)) + return 1; + if (get_map_val_dynptr(&ptr2)) + return 1; + + p1 = bpf_dynptr_data(&ptr1, 0, 1); + if (!p1) + return 1; + p2 = bpf_dynptr_data(&ptr2, 0, 1); + if (!p2) + return 1; + + *(__u8 *)&ptr1 = 0; + return *p2; +} + +SEC("?raw_tp") +__failure __msg("cannot overwrite referenced dynptr") +int dynptr_overwrite_ref(void *ctx) +{ + struct bpf_dynptr ptr; + + bpf_ringbuf_reserve_dynptr(&ringbuf, 64, 0, &ptr); + if (get_map_val_dynptr(&ptr)) + bpf_ringbuf_discard_dynptr(&ptr, 0); + return 0; +} + +/* Reject writes to dynptr slot from bpf_dynptr_read */ +SEC("?raw_tp") +__failure __msg("potential write to dynptr at off=-16") +int dynptr_read_into_slot(void *ctx) +{ + union { + struct { + char _pad[48]; + struct bpf_dynptr ptr; + }; + char buf[64]; + } data; + + bpf_ringbuf_reserve_dynptr(&ringbuf, 64, 0, &data.ptr); + /* this should fail */ + bpf_dynptr_read(data.buf, sizeof(data.buf), &data.ptr, 0, 0); + + return 0; +} + +/* Reject writes to dynptr slot for uninit arg */ +SEC("?raw_tp") +__failure __msg("potential write to dynptr at off=-16") +int uninit_write_into_slot(void *ctx) +{ + struct { + char buf[64]; + struct bpf_dynptr ptr; + } data; + + bpf_ringbuf_reserve_dynptr(&ringbuf, 80, 0, &data.ptr); + /* this should fail */ + bpf_get_current_comm(data.buf, 80); + + return 0; +}