From patchwork Fri Jan 20 03:43:03 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: 13109088 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 D7567C38159 for ; Fri, 20 Jan 2023 03:43:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229449AbjATDnX (ORCPT ); Thu, 19 Jan 2023 22:43:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44474 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229437AbjATDnW (ORCPT ); Thu, 19 Jan 2023 22:43:22 -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 065D7A2951 for ; Thu, 19 Jan 2023 19:43:21 -0800 (PST) Received: by mail-pj1-x1044.google.com with SMTP id bj3so4469072pjb.0 for ; Thu, 19 Jan 2023 19:43: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=VaLkm+F26DdGfFgA0j+DUYDbXVbQ5FuBOQ58rlSnxCU=; b=oLFgQ3Aff6rTQEmptdnbu1sZqAqmqufGQJnIzn5T+hDYlrCfA9/IngnKFPMwryIfUJ rnLjWk8IcSoQp0frmzKHu+9KPeTBy0KcWnmkJjlw/Vfw+ytxchthOOpyWDTeUJxLSSWw baQVw8KZCNjN48xK51KqtP5nY8aj2N7TgUPFMH17bIa2BzEDc5vQPts/3kRDrzwMuJoj Yz0x7Ogom+ccFWfAUt/AGtG62LFjiCJuHtklbD/xaGZJru6IQlKnWMPx1k4Bt75uyTNQ tQJYHx71rqpbsLCdjtHdWvDr2cca49KQG9e305elfcfeLQK0PfQ6tZvFu6/8FXnOB7sp UQKQ== 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=u+UVO4ueq+2fE1dCTGD7a5IAbaDuXtlG2/Ahmf4GLQQv+6yb1CcjErsfk1CbJdZUCM OsiMf2Ee8BCy0qSgAwviruwT52Bz3u3pUGK7ovnUORZUkN/Shl1r9/cTRHvT3B8CxxQx AGqAjbbms+zSPVOKlg/qUP0AY1gWvp/SKOotQuqRuOdv4/DymSNegubquKM92Zh9N/Lt WaG/1ZPKfzQpmxBlYcO/+UMScaytc7+SGaBxpCBPzCBii13lYmiaL+VKG6ugDRsE2SJj 75n6DZPeZCYiLZk1oRU2i3yb7h8sGVgHvlSKBxiQ36telRmBwqTFAZXfzjIS1BkzMz1S ylvg== X-Gm-Message-State: AFqh2kprJaucUVXvfkV6jxzerbug4i9dBfS46LASlFlOH5FPM9Z8jfWR K6shAtTxzkXZKXDXK8cbd7XsM14Nuas= X-Google-Smtp-Source: AMrXdXuR+aGSoOED9RyU4m7vtVqhiCAONlcj1c11i55FZC6CQmt1P5QzdVYLN6yLFe0aj6eGq39S2g== X-Received: by 2002:a17:902:d2d1:b0:194:c5d6:328 with SMTP id n17-20020a170902d2d100b00194c5d60328mr8268043plc.35.1674186200186; Thu, 19 Jan 2023 19:43:20 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id i11-20020a170902c94b00b00194ac38bc86sm5896698pla.131.2023.01.19.19.43.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:19 -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 v3 01/12] bpf: Fix state pruning for STACK_DYNPTR stack slots Date: Fri, 20 Jan 2023 09:13:03 +0530 Message-Id: <20230120034314.1921848-2-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-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/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg28L83VYouCKa4i4kZhtgc+y/4A2p1iM42Byfm9 YpAPpFSJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvAAKCRBM4MiGSL8RyhTOD/ 4yvQzALfFh6wRq0aBJjRyL+mvkBTmZ7o8JJUhJmFxZXdoaEfCzRa+IDUgNrZOCQCS2y5s7Kef9OAG1 RJRqqoEmtH3tRqi26nXzzwiovWEHE+kzIk1wnGg9XK7V5vFJ1O1V76wNtMzShZ5KKdc5fJO5iQoQVe EgzojSCY/kRFUEBrbUXr4kziVS41pK90VcoSo2H2EHnn9nxjlEABH3WZiHtPDeSN+vbZWsBYIFo9qs WyrQpRscNFomqwM4cxdbM6QAQKwGmZgHA0ObmdQE6v59nhrqUZiXlPcANVg2H4Dxwq94t/LHu/kz42 6WVXPKVGoABN4uj2gcrNjd4rr7jAh154mDpwc49zQeGeN6NPBiF7bvkvR7UZLmp/iO+krxOj9H8GzF RcIfL1kCP+GL0GZDsVY4YH/VLXU2RnN7a+pzsH0LlGaiPQQ0cV46YRdJl5Xd/3jpUdCuVqHjmFzzVg y4dPld/vzTHkcAxSRujfoBhcVti3WyI/ncbA0dcwcnaILn3sQytQVcl0XTwDdMlzbXcDWu4oYn0z/F sLkNHzyxESOky1NeYI0y9PxH2km1/WmVKlK4q61K5qJZDFawNBl6lPBVSSrOKQfWhBLTmkXoT7tNUH P45r9+LwPzrz4Thzc4zxq5CXMd5WgpMupMtOIp3KRwKD//r2Fg4vHPlk7VxA== 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 03:43:04 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: 13109089 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 C67A4C38159 for ; Fri, 20 Jan 2023 03:43:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229437AbjATDn0 (ORCPT ); Thu, 19 Jan 2023 22:43:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229595AbjATDnZ (ORCPT ); Thu, 19 Jan 2023 22:43:25 -0500 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 45DE8B1ECB for ; Thu, 19 Jan 2023 19:43:24 -0800 (PST) Received: by mail-pj1-x1041.google.com with SMTP id lp10so990849pjb.4 for ; Thu, 19 Jan 2023 19:43:24 -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=BAxhM1kHH1vD6kmS3jNy4BO8RKxnGEl0+UtmkSovJausohrZnSidVS9ITwnckn7tfJ m3iiYLTe3hgQV1Gw6+vz7sXOcdnLfcMPmPvEN2eGX/jnUFr8DuncBTL36ROw74eldmIf 74KmeBI/+mJWQc10ciHDMAiNBXlXpklMfADDCzc5yeWDb7I5preGU1mXwmYnWqo0YfUg BaL1iWWrh3ZVcKc6JdhD8Hzw8+o2kXHJl0sXpt6jH8z1tpQAtsoTh7hOW7OpYdJm6YgN IzHCM2sWhZMJb+NNkJADVx7YMl8LV9ci2YQdIeOP0YvZuUZ4GxtJC+ZVQIpC5VTRjHVV ZvFg== 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=SaXtaDTCkBMk0vkX0z3GD6AwMcAx7uFzlixhKIXp0h0coVy7AbmjwEM/TgO2cVXOFo eJx4BXpjUmfqlSOW2B4YhrH/FUAhJR4dFMQvwLQ5hv8bccYycifNMda4tyTY3SfSsSmy UHHXnGsPD6uDLtMd6M3uLxzi62wbylq+h+aSx4UsZ769bfm9+ccb075v0w5yINK16qYq dYNi3LBPVszt7tFZY2Si1gMNxaIt120CR7njgBwHGWkLU+6QrNVBML5yEONuTdcho2AC pn8Q+Ey1eSZQ+R7xDaNCDM3Qm4okq3kdIVlpdtmzF8pR/WmgQeNYl61Mpv01B73c1xNT xOXQ== X-Gm-Message-State: AFqh2koUTVSgybJRfni488RnwBKK/uNRFuna7YMhy+RFyLRKVhUFgPbo IrnHcAJXdv4sEQCj2cDTrqEdUsxinCk= X-Google-Smtp-Source: AMrXdXt1sctt6qie7d7vaUaQC58L6F3X7IsEnFH4siEAK1tdpVCaaG3XikKtUu/xpzL1W/0mbR2h+Q== X-Received: by 2002:a05:6a20:438a:b0:b8:6583:b654 with SMTP id i10-20020a056a20438a00b000b86583b654mr15858487pzl.28.1674186203484; Thu, 19 Jan 2023 19:43:23 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id k9-20020a6555c9000000b004a052e93b77sm21702171pgs.7.2023.01.19.19.43.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:23 -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 v3 02/12] bpf: Fix missing var_off check for ARG_PTR_TO_DYNPTR Date: Fri, 20 Jan 2023 09:13:04 +0530 Message-Id: <20230120034314.1921848-3-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-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/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg28l2cE3exe3tS2y0dU5xdJr1Vgd0s+vbl8nS4L rp+xG4SJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvAAKCRBM4MiGSL8Rylm1D/ 0TU0oD7Cj0kgfNhHEtdwVDQXqOJk6pyf/REiEhyrEngkm0V0Iz5DZjNpNKrCdHRmTvGoeuQ/30ArIv ZXYfYKyL735YuasVwbGqVGuNdxriNUgDO7O3ApgTSgOiYkjyPPI3fIFkLc1AA2gEdOrBLVcIuHcd1I pTVq/D5xp3MTBIyfLaQtKG+a/N9Sr2krXruLQqQdzptOrrBvGgJRmwlOhVig9nLhoUwEWEdC2Qe5A4 y45gunT2NtVcPsdOu1VHf0TuxtAbTWegVfdpLq8v86fKkLt2gf4J8Uh6YKaAoy/vCkEnmNw4VkzTlj pzsirQXW/rAajtihhfjcw+ZRUBYKYJPCqhxppDAGZmPSBFbxa72TTC9fJ9VUKFi+/US1eJYe5kPp22 j0L8UGXsfpSzYmlkOW6r+YFwYune5FgDPyhY80x2Ih6OfG50HE2ppt4ovXJL6tkxCZarAsXMRtvzgS OacLyk2SBuIYecCxbHO8q1BU2FM8HB7RQHSj5+6Aqb9ghN6L7yD5JHMu0fDj3ikAOi3uimqlwDGw12 Ii/AsOWH2Bpa5dDF/nawMaKtmW6AUnbMggy4q5yEuXnN7B4CkWO+zjgy11S4jiLno8q5YHpC3TZldr inAkl2NYGdfU7iBkg3JGDevUq4duZhMke+izI8tco1trdeGUSRL0quWAK9aw== 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 03:43:05 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: 13109090 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 0E7BBC677F1 for ; Fri, 20 Jan 2023 03:43:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229867AbjATDn3 (ORCPT ); Thu, 19 Jan 2023 22:43:29 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44548 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229595AbjATDn3 (ORCPT ); Thu, 19 Jan 2023 22:43:29 -0500 Received: from mail-pf1-x443.google.com (mail-pf1-x443.google.com [IPv6:2607:f8b0:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 93C70A317D for ; Thu, 19 Jan 2023 19:43:27 -0800 (PST) Received: by mail-pf1-x443.google.com with SMTP id i65so3096254pfc.0 for ; Thu, 19 Jan 2023 19:43:27 -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=AyrKQGwCsX+IeoKskXcod+gRVDLn8uWCSa+pmNBMHca2lVjXdvAW9C8lfUUYoih0Er ToWA0Vu5Joggm8lPY/T4IhLuKTeja53N/9ZYF145W8e5rxLbEyrqSHi3qi1+xw51nK7G 03l/3GjGyIRbV+/NsfZAJknvNg35YpHTzgCOdKVrsTh7rjoeYFXm5lYAUnKcuYEQzDyR W2YJ2X3kfzyOKqDZNFtm99yAfQY6Jd9Oge08iVeu6NbRff/PCAFKYqZj+2BiQKEVKZYK LgVGKuNJuUGzYl4z9jzIreDUXMMo9g09zVIg9l3joYDQeIQMdsuaFePERZW3xBpijuxs BN9w== 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=mRXjN6jXE3PZ898o/1vjR9jpXYFaRuW3U+ChUqINMVTiuSnoBpE0fgaPIFs6GCvUCR aikcRMm6Mv7+m4II7YpVAecpH+ZaOcrMb82lGNnSzkgZynT5SzfA47NQLFFEuVODmA1d +AoNBC+jujLehPZS6QxyPhgDwH3ne0VGxZrSFf+CsA5tYH0gblI17B7Pj9zIdJE5nFny DDO8Oesm+hOG+aGpl6FBHj1hsCBQ4WgpZp/Ix7Zbm599tyy1S37Kkz+kidT3S7e9lcGj zPMkJmPauyEg/dPpb3k5Z8DL/WlomzEl5zpKdGB4HzeCcX+CbxcaR3eSt6uF9e8DMKr8 0tIw== X-Gm-Message-State: AFqh2kosjKX6UwS/BTHn/0D6o9BgnfJ6mLzl1Eafhy54dTT+/Os0e0ny A+7e8zlhGQ06UXKu5SljH9LgTdufse4= X-Google-Smtp-Source: AMrXdXvjg2Q4D2TTn4K/mRIKNSCwEWeL6J50Ls6WWKBZZ8hIjm/P1fRkaljRYKpjcM21b4Gf32OMSQ== X-Received: by 2002:aa7:84c1:0:b0:58d:9b18:a36e with SMTP id x1-20020aa784c1000000b0058d9b18a36emr13437645pfn.31.1674186206818; Thu, 19 Jan 2023 19:43:26 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id l67-20020a622546000000b00581fddb7495sm11545437pfl.58.2023.01.19.19.43.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:26 -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 v3 03/12] bpf: Fix partial dynptr stack slot reads/writes Date: Fri, 20 Jan 2023 09:13:05 +0530 Message-Id: <20230120034314.1921848-4-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-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/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg28DrwrMkcLcpOoo/SrtdbpZSaCSGvVeLRH4RUd 8HAwUXWJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvAAKCRBM4MiGSL8RynbKEA CxeddEDgqxdJW/DHH13iLunO3dTHGjwzCv5aAtoIfmiak+VZaMAC4YT9HGqkZqNMSf7k0RRKpNEFco hMpZBE0mSQXQeYIqpyp3vWiogRcy4BXoC97gyYJNbzScZ6WNcAdYOytqu0RsNGy5P3Isnx6VxwJvXX YMtO2PCEkl8FNtBQUgINQgWPSIOKNAxmZksIXDvvn8NQ6Y2ome8dxb3jxrsOs1AW5ZqcodrI6eeML8 JkBPgLakCf7rhCdZ00DJHWGyBM1YCwphQBA0uLQzM0QEV1xBg/ibvEl4NJc8nTQoL7F6Hjt8pU4wQO IyH2s8+1sLBml9e4iCD1bN0axHVphvYvCG/jxivQ6H/s7tIHtyOnv3MRixzyYNkvHg8X6v2dLnDTyp N5gL7ywgjQkiGRMQWLcs27DbhrXRvTwUU0CSEkGlK072Hu1sIxEgxGqXc26seDfNFUHw+rYdvC43gY BuBkktq+Hf6c5c/DLCXp4s9IbAcWexT8Q0dC4qsPYT9/kCZ7nYfhjUJ+6F0zcoYuRdUbR/MRRJ07ro u+hT090Z/JUpZxeKK5XxbmnfxyOFQCkhRjXEVxklzinHHBhYgudpli17UPTzFyngbtE7fKhrm/Ed5p PEZCpupM89GLygV3mKdNtKfUyf6YSxSzSBKtU9utnqb7jR3rTS8Ej/qqi3eQ== 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 03:43:06 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: 13109091 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 6E243C46467 for ; Fri, 20 Jan 2023 03:43:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229883AbjATDnd (ORCPT ); Thu, 19 Jan 2023 22:43:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44590 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229545AbjATDnc (ORCPT ); Thu, 19 Jan 2023 22:43:32 -0500 Received: from mail-pj1-x1042.google.com (mail-pj1-x1042.google.com [IPv6:2607:f8b0:4864:20::1042]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B97B5A317D for ; Thu, 19 Jan 2023 19:43:30 -0800 (PST) Received: by mail-pj1-x1042.google.com with SMTP id x2-20020a17090a46c200b002295ca9855aso7875879pjg.2 for ; Thu, 19 Jan 2023 19:43:30 -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=mOqfZptm3xpSuYPArMgkAUtjuUg0XwEP5snA0Ls107ZKlhpzb/6j3C4hBc0L2+ICDJ dJQQMnG0Sm+giWl57q2VOZUO7YtsPN+g7ZOk2XAVCyC3q3sDQu5NOMlX7KMYR/UNFjsW 8ieR6jaeqpD41Aeqrb9pfHG8Xi2aw1HErO+yVOyQ3Xythq8LOPhkIz51kj1m1Jd5Eibb 8ObO5c7BDGPe0VTfxVW+JvdhMOTVbhgu6/0b6PNcWLTxgMaNUJH5AHn09WfMT4BYJZdc oSoD/tp3WTO+cpe0ayq7RiuU2dIfY6hVvqB24Ibp0MsksUnVoRmA5geqcuRz56Ss/DRx Xaeg== 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=tkJ9ihBlLQmvY7voqg+zxoBTde6yywzU+S/TtltyER6FDe5c5UNtV/064jl4pW7+iE ny9WrjZo11rzg0OJMwyKBFEyg5wSoOWQyUSPyWRMzABG0rvVcmcyO7+u0A4gtrPWLq9B Mz3q85tUX3AePGvR1OPCpFnxlXEznta+Ruh6reJURvw/4Oeyc5AqikhVDA+u3yaavDCB VdVcx0SgZgcCxaZAFTCFyopzCZEkD+CPhAgyygO14iB0Mw9W7eF5pqx/ProYjLXnBAjr Q8RSJlu9aZHfk4EoY1KcnX6qtUZCWp3c3tz8ze+gemJxrKn43ApVE4VTNC9liXqmLwIV DOvQ== X-Gm-Message-State: AFqh2ko8MJeo7IOCa3NT/KkSawYGJwAW3yK6I8daZScth0UBrIrS9ltJ wUM7Mg4fkoZEEhlYMCCoX2Z0VlmHwOg= X-Google-Smtp-Source: AMrXdXt5qU2RlVVRg++P/nLabdfs4H+Ac3aLptUXyCgFUMJnMB62ZZr/UqDn6YCm9NeckRUqei2Zhw== X-Received: by 2002:a17:902:ea0a:b0:191:24a:63e3 with SMTP id s10-20020a170902ea0a00b00191024a63e3mr17397732plg.50.1674186209898; Thu, 19 Jan 2023 19:43:29 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id n3-20020a1709026a8300b0019492b949fbsm8674207plk.272.2023.01.19.19.43.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:29 -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 v3 04/12] bpf: Invalidate slices on destruction of dynptrs on stack Date: Fri, 20 Jan 2023 09:13:06 +0530 Message-Id: <20230120034314.1921848-5-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-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/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg2873eOalfR0quAX8XXX6QYGDZfPEkqepJIzmC5 8h5tvXqJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvAAKCRBM4MiGSL8Ryh/LD/ 0d2iiLWFEwJkG8yX8u9a+Llsqc1gYeK1LxpJgFhWuOKk3gfikR5KNa3M1u+Bk988obr4h/bA6oAZ0d Oc3EBkm5lJJycjeoYO1YmOy5oFq9SENY5o4QQfBLSGNfo3VqolBaKe+LTDkUclv3OYmoEYtuxeq+Bh zKkkkfwvYflApSVi8j9+dpEhm9vHr6cE+jft2bGuuo7jXc54hUGv71FQhmfzROOFmkdBPSkQ/A0IQk yZbF2ZiwnbZaaQPlDxXHnx3WM7ESzSGNNvX7WdJ7DMBheEBDPpkONEqarVJzJ8gtg3cMNuxsePTT7F gzxfiEDvCba5DPoSO4+JvfO9MHUfDKfrsHpAdugdvcSsW8GLrDL8zAMu9gch41YXhxnmNvS5WvLvgB FMqghp95/ozGePnpHrDNrXdTtCPDGA/q6MEXosiUYWliuis0NqU468qmybuWWVuWqyWuE7RLuc9psg PHjLwSuTqnpfeeNEGHkKPCeYf0W0hT7fAtpAMXw++ZPX8IdQRpgVywFHXGCrk4ZdnEvh3ZGEG9/MVG s3q/oDMU4pfXRRvlC+ynZTYExoxpXIlQ7VdMSsRyRs8qm4Cyc6IfkMYf3EfUjZIv++/LyRIWXxMkoB Q9ULxPE5nri1co6ku9hZ759D8/0uLUgSvkhhJLx5hxgP3iPRK4YWvmaHqdIQ== 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 --- 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 03:43:07 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: 13109092 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 A73E8C38159 for ; Fri, 20 Jan 2023 03:43:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229595AbjATDng (ORCPT ); Thu, 19 Jan 2023 22:43:36 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44630 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229882AbjATDnf (ORCPT ); Thu, 19 Jan 2023 22:43:35 -0500 Received: from mail-pj1-x1043.google.com (mail-pj1-x1043.google.com [IPv6:2607:f8b0:4864:20::1043]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E513BB1ECB for ; Thu, 19 Jan 2023 19:43:33 -0800 (PST) Received: by mail-pj1-x1043.google.com with SMTP id n20-20020a17090aab9400b00229ca6a4636so7000182pjq.0 for ; Thu, 19 Jan 2023 19:43:33 -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=dfMkB521OcQa1f3nrATddiAd+PcvT71EbJLKrimDD35olpG0v+iPcmIT1FPqACzbB/ 5DMSAbv6rOWMtLqkce2S1iihRCL9dtc1eg+HOM3HVqwSci4fhfu6vGlnKyNaOly8HlfD vnow+D0KV4O/Vp41EnQcgymgYxlgUJrkslaVgg7Sq6t9USUpGrlp7lgPpqGtIIzE/7Ba FSQqDKz162nE5QSfka9Qnv9+bLMI6PxW9kt66eP8cOCe3V7weVdwUMl6418BiS/hf3Ez AoGqAnZZFry2QGuggJLpmFDSGeNC4/PhQHQDcsRyPWY/vLVxt6G3a88mEoLYF+y9zLAI oJDg== 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=PUIBiiYbWXPIZLzV2RT1uE6ZupiX9fq60MivTdpyF5ulkB/T6lWKovIhERlzQhb7yI MKApIiDewRCmdiHv/uTlUSV2u50tOkxZeqGzMPFBJ+MguD9lG9ca56L7TQoyBo4Xn2y0 WkOVk1KqCW4h4yeG1yRJlrGSHUl2vd3RkXme9/R/oMvUMu4+CdvxIYmqTJURKiCsBQCR MpuBMbchEqngi4Sw9i2uL0bKG+vuC4u00EbsDHN7N8AofrHhYFeafRa6ci1kQAXrMjHG /BlNoyHoOYpXKz0DngrykYQo3YCKEkCkmUdG49uDThtZYbpHMeSjYFjg4ZuK4l4/EE58 moQA== X-Gm-Message-State: AFqh2kq9+jqy0yhiUsEmgVN6T+3YwSrIFDIU9qsrKSPrHewHr15Hcnlf jiLE54rT9MwM2tv8ssG4XonVESa3FDc= X-Google-Smtp-Source: AMrXdXsMpJt/Jtx9Zdx0e8NGa7hpktpJvKBt17UtFJKAxvRKZh4brTmoxy1jV1WpTCLKpgtTxtvMZg== X-Received: by 2002:a05:6a20:43a0:b0:b8:f026:754 with SMTP id i32-20020a056a2043a000b000b8f0260754mr10898333pzl.54.1674186213206; Thu, 19 Jan 2023 19:43:33 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id y128-20020a623286000000b00576ebde78ffsm24668001pfy.192.2023.01.19.19.43.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:32 -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 v3 05/12] bpf: Allow reinitializing unreferenced dynptr stack slots Date: Fri, 20 Jan 2023 09:13:07 +0530 Message-Id: <20230120034314.1921848-6-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-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/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg28Jqykp549TuD8qfi+l8J7mju9PTDOmDmMoZB7 eluhbseJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvAAKCRBM4MiGSL8RymlTD/ 45xWAJE/Wk0rNdGLz2HSx9OB+y50DL8In0OtbCod3+NSt1ABKEtbfzDitfraUNIQ2JUGj02nagnNu6 t0n7lgK6Y+X6mJkmvfiP0NTKe+jR9y8nk1YQOfKFpQoeb7fn0+MYowc4jgTO1bWSCf5YkaIWwxMHJu 6f2OAPArIWrv0wmNZey14f60zxeTuPqrqnBef4+Z1KTcKy7t+f3PJ6VPndpFnBx6R85ljX+2hN7csC /9aOnzcUr+V4m5gmwyRF7gCtg82VmJpTgx/iNK981T42W0Q05S30GiZeslwQguwoTAtIjZwSvYRTrd +cgpGcSWO8N44msU/u2lg9arKxoeUJohvuG3/Us6vROL0OCNYfYuGjpN+Ekj1rCNBKwh3fZv+zuZMl cmuX28DMQszu3rqYcffb+lwFNav9S8xwvckLSLZkZ+cY0Lm+WGKPOhEsi3m6nSea1EL2vgOC5vKNyV hm86++VAJAg5ZzUPzePfZetEyeDnEi7u03PWkFMzjAMtjakFEZSKzoPkFnXabhEmX5rqizaIKMTDB6 vwrvIlVYMWeA1N4LQ0x3Ejh41HFYDKVEx/awGd9rBSG4pSBlGe0OvHJ4remKHvHJw7dKSAafWoHafD 3gqtXiVRf7gxeJi3l6+ZnGeLjVoht8xTh+8HTwZWSpBg3NX3h2XPG3/ECa+w== 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 03:43:08 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: 13109093 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 D2A04C38159 for ; Fri, 20 Jan 2023 03:43:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229545AbjATDnj (ORCPT ); Thu, 19 Jan 2023 22:43:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44672 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229885AbjATDni (ORCPT ); Thu, 19 Jan 2023 22:43:38 -0500 Received: from mail-pj1-x1042.google.com (mail-pj1-x1042.google.com [IPv6:2607:f8b0:4864:20::1042]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 62C49B1ECB for ; Thu, 19 Jan 2023 19:43:37 -0800 (PST) Received: by mail-pj1-x1042.google.com with SMTP id t12-20020a17090aae0c00b00229f4cff534so467066pjq.1 for ; Thu, 19 Jan 2023 19:43:37 -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=JY6L7tawJZ8OMcye/IcKmpTSG5/bQDnH4/oklrNqb6W/YzQlLQvXAaSBM6A+aRNpzD vWEGFqFucw5zAMVrcs7srfOnyzrnkqKiDr3ojWYpi2/c6rBIND4mL0ml62Q7SIqVZAwg OMV4AltsZjXW9LHVLXvQ3rfeHhJSAlNsgKhvkPYtiluwQHioEoR/eeQ7S/DTk4WeVnRu RFc6JWwad2XH+9HjuweGYnrCct60FNgLEx3Xy/DSua9i435Bv4yYSYfmLJbUkm+EiMMy 98sluY1AFs89ocm+2bM4Z5AF3M2gnBojRcgwhKEhtjzkBXjYjVbRx5lGvTgJm2dH0UcW RnxQ== 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=mm7vkmZP2YEN/tdq9M8yFTGCe9c7rK4lCWV1GbgYTFGGLQ+wZOSY4lAJ2pLqFb+O4r NBR16p0nCdALIFmGstYRMHHPdiauhLNKK3C9RZj4o1XMt0rh7yYPFxm4zc9GXVkzSJZ2 EjyDrpfPFdDwmILSB1E4Pwz5/y+0UA5PixU8BnqYvY4vF3XNwSKUXLvq8AtE7bBYEzWi N7QozUvLlhe0SAHBKzFP7lml2sc0LnK7vb0Ia5Kk0FKsHA0zqSgn6PZjt/M/oY43H/U4 omlnSsAm4XycoAlvD7ph//7YVj8etA7XfGW4rL0OLoMu1wKC6+dpF0Eh5eE2c8WIPuM3 PPrg== X-Gm-Message-State: AFqh2krGhj8oH/+T/BDsK6otiDXyCIzGlfuGnyD92rPEVjvSL2J0P/C3 KXgF5HGX37S68KyrRNb7FOMbg2B65dQ= X-Google-Smtp-Source: AMrXdXskqZc1Hc6Jr3b0pmYJGLkogK4nE7pfNM8yhbYAFLwtNtuRwkTSNnNvH76ETq28PvCPspRFTw== X-Received: by 2002:a05:6a20:3558:b0:b8:8d2a:d37d with SMTP id f24-20020a056a20355800b000b88d2ad37dmr13333453pze.10.1674186216641; Thu, 19 Jan 2023 19:43:36 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id d12-20020aa797ac000000b0058da3f2eba8sm8716235pfq.40.2023.01.19.19.43.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:36 -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 v3 06/12] bpf: Combine dynptr_get_spi and is_spi_bounds_valid Date: Fri, 20 Jan 2023 09:13:08 +0530 Message-Id: <20230120034314.1921848-7-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-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/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg28M4sVtlsxGoBslFWC6lc7p/kxompbmt4+GNFv n46VPXCJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvAAKCRBM4MiGSL8RykH/D/ 4tgG1TmwJvmGv4tAdrlrR2mQIRaUIrxeP1I6tMTM0JuC8jrOJTYUT7iUK4LCXDDEgf+FdOCzeNhCE6 P3s4OiEHpdbnFhEWVZfQ8zadm71upEkXzebTDPKRHd9rqQcOIJfNljNtds6j0FUNMfBNobgJDPJ1bO 7SjrGSmXFbG+jI8dmskcm0pi9NEfhIydm5KUpY+TiZMClyKCMINLc8gmYAq1FFLKqx24rZ9+F0tO8T /tUj7qPkbltViUXLTTqhz7cKu63Z+NsRH0xG8GACVMcHde8q2DpBp0EpduEJmnFoyxXk0DLD5nLoVT 6KKGgJmt5pS62d//A3CaVRXgYp5Ixw0QtPWr8fSz1RcpAyE3Vb5+BBexoidYTpXQK0DyA2TixUZ/6W NwKvLi1mjWkcyu9M+EmzUiDiX5hPGRhTHI1HxVK3XJq59aq6HUTW9JVh0LvVBF3Q9kObPALjqF4lQd YLMzZnWxlxfwcrhktkiNR94BT2mpO7uuq6xzF5phTdnRrZqm0OM1u4GHdDUgEUicMMlgJrHWVLqO1X kLxK+cce99N03X6Cjzpol5CpYyjx6KTeTvsotx6fj6Tw8ASSfumOOE7Ds2P6GcEo+VMAu0cBtpjIVC DbRYQ0H5J1KXyGsrBoEZ9CJ/ROe3h1PpHT2iFzrwogXOURr7lii5Jm4G3YQQ== 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 03:43:09 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: 13109094 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 87547C38159 for ; Fri, 20 Jan 2023 03:43:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229882AbjATDnm (ORCPT ); Thu, 19 Jan 2023 22:43:42 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44720 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229885AbjATDnl (ORCPT ); Thu, 19 Jan 2023 22:43:41 -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 B392BA2955 for ; Thu, 19 Jan 2023 19:43:40 -0800 (PST) Received: by mail-pg1-x544.google.com with SMTP id e10so3170370pgc.9 for ; Thu, 19 Jan 2023 19:43:40 -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=PkspaGLT1TLPJqEito8K0eXi00ctzrwHIrx1Pt9zWwdOacC0iN4H3GwSUicXWPTRrS 5Sq81bWTWs+Zkz7+6z2Z3NJEWX0z2MR9lkqUN6TxZcA+mFy1MtSUlrWvOj/Uc7hYNoi8 aspp241kNnLZWNiOCQg60vJDMdfmeXZkJY/XTHv/glZA1pJDKSvmyfRejrXphPUWIhKV 9hBXvin0T7T8xxtyh47U/RNn/xnmufjgyjSvWPCqLtgzr2qTKHL4RI/N4BS8GF2TLK8S 80rl0HolnzqJ4QM4e992AwrTrOIjVQtqn79t8hcc0/pfSSz2jdgf+4WS3qmAXP9FDa5l ARqQ== 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=Ab66wjxO/SLRXxfquESGoCfJk1s3fBr22ap2OegbWCEWTDW9hcGfZshZRJL/kNejdq VKYFPhRdKkZUu89xUDtKm2uOGRxROWg8ia8wgCdxVVno1y7fQI9oaNd0iIaMTMWV9f+0 axAnI6W850eqoxpBmbxXJvzfRRGmV0s0Sz4rqvZfZwknIxkhPoWZ22O+g3w3QKujmzCc Ps1qVRsZXK+8uNk2Pta/rARwMwWVqijx4Ak11Cox8BdR92o1ACHNU+D/9k/Gtz9uR7jC yQjUKsnJXgMy4ygfE66veMf+OcBuPMILNEddQWjvQYvseZG96JO+pVGM8hsHYlHJMC5c cJKg== X-Gm-Message-State: AFqh2krFUvqW9QojxQUd4+Wtq7dtvKmXnXpNet07TTxieuGA7u0om4To QVERyNrEfcORa+Y/qgRTwvXTO4bs1FU= X-Google-Smtp-Source: AMrXdXvxAJcBq86/kouxO1tFEr0/gZ7MhcLckXoHOUWs9tS9ICjAyS8bQSrkcfXgK/YsZuzXYDqTUQ== X-Received: by 2002:aa7:946b:0:b0:58b:c873:54e1 with SMTP id t11-20020aa7946b000000b0058bc87354e1mr17197248pfq.24.1674186219894; Thu, 19 Jan 2023 19:43:39 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id q17-20020aa79611000000b00575b6d7c458sm13080373pfg.21.2023.01.19.19.43.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:39 -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 v3 07/12] bpf: Avoid recomputing spi in process_dynptr_func Date: Fri, 20 Jan 2023 09:13:09 +0530 Message-Id: <20230120034314.1921848-8-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-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/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg291RvlX6RfReOLAFbMWC9wK/gvWsxIfRy4IvoV dOq3G5OJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvQAKCRBM4MiGSL8RyotlEA CuomRTTJmydbDJ4a0g85hfdH+bbBnk3if2iuJ/yJnoh4p1nvMM7dR3No13UDo87TIgItMOFq8lp+Hs wQWIwl9HMvT5zyeV/px9ezUbuTyHcRIXHFqeI/l6u/zx+cxdUrPaRz5t3vEJn9qRHCXdez5zpTYdrR EooWEr2nM1+4uUHhlZ9ZNSmVPcNx6wVSCak/usM6Sn0b7O9IOHL0oq2U/DJeLmiac6a0dmwsLdWPcu pyRdfNfQBwGEKnGGZfxiHyejIGrvdaVSWtcvS3NSVfmCzVwK4BBaU9ILsm29KvcOqY2g9fTIf72FE7 4I/dH4T/dq8Nh3i7v2kBQqdj67SnLCUv3r3h7G1tDfroSi4X9u3u3G9ZMcAuiUiD9W4DaBaq0eCgiU hI1R+xlEnQSBu6jY+np1GF+uxBXz6suDHca6jD5oeUVJczRsLgc6rbalguLC2sSQBfNY7lzylGnLPN +s47hlkUmssOTy31MnOyo1Z/jMHYB0vQdmK10Fzp0Fdcdvsd0QDL97XKUrZOEUNHCX056sDZ5p4rHD AYxz9mwPhfU00EdfKYM3LsT1GlMp96R07/3IwQ3ivS9HOhFKatbpQlgm3fI6LknZvOhiU3DKnWpdPs l2o3vWagu7TWO708yIOHJP/tkQzaB76H+JFFaM5zv0Vid9Vqivk6NqsqXN5A== 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 03:43:10 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: 13109095 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 3C92DC46467 for ; Fri, 20 Jan 2023 03:43:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229534AbjATDnp (ORCPT ); Thu, 19 Jan 2023 22:43:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44768 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229885AbjATDno (ORCPT ); Thu, 19 Jan 2023 22:43:44 -0500 Received: from mail-pj1-x1043.google.com (mail-pj1-x1043.google.com [IPv6:2607:f8b0:4864:20::1043]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id ADDB4B1EED for ; Thu, 19 Jan 2023 19:43:43 -0800 (PST) Received: by mail-pj1-x1043.google.com with SMTP id n20-20020a17090aab9400b00229ca6a4636so7000492pjq.0 for ; Thu, 19 Jan 2023 19:43: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=Zs5n2Q1YRbL+FZCik99wIL5O/1FCJ2BZ9GlI4NG+zGU=; b=P34x8GqHVh8pFfUlaWPHhimoMq6YwoPxSuiGzssVnWeTvsM34NsS/J3OM8SnajTXcx N8TX2FbcSJ1XJRAUvaGLwedoBMsDLsWWC04ISJWQfqkIdzpK+lToHRTJbNOwzJ+nXD1a 6mOHJCHmyg6DjV+dghhZ96l1c/MCY115XW0aUXkxfdFDQKXcU9pbr5r1re43gSHPZj+t YWDyjxbaT2gtEey8O9MLZGsXVnSUzZoysrdjJx9XpcyjLCYi+9tTjSiIpXPg6bR/40NO 8LBwYgknbqs4OOiTAy1o7CubkJXNAuCvjniDYZEnS5zUMYwHqtRreCpgkV2w8uZN8zzl WnyQ== 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=tdXO6SpZuNBJ1bz/oYfPwsPHjgpfhc6gPnVHIDsAEHz7+d4tGD4XbXbcomzZA++HGy 16XUf+Yz/9D2U3UTgeHpGQS9ojpsAS2DG0YZBB6nv/yT+lC5TEgluqg2+h5WaOQk/KRe B0RioG1gkhxN0S4YbB8v0IFct0uJjdCNcL3VCwoXgLUcubCIHm00lHA6b4XF+SaDgoh0 eJAjoHLjn9LrTK/nmDkte16hHHKEwRqAZwgfa6Q+Qx1a2eQ7uMsrgQ4xd+sVeaof9zUw WGYAsz6YZThJ7pv/GztpeVnL89kCN9ticcC58mz9OMjSArFIK1hSHTh6r/fijvcibzTE ti1A== X-Gm-Message-State: AFqh2kquJ1ah8t95m/CoAkUkDFbMVZfI3LGWKlsaj1VHB9yhYnCbhZI1 W/do7eZ/in1dV5UH0f8mdX22gN7rtVc= X-Google-Smtp-Source: AMrXdXu8aw2PzXm4PvDz01i3ooGcOtIawpRtw3fCPvErZzlhxgmfIFHaDOO6h3L0sgNQQSVjd7PMsg== X-Received: by 2002:a17:90a:ac0c:b0:227:1a22:d182 with SMTP id o12-20020a17090aac0c00b002271a22d182mr14393111pjq.42.1674186223108; Thu, 19 Jan 2023 19:43:43 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id p9-20020a17090a284900b0022b787fb08dsm373551pjf.5.2023.01.19.19.43.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:42 -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 v3 08/12] selftests/bpf: convenience macro for use with 'asm volatile' blocks Date: Fri, 20 Jan 2023 09:13:10 +0530 Message-Id: <20230120034314.1921848-9-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-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/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg29s5JRMTd9QT6uBQgwWA2rgY8rE3jweRYNnMEY kA+RJ3KJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvQAKCRBM4MiGSL8Ryo/cEA CSMbgc3oh000fQxhLDye890b83pM8LM2nZaFIjii5c26zP+HOSw5iX4mW2mZbL7zH/nnO3ioy0pZde 1cfxfr3VN898fcFWN6or8VgnHOK863VzDohossjXJhc9cCx8vs4BfLXfOsQa+xxTqAXU9D+fFwHzkw Ke5rbyKkeVKK3NsxBhMfehDDLa8LdJmKI7VItQliUm6qhj8G9Zs3AYg3cOljO4DrFh87ksxck4XRIa ZrOJWDBVTDf10yIrBE7+XwjG+9XqQMl+vjWN9I0uafPnPmRvfWficPIcKLjhRVvcYPvw2mPpMJgjqg 4Ix5cRVOaqZiXw/nlvMv2JkSEiInD/8uxPbK2pFfjn4IuKR2X6T4RxByo0yHcCn7WFOHsoom4TOfGp YlcBE1kl4PeG2JZxHvmSOJ9cXPWHFlvbgYoDexR4/TwNHGZyApEMiOIZ3U0OcIUChdC6JTrqoNoKlS 72P62Re8YBo6tV5lW81BNpNXVhw2Mb0tC/gPoxOXiDCfi0KyWhlcBakf9DHo8slqQrDNoGnhC7gaSe 8sbnjM1DZr1VG8fTHMDvzMcXt9jIiOd9f1F71GT+vCD2IOoNxdF1O/qKA6RtQByhqOYCFmpsqQtLKJ lymmtuME6RFa4ywsOSnteu6ZVkrzEw+/jcUF3g753eTC3FsUbP84PAUVUA9A== 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 03:43:11 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: 13109096 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 7414BC46467 for ; Fri, 20 Jan 2023 03:43:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229912AbjATDnt (ORCPT ); Thu, 19 Jan 2023 22:43:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44828 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229890AbjATDns (ORCPT ); Thu, 19 Jan 2023 22:43:48 -0500 Received: from mail-pg1-x541.google.com (mail-pg1-x541.google.com [IPv6:2607:f8b0:4864:20::541]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 64518B1EE5 for ; Thu, 19 Jan 2023 19:43:47 -0800 (PST) Received: by mail-pg1-x541.google.com with SMTP id b6so3176854pgi.7 for ; Thu, 19 Jan 2023 19:43:47 -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=qAhJW0jKH/4jx6iQxNZeszIBjJ7usu3mlK3zqEkOoME=; b=izWWC0EX8elf1xqHHjX/rMwZ48HNzhUrL+tksIqqBcEUn1lMXRUWkDsYG/Vh4Wu4yF uCYUUm4EUGbUzfGBSffxNAwJuVbkYd62X22aoUJwghYRz9SRsR4EBhyRoZKcetRUvCLv /RY41JegjGWg9dQrX1kScevkPFP15x9IgEpxmYaV+IHognDHIjKqWB+QJdyfD9RWRt1d QINahIsYZ3Lcomvt1DZHisKUHGZ0RdzuIkSTnrKbW1XToUOb3+MQjoQ9SCt3YKibH68c znnxOkQL4hLmeXFBGw8YGUSewRelwfEKPDzFuLVcSdNnpe+1tpkQ8lLvdCCLwgpUC402 zyKg== 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=qAhJW0jKH/4jx6iQxNZeszIBjJ7usu3mlK3zqEkOoME=; b=p+fzdszX4yLnzDW0nwUvorqaXja+4kFaroLlEhprCDL8relgaO5XLC3iJsVdt5yl4c sizvYPcVVZjTacqJYif9RTPXyJQ3MlqdKe0jmW2aeddw7xdR9o5wfbCP33+w71UeULS/ Er4rkpgzuRPSYZVmVRwi1o2CzvhbBTgHLh3mlTxtQs6TLOhkquq7tydXC74WovitOIJ8 bTykEAIwlcX/PKsDm4YwLaWXP+YezzV8qufjqnAtrQDgMUQJpgaSd50ZrM2lFxIhkO5j oONPqCHsiWMv7Bzmn2azjiMEfyH6gRR4PudKNKquB5HAFyS+2uPS/cQS7lAoi2tdow5d /gQQ== X-Gm-Message-State: AFqh2krwvXq9ru2rfNGn93R7p4VDhJGI826FHm9rmpDj0PihPWDBc2lU muJfKvU/7HarOOTSc9ZCEtAEkbHG//A= X-Google-Smtp-Source: AMrXdXua4dz87qOsTkpGZEplHVRhE2Kf0rI6/geAp6i29yohOIGzEEiIwpi8GghmJfVnzYO0GYjdog== X-Received: by 2002:aa7:8006:0:b0:58b:cacd:2d12 with SMTP id j6-20020aa78006000000b0058bcacd2d12mr12972768pfi.28.1674186226683; Thu, 19 Jan 2023 19:43:46 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id x21-20020aa79ad5000000b00580c8a15d13sm16442303pfp.11.2023.01.19.19.43.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:46 -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 v3 09/12] selftests/bpf: Add dynptr pruning tests Date: Fri, 20 Jan 2023 09:13:11 +0530 Message-Id: <20230120034314.1921848-10-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3977; i=memxor@gmail.com; h=from:subject; bh=8XInI5egSU1iq6glPmT9oxgPqOpmV7u1yWWICXdA/Zs=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg29r80p+sB+pDh7sMlZEJcoWFpJecuJjto4oRfH 6JcsCNOJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvQAKCRBM4MiGSL8RykRmD/ 9xfDoll38+IV+5IkkRgYkh7vuPW2CvuHIFg5u3MahBwRgbwCdbMxUARxIprZCDF8cXZLiCfMvqsriU Wd6Hnx0Hvg/zkLbIoLfShDOITx9T9gD8ddhjvfm3acV85fEyWBJhYjsweVUGKSGbruQ5tCDy7Prcku khbzao556T5FP4CGbPZr1HTW1dLhHdkyqhnaMyycVZEbizv/oIdosLuG7jAqDNPnLHWd2HYVtQBDd2 1JUOOlLf03W/p+BrnVoZss8ZZc5TA9SQQ18sCFzrU9/fSZZAbeW2X2yU/m2QlHzMrmLTJgxOy+/4PZ HUK3rWsA1WWjhNk2NXLW9BIh5ioFbKrGCAGJVYFOPo7uVSSPHCR3QCrBsqsv5D2+dluwGnCl7KQoKD 5uOQIAhnuTxX4ETaUlbuWV/wawJ9NtQyMZfbP8AvWuaSvPuZBbZnqxtjHuNXy4kKNgtvXmVxqlC5Kc 07K2klcGAaAsybHRl2zgEFShJjYayB+w9dc57jGUKcN4uf6kA1g4C77FXN5Nj+OqUKJHuJMZNWl4FE POgiRdsay+uDyLHZ7S2OsSWyag5sTBKnaf/gonWPNU5AqAHHt2w1ZCKaQWoZsVaGulEHgyRfJMrMuC iy7qYMaKvZDtfdBykimZgxeaRhy/x4BvywRbkhH85TMva7Oy5jwjys3bRI0A== 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..8f7b239b8503 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 03:43:12 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: 13109097 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 0A81BC38159 for ; Fri, 20 Jan 2023 03:43:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229885AbjATDnw (ORCPT ); Thu, 19 Jan 2023 22:43:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44856 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229698AbjATDnv (ORCPT ); Thu, 19 Jan 2023 22:43:51 -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 8198AB1EE1 for ; Thu, 19 Jan 2023 19:43:50 -0800 (PST) Received: by mail-pl1-x643.google.com with SMTP id z13so4236489plg.6 for ; Thu, 19 Jan 2023 19:43:50 -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=sytj1Iu5muU2N5JZ8RxnQi4JbWwNtRBWbJ3V6flnO1k=; b=hu0+4C1MybmHcHcSqC7OfbLBaOACi2t7yX2iEUq0yRUqaqkBNjUuWzyyS7zcG0Lwj0 0i5JMExdHsxbf26hat8ty09aBUAAi6Tw5WNXc2TBydF8hbL0BtsHQAMsmRu6TOJ+k6IW ogxiplWTiRZLeDgJFi2yvbGYcNjQ90q9vVqt++BPB/hSd7JEIw/PvsGjS1jxr/a29cEF 3KGrTUcahUMNhXNGjgidHB1R2CVwja84qV1HWwuER4oWFWe6+1TOUl3bUO+cvzIeDkSC fQbRirzL7XMh6xH1zxWd+IbHJSZXJ6OlhgzYrGgd3zFzfolEndsa9+xjs3WoUUM9rfgh RKtQ== 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=sytj1Iu5muU2N5JZ8RxnQi4JbWwNtRBWbJ3V6flnO1k=; b=06wu1uteA3CGndLmwY4aBhiKdsrUkkpU4gaogYuiYu9Lfsij8Jvr6onDotDoODRk2e c3C6/kh9IIv4inhLjJadm5yGgNmKp3UZOidtDw4EfLx4vHlssFx+m+BytMAbZZ+eLKAJ 7rPPVa6KVInJavMDZPXTfcH8LD+AHv081OfABkNpuj6/a3X3KeTmytgBpKYD3CZWoann py7zYkiDyEdsxoykaARSej9sK4zbTOMtaDMKs+vd1s9IbcddEDVsiWJaLm8orE338sZ/ l7sbC8Sb19cxc+qbRUM0eXR9wM82jD8vYUDdYIEuOrcO+fC1yYqDb+MDWjVrexDvLwKg TmhA== X-Gm-Message-State: AFqh2krnbkqcUccKZLP28hOwfkn5UIMnZn9XH9+Dqa3rRW9nriouSntF rbkUCyIRDouLCRKHGyLLXOo7MZSCXuE= X-Google-Smtp-Source: AMrXdXvnfGTmGEZDDXDRn0JrGCK7EAXlybssxdCqXuFtE7i2C9tbyVg+9WT6QP3pCnE2OorXiVuymQ== X-Received: by 2002:a17:902:c3c5:b0:194:41aa:2131 with SMTP id j5-20020a170902c3c500b0019441aa2131mr33138919plj.25.1674186229798; Thu, 19 Jan 2023 19:43:49 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id j15-20020a170903028f00b00190c6518e30sm25906039plr.243.2023.01.19.19.43.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:49 -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 v3 10/12] selftests/bpf: Add dynptr var_off tests Date: Fri, 20 Jan 2023 09:13:12 +0530 Message-Id: <20230120034314.1921848-11-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1570; i=memxor@gmail.com; h=from:subject; bh=ew9dT3h+i10P/6ZuvC/eNJnDhWbTKKMoZzJ3kbQWv5c=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg29fUSlp1ePbkU97nWvsvmf7MN9MKVsrwPP8lRU jhkjSJ6JAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvQAKCRBM4MiGSL8RygUkD/ 9WEwr667b6kGH+Tn6Vy5+2pCqO16A82aGWgScdSGuVmWPVYAX1YnwtfWefKH5i93OJB9MUM18MILpd SWu7WAf799FcbXHg0yLT4f6JXtAILUGD/sx1BqmU95oINtrDChIDqORDHl0FLT6RiOkC42ay2SvFz9 4ENb9QvFhr4fHx1PXcEWP7C/0mtSe7fodyW9TVpOnHmn7ow6sMCfIjOE4+gToJseLrd0vlUCDZfeWc qDDyrB33ZwaGorfjPa7w5lQOZENKRV+ApYyPRj762MbqU+gkwwok1LrCJd+w+qwfx2HBJ44evRoeHg j9vLdyRWui34vrb+A/h78A/be3CIsPCHX4pED6MiasF+tCYQPppTbMlf9cMnX9ZZEvUBA2b/Who4kQ qrxKVD9WZyTboOkxT390xAvf/v9F9sfd9IKUULSCYmTuv1qoL+l+D7wcW1+UzUkb6DjB+AZ+TRE5fo n4nolyE/QkvbkH47dFuI51H5cRaNv6daHmdGl3KwG5lJRdLNXF1Gx3I3Euz19t7vpXyY821y88XeW5 6wLy8Qnv192mKoHFmCg6tJFJRlAjV15PBA/mUJ9GScFOArKbFUqsjRL5oqudP5AN0hNkFth0cpkakZ l9ieEaoS4T3PKWhuqONVctUPeuP5Tan05KAdk/FaAHY+l3xXaK37J8Xo3zeg== 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 8f7b239b8503..9477c238af57 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 03:43:13 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: 13109098 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 66479C46467 for ; Fri, 20 Jan 2023 03:44:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229924AbjATDoA (ORCPT ); Thu, 19 Jan 2023 22:44:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44906 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229890AbjATDny (ORCPT ); Thu, 19 Jan 2023 22:43:54 -0500 Received: from mail-pj1-x1042.google.com (mail-pj1-x1042.google.com [IPv6:2607:f8b0:4864:20::1042]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F03BFAEDB7 for ; Thu, 19 Jan 2023 19:43:53 -0800 (PST) Received: by mail-pj1-x1042.google.com with SMTP id u1-20020a17090a450100b0022936a63a21so7846890pjg.4 for ; Thu, 19 Jan 2023 19:43:53 -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=HoWooIhorualD/dUDPJYQZFrDqrpIAeU3u/bnIZ1h4E=; b=jIclV2b9DiWkC/iLscNR0YQWz5E+BqcGMhRxDfAAEM0/OuxwAI/qv2j44xg0P/Va5Q 7qDwp+SYHsoKQGl/PMIrRGoSPyEkzkJVYOJdzMIysScFGdUF3wKhfK7p2HQIyssqXGBm 17JpUOR0Zk0XIThX55LnCJHohjCvuJKsmTdJY7t2gQ0Jk/a7XSaeP1emeHzEj9FHZL0j OizgSrCXlok9nVz9kLF3L7Vd1a2TFUj6lLZ3+/Sqv6hxvLvpE5at/qOCMAlKhmGNy7X6 sjd7OHcb5a+R2JqgtUOGkB6jUnxobe1vtxD4y4JGZ/4tzQlOs7PpB8OyzxDoSNyasF5G fz1w== 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=HoWooIhorualD/dUDPJYQZFrDqrpIAeU3u/bnIZ1h4E=; b=MlpvGWhSoxzltU6+dii0xV9taZmY7jkj4T02e2hbxSgTBTBh8L4WGMSt3tzXShfjUD zlcmJRAtIwhmEQFqcZsOuV8HUKkHXEpRUpU5oe5b31r5Cs3VnrZ+LdXjWZRQktJA+5Rs XXcTSL+NEdTAjQcNKBnW6KP7vTjses9A2rYL4cK1BwgP8rnbjo6IaL357g40FxapxRun +9JZ/2teQrCyfaSj5mTsrRhMJbt5bVj3YS4tvOuHX11WarPhKQzq/8Hu+IlHUbBxiUGt DgmrU49CAUFtOtY64gGW0d40k6009ilvIycu3vO428s+mAMZs8fswRU1YnCQZUWBJK+6 h+Kg== X-Gm-Message-State: AFqh2kouL+JjwFNqcvr+sQLSxeRJQID7L4iaCIbYAbNMloYhXk+jXoCs 4RimHRXiE1P0PWp1WSGU4KZUitDkqd0= X-Google-Smtp-Source: AMrXdXvVwKm0ohHiCSWqdzf/XAXWN+67le4u8uX0/graEtJvGGWIroi137TFFaqUrpZqnCu2COW/EA== X-Received: by 2002:a05:6a20:d38f:b0:b8:723f:e21b with SMTP id iq15-20020a056a20d38f00b000b8723fe21bmr14574616pzb.3.1674186233263; Thu, 19 Jan 2023 19:43:53 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id y14-20020a634b0e000000b00476c2180dbcsm21415255pga.29.2023.01.19.19.43.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:53 -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 v3 11/12] selftests/bpf: Add dynptr partial slot overwrite tests Date: Fri, 20 Jan 2023 09:13:13 +0530 Message-Id: <20230120034314.1921848-12-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2293; i=memxor@gmail.com; h=from:subject; bh=iON+lJXc9/1194SE26yS2gIj+59nymU6GNzP9SOpqJg=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg2941X5gs3080Qk68TQVWyXHYez/oGY+bpWZd/y LKoZw0iJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvQAKCRBM4MiGSL8Ryhz5EA CAZ4GdBHgrmi6MtMPi2BIPPNgKsiPJ41a1SrrQ6u0VGjXBy6irmYAVLKzZMzn1hmqGaZEKhs+Q2fln mNrfUMQRmPDGuj8pGF2Mtl5S8dexCcGrR2AG8PM5Xfqmp/YLlkiaaQxchxzlqBzYkR07NnHBp5gw5j CiRWwXkWDtlHSBZzKxrdcqWOSZIwRBePvWp+9XAjAKeDCdFCAHygorfYrpMUrbeoutz3Qq2TKhgMUe 1kHH3qZMklZt7QLY89rUHn7Xpj+d/Ihk3Q9stcJ76V1YNfzdMMMUVGb+srPVTren/PsDUdS/D7p8C/ uWSdTCW+kLpE2v7voViX0NKenXZ/lR8tw6Vzpp4NyyEdApt6ksdQjt8CbPYWwEnX2N2DXkMofm5cPj nS0g3TcWHP460J1Z4hPw9pGkppQS/nwiRftune5HmdBTKsM2YqstOhqUrWyxs00ryuMqO4XKpFCji9 lulv72wbgkP+xi441GzqDBXJfpAQKwQdN3jl03Y0K0k5UikKdvYQMYTouBDFNn4+q5+zfc78SP78QR BPMv9lY0e65CJACCuYdeOO7etUv1lFCJ/rchcX36A7urAXWJw+Qbh4PC9WBE3FUsOOFAyOk2EGAQjR poE7p66MeIERh0uRzYjoQVxr/O4sxq78ABhgaLZbYklJXGtCm/BdbFwazGmA== 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 9477c238af57..cf2d12329a1e 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 03:43:14 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: 13109099 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 075E7C38159 for ; Fri, 20 Jan 2023 03:44:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229823AbjATDoD (ORCPT ); Thu, 19 Jan 2023 22:44:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44942 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229458AbjATDn6 (ORCPT ); Thu, 19 Jan 2023 22:43:58 -0500 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 66169A2957 for ; Thu, 19 Jan 2023 19:43:57 -0800 (PST) Received: by mail-pj1-x1041.google.com with SMTP id a9-20020a17090a740900b0022a0e51fb17so1164489pjg.3 for ; Thu, 19 Jan 2023 19:43:57 -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=kVoBDZ5SNhH0ZFoFgKY8BU2apEtlHMmGUzr6lM5NKG8=; b=dsXHhSvwq+lHHudqbuFE/RSYDRxI26UqijY8g7re+tP09F2X6gRe+prdU9UeWfKhIx pWWb6rTpTKAu0NQB1cy35fHuPWevhfo88c3VwwD64nicpxu1Xo2myE3RQAuV+gnsNHD7 v+ibRIVBm1ljD8bjijBw4M/or22w+hh1zptmPBWbbvyVlPOsTwvDn0F62OGZ2JN0qKMB /SdXn7LuMwFpSheDZnEqX4dbfvnYZelXadthZ8iPsfYNBAxIzoY9dtCUwhnHxPchFsFB t4r2PFd/Jy39/ueF9ji30rUfVkUOI99IJPpj5z/n0nk9gOO6JbHbxWL3WVmZRmt5bbph al/w== 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=kVoBDZ5SNhH0ZFoFgKY8BU2apEtlHMmGUzr6lM5NKG8=; b=AoP902FvVGYn9eFlPNQ1xi+vLmE2xsIADADZm9gX5OR783loLV+74sjhzAbHkFT4y/ kJxfWnWGI8HxkimljxbpIB25/fmk16rDb4Eb7rkENT1jtJN+wNNOqc4PC1aZLAtTX72J WJ1Rq8dJHaGYnlHCTJ3/0rjJwDM+fMGV4YJXyML7u8RJc1+PJvDVlqORtVBQhqFSuCs1 YuGWaIl5ch5p2ALl+rqCuRDYlV670F6O2z/r+ka9CM9Qjs7OEAEqYPw5PX6+oGRwhJJd AlN+fpmvK4ljmHPyYPjnQZlWkzH+SzxQNmnjcDGJvKWj8VxblGK334c0EWr54LVsPjmu u75Q== X-Gm-Message-State: AFqh2kqRQeTyE0C4XVsgQtaeRJ0zuXx0QArDIAH+ytN21Wm0uEa9dVlT 7HGmiTMu5YoQs9BoK/nB73hBUIMemKs= X-Google-Smtp-Source: AMrXdXumpRquQLmqt+oskm4vNl2M1xqCDRXwzXZbgzwvYpVsITAVeVqpocby92GFsOhhjfxRyO8dZg== X-Received: by 2002:a17:90a:d393:b0:229:60cf:85d5 with SMTP id q19-20020a17090ad39300b0022960cf85d5mr13357622pju.13.1674186236937; Thu, 19 Jan 2023 19:43:56 -0800 (PST) Received: from localhost ([2405:201:6014:dae3:7dbb:8857:7c39:bb2a]) by smtp.gmail.com with ESMTPSA id e9-20020a17090a9a8900b00216df8f03fdsm384908pjp.50.2023.01.19.19.43.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 19:43:56 -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 v3 12/12] selftests/bpf: Add dynptr helper tests Date: Fri, 20 Jan 2023 09:13:14 +0530 Message-Id: <20230120034314.1921848-13-memxor@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230120034314.1921848-1-memxor@gmail.com> References: <20230120034314.1921848-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=HfD0Om3cGvwlhTzGOGIyATsjFl1oVSy4NxvuU8Dag5M=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjyg29g8kIXgjpTU6WXgms8HNf59l7I3AI8OiuQn3+ Kk5oy3iJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY8oNvQAKCRBM4MiGSL8RykMyD/ 9iT+QN7NP91n/QorxurSNNB+VYC6ByvmBj4h1IUstEe6Vsefp/AFKiI3BjDaigP3BcfmTZE9NzLBtR D4wMI2sI+iErrQWg7QhySrw6jf0tMWpXRSra5k2ljVphAJDXh9kdJjbVWdt+iBMObnN6vBSSfN2mxd KnUoM07dfbyECeVWzBqIF4ZYHuXFF9Zzr9dlvgQlb4NrbA1ldNlqiOx1b0+4Dz99wGtrIz4+Pvu/bB A2o3DV/6YXZtOvQrkRUztOXLXc+VtFPgXE7yl11sDpfGEm20ZXg4TXO4rVEWbfJXeCTmhr5GN3JcBb MuSMLrf1YK3m/5hYG+odIeRUKKMbsTyiepu4M2d7XmvzBkpFkBLHRDyYcclq6hhRmAs3jwDzgI6XDx WMCGTCkvb7ZTpjOC7oeFcam7iKaf6C7OugvgJQZgz7sQAAfSZgeMSILs+76GNiLnmPolLccp/cbSPT wsppiH3kH1hPNvpCGuU8EbgoZl4Atz7UrOmLrluzNdQFXZgh6BKiG3GIk0Oui0m/yL6DnW+MkAMwLE QaE0fhuxRhCXjCcRjlHV6rXv7RdXEBSsk5t6khAF89W32rdM+7/mGon+VUUEFbDuE2xp8eQl4vSLmU frUv75ZYQ7YyI8ZF0A23Rm4zs/mKCOCcXlNToWdE5RFRNrJgrP9lL/ItO1KA== 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 --- .../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 cf2d12329a1e..928ba778179b 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; +}