From patchwork Sun Jan 1 08:33:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13086333 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 C1AA2C4167B for ; Sun, 1 Jan 2023 08:34:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229553AbjAAIeM (ORCPT ); Sun, 1 Jan 2023 03:34:12 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39286 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229446AbjAAIeL (ORCPT ); Sun, 1 Jan 2023 03:34:11 -0500 Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D49B02AC5 for ; Sun, 1 Jan 2023 00:34:10 -0800 (PST) Received: by mail-pl1-x641.google.com with SMTP id 17so26377049pll.0 for ; Sun, 01 Jan 2023 00:34:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=7AuazTYNUQzO6gNY2IBgLBRjTazTtk639oaSsMmfaPo=; b=YrijGSKGXeYCvO3EnYwPMTzmOcLN2pVJM/7udPmQd1Re2lY8+WIWReggvq4F4W/Yr6 H2TP8qzCoOmEtk5grVpJ4RvxbGYFf7ZIOEitLQVjCOQ5ObCyuD2OeMqHu21d/+39GkVj mFCI5GFL8w8o7K78CIGPL7EQkFV7si/8xuVZjv75LspGGqg+cXjsAxlmqbtrVQlxws6k jMGH5HD/xanbMcN8nfxL/55+dQw0oqutmdEoPHT+uI2NX2g910bLovSxHz2jdP3iNPn1 wR4w8HjeWvbshsT0AYXlWRj5b0evEDGmCAS/XcvOy9KrtDlK0lhIXuT2w07AEESaaqCC Sbhw== 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=7AuazTYNUQzO6gNY2IBgLBRjTazTtk639oaSsMmfaPo=; b=lxOphggFVNHeOiH8cPOQR6lO6CZiZrNovr/JVxAxNZCU0deRdF7InyG1/xUIHDO+VX nkf07X9pFcJNeKaOuRo6O084Lht7Nto1IubTl9pWI8bAyulwguYpGayW4oYBeRNL/gwc RNsY5gonCunZwIqUOIVsydnyhnn5MMpTz6mVhCmlNcA1xKHrmcNzYUBQ9a1t3JfSb3v7 EoLwmnMQrg2m7qo8Omj0Jj91u/h64HzY1BQy+c7v31Wi2J3CBEyNUOUqIg00ZzkodvnT NZknK8R2LnA+STN5+2uBEMtQTWpxCviYq+Ds6zH13q/0lFNX2qtK/OrHIx+Ll09aR6zh W5rw== X-Gm-Message-State: AFqh2kqdl+6IB3Nz/ZyHeMSxsPKaSgRpzND4z6DZ3TZiMeihfLf4y6RA vBkgtpDphiRjzM0nm07zpuh6A1R6HWbauOLk X-Google-Smtp-Source: AMrXdXsGn4wuuOT6uv1Fju7xPi/o3l0cdqRJmskd0Mp2mHPeVmiqPiMPGJJQKryjdIL3nyYffEeuLw== X-Received: by 2002:a05:6a20:3c93:b0:b0:c30:5819 with SMTP id b19-20020a056a203c9300b000b00c305819mr60533604pzj.1.1672562050073; Sun, 01 Jan 2023 00:34:10 -0800 (PST) Received: from localhost ([2405:201:6014:d8bc:6259:1a87:ebdd:30a7]) by smtp.gmail.com with ESMTPSA id x10-20020aa79a4a000000b0058200ef9caesm3786220pfj.39.2023.01.01.00.34.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Jan 2023 00:34:09 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v1 1/8] bpf: Fix state pruning for STACK_DYNPTR stack slots Date: Sun, 1 Jan 2023 14:03:55 +0530 Message-Id: <20230101083403.332783-2-memxor@gmail.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230101083403.332783-1-memxor@gmail.com> References: <20230101083403.332783-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9155; i=memxor@gmail.com; h=from:subject; bh=fAx0a+r4415F16CuZnPL0MS/ZRZjqQ6wTRacGUUpIgQ=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjsUV0venLasCZnXFe+9oorY7PwfWDqVJctG6hQ46T yQAodZuJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY7FFdAAKCRBM4MiGSL8RyvdaEA CquIH4y/P5n25EAEU3H0PcDagTr8xapeXUERtbbhBvzH5hkq/rAGs5EFi5XDgutkKuw5QzLViSACiW OM/aQA6YHCkVMGFR183jV0RyUQwjQDiD7dfMYjjNVu8ZFaOQgJBFgfPuC5AhAPUW1JeqM/mOQf8FFu vsTwq/tDrPYWdFKqdO8N1Ap1PKZIcEuj48El5i5gKD+5FXuHR+2+au7ec3UGRkmrgcSb95RBjN6Dma +ClANuQYxL8+SrnUbicu6H0TiqPjMax1lAKB5ROCvfW3u35qFGZOtsCh8nriY2wENqvLytIaKTutck vEi4IsO4CEnMKFB9MHHZTo+UyeRUzaDmQ564KEm2F2ucLo9HHbncTgn26PKeSy1xb1qEGOGRka/OpR oDJjwBBF1n2QypPxm3SJiR/KcWdb7v5JDDyR92+xWV5L8OA2AbF55oDcg+/SLHscZChXaq17dtRDUM NieDmFDES8zC+KSqOcoVqx/F5U52d10KEns3rrV/3ucd4MruK1OaNwwJgcKJD1nchZ6/xYrgvThZY7 0bWFvlGbBYmYifxS7FcPuU9pz0qeqio5xFohqWqPSltUkVeZlPGin8zshTCQtDXc5kagCoLjskImHb OkTaUZHWHKvnN8dmagcEImqC2Xe65aLbZDTVMq2f4n/r7lgERcKXcCkRb2YQ== 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") Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: Eduard Zingerman --- kernel/bpf/verifier.c | 73 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 4a25375ebb0d..f7248235e119 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,26 @@ 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 screen off those liveness + * marking walks. + * + * This was not a problem before because STACK_INVALID is only set by + * default, or in clean_live_states after REG_LIVE_DONE, not randomly + * during verifier state exploration. Hence, for this case parentage + * chain will still be live, while earlier reg->parent was NULL, so we + * need REG_LIVE_WRITTEN to screen off read marker propagation. + */ + state->stack[spi].spilled_ptr.live |= REG_LIVE_WRITTEN; + state->stack[spi - 1].spilled_ptr.live |= REG_LIVE_WRITTEN; + return 0; } @@ -2388,6 +2411,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. @@ -5928,6 +5975,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 err; /* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to an * ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*): @@ -6008,6 +6056,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; } @@ -13204,6 +13256,27 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old, * return false to continue verification of this path */ return false; + /* Both are same slot_type, but STACK_DYNPTR requires more + * checks before it can considered safe. + */ + if (old->stack[spi].slot_type[i % BPF_REG_SIZE] == STACK_DYNPTR) { + /* If both are STACK_DYNPTR, type must be same */ + if (old->stack[spi].spilled_ptr.dynptr.type != cur->stack[spi].spilled_ptr.dynptr.type) + return false; + /* Both should also have first slot at same spi */ + if (old->stack[spi].spilled_ptr.dynptr.first_slot != cur->stack[spi].spilled_ptr.dynptr.first_slot) + return false; + /* ids should be same */ + if (!!old->stack[spi].spilled_ptr.ref_obj_id != !!cur->stack[spi].spilled_ptr.ref_obj_id) + return false; + if (old->stack[spi].spilled_ptr.ref_obj_id && + !check_ids(old->stack[spi].spilled_ptr.ref_obj_id, + cur->stack[spi].spilled_ptr.ref_obj_id, idmap)) + return false; + WARN_ON_ONCE(i % BPF_REG_SIZE); + i += BPF_REG_SIZE - 1; + continue; + } if (i % BPF_REG_SIZE != BPF_REG_SIZE - 1) continue; if (!is_spilled_reg(&old->stack[spi])) From patchwork Sun Jan 1 08:33:56 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: 13086334 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 39B8FC4167B for ; Sun, 1 Jan 2023 08:34:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229555AbjAAIeQ (ORCPT ); Sun, 1 Jan 2023 03:34:16 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39310 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229446AbjAAIeP (ORCPT ); Sun, 1 Jan 2023 03:34:15 -0500 Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 53BB462C9 for ; Sun, 1 Jan 2023 00:34:14 -0800 (PST) Received: by mail-pl1-x641.google.com with SMTP id jl4so20045350plb.8 for ; Sun, 01 Jan 2023 00:34:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=VLkWU14WmaVOIV0tqxiIFoeAF1xV5RkRbjjIO40C4iY=; b=l10qKzG4Up4bb9yQ3I14MRAxHRx3jMDoey51I4dl3wVnbY8NHba9kbi6RzLO2yYVp4 t6seaygZpxQknMPIeGOR+sh/SY5rirw3Rhh2K0xtOlX+YTpahdrUXh37qVHKqrkLZhkj cqeETVeEHXcN+mxPaWICx2nZWiad+PvM5JhPqdowoj14eLqBtALXrT5ZUgStgDPnNW2c M3qC09A7QKLVvrbUgHYs3TaWJimnzZxPUksgmrO+f3KGe8p5PDwp79hOkKr9g+KdJdh/ n7VO/mMMZD6vgq5YmKLHQ4A0VxfrfDlwnZCzlo4ZBoEOaxzZctM+PcILfXnvMGQYaUik Eq9w== 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=VLkWU14WmaVOIV0tqxiIFoeAF1xV5RkRbjjIO40C4iY=; b=d063MVX4UXUi26gNP3egsfOH9O2fq4q+o7iEPF7R57US7FQBPjFrA0E9h1/0W+4FRv 9Pk4b7ep43qeCFO6ysOA+jfEoE9LO6LITiaO8b7+fFF9Mptcb3BtWZ1b4SfqmeQHWkPY W5/D4u4uHRlCNHLIsANVokpYpiQnBP6Gpx4JFwKZuuw17jWmELiVTofW4Det8nQgjVo6 3/uRIdYStxLXqBMuFUauXgvv8ryCEUDGoYo17rJXd8P96ooV5y5hYgv7HeDmdX41yYS0 ArzEzpaLnlnAgs3YppneqLfrDhgPZv+2lhm+ox2343n2P0s0hnWNjEw7g4b/LinOPskE x/rw== X-Gm-Message-State: AFqh2kqlPbgLWwmLtpNE5mb7VK/Ok/ZJUxFCdfBoImyBT99kpfkiMbiW VeQlp99ES3om2sVnaMlOEwsdt7xQ7Iq5OqJt X-Google-Smtp-Source: AMrXdXu6qjkZw+S9hUxa/ml51vzi4kfrD7nrjSB4JXGp98+wrYTvtEjQ0AlcvaF0atyRzUWpluxcoQ== X-Received: by 2002:a17:90a:348d:b0:223:d8c3:c714 with SMTP id p13-20020a17090a348d00b00223d8c3c714mr39124252pjb.37.1672562053582; Sun, 01 Jan 2023 00:34:13 -0800 (PST) Received: from localhost ([2405:201:6014:d8bc:6259:1a87:ebdd:30a7]) by smtp.gmail.com with ESMTPSA id x14-20020a63fe4e000000b004a281fb63c3sm344326pgj.87.2023.01.01.00.34.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Jan 2023 00:34:13 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v1 2/8] bpf: Fix missing var_off check for ARG_PTR_TO_DYNPTR Date: Sun, 1 Jan 2023 14:03:56 +0530 Message-Id: <20230101083403.332783-3-memxor@gmail.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230101083403.332783-1-memxor@gmail.com> References: <20230101083403.332783-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9589; i=memxor@gmail.com; h=from:subject; bh=7eS+FfjKMNjLuzgV/2TcTA00hQwDINQ+MVG3xTyYlxo=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjsUV0sdEDUpGefJ6GxRdyBvdJfHo05NhYPHU1/6Zl N30f8CuJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY7FFdAAKCRBM4MiGSL8Ryh2VD/ 9MDJCs1kKaHratXkiwlQcN3pq7RfD6Ypwdv58wZFg/PAkcN4dYke8GPCq6Bvth3Ii/a86pn8I6q62r Dykn9SvYom6OegZbsFvKQKgYnFV+79taFNQ6CLNrilKUXRoYCQ67kYEHOlS2NrB5z6HPWdhCASy/wd YH5af43shVmLbXZMwuCTbJI+CIQCxKuD2Gv3x95fISFSql+psTbF71fX449rcqlpWPavYpp2PeJ6f8 Xa3P9KNZLRIF0HOw6dKAhJ+DZMS/XcVRNyp7Q6jH16BvkdHfhq4B4cLsMVHNnOfIsTcj7OvWcDalAa FbMlgxfuaI4QIOmeIk98KZ5nqOeB/5AexG/NiciGINDORbUPwghhsbQSufK1EuN/xin+1+aMlNMvCt R61eXJqOR3MQF3mNtf/m/bCOSE7SVcoKrPAwRRJtD2PIGoMiTGUFd+4jRaqkcjPHAgdDyskMUupZHY T0unfezCyjMX/ISvcjQG3B8mdwnXnriG0KvuDI/uPw3q63R42d6Q6wwOxLniS8uAwLgii4HMlYy3hF XMIpaV99QwOz2jU6lkCxK72Y+2Nn5jtVMDT0JHkMjyhWT+G9N68LF+07DnB9LJ+ish8b24+e+WLVA5 ZQw0ACc0j25hBEUq274i3QAq2L3PIQBED3OEdYCgxjBf4CaNFWPT1rslueMQ== 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") Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 83 ++++++++++++++----- .../bpf/prog_tests/kfunc_dynptr_param.c | 2 +- .../testing/selftests/bpf/progs/dynptr_fail.c | 6 +- 3 files changed, 66 insertions(+), 25 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index f7248235e119..ca970f80e395 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 the 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", reg->off); + return -EINVAL; + } + + spi = __get_spi(off); + if (spi < 1) { + verbose(env, "cannot pass in dynptr at an offset=%d\n", (int)(off + reg->var_off.value)); + 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; @@ -839,7 +866,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 spi; + + /* 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; @@ -855,14 +886,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; @@ -891,7 +923,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 (WARN_ON_ONCE(spi < 0)) + return false; return state->stack[spi].spilled_ptr.dynptr.type == dynptr_type; } } @@ -2422,7 +2456,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 (WARN_ON_ONCE(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. @@ -5946,6 +5982,11 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno, return 0; } +static bool arg_type_is_release(enum bpf_arg_type type) +{ + return type & OBJ_RELEASE; +} + /* There are two register types representing a bpf_dynptr, one is PTR_TO_STACK * which points to a stack slot, and the other is CONST_PTR_TO_DYNPTR. * @@ -5986,12 +6027,14 @@ 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) { + 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. * @@ -6070,11 +6113,6 @@ static bool arg_type_is_mem_size(enum bpf_arg_type type) type == ARG_CONST_SIZE_OR_ZERO; } -static bool arg_type_is_release(enum bpf_arg_type type) -{ - return type & OBJ_RELEASE; -} - static bool arg_type_is_dynptr(enum bpf_arg_type type) { return base_type(type) == ARG_PTR_TO_DYNPTR; @@ -6404,8 +6442,9 @@ static u32 dynptr_ref_obj_id(struct bpf_verifier_env *env, struct bpf_reg_state if (reg->type == CONST_PTR_TO_DYNPTR) return reg->ref_obj_id; - - spi = get_spi(reg->off); + spi = dynptr_get_spi(env, reg); + if (WARN_ON_ONCE(spi < 0)) + return U32_MAX; return state->stack[spi].spilled_ptr.ref_obj_id; } @@ -6479,7 +6518,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); 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..32df3647b794 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; @@ -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("arg 1 is an unacquired reference") int invalid_write3(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 Sun Jan 1 08:33:57 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: 13086335 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 C6BE6C4167B for ; Sun, 1 Jan 2023 08:34:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229568AbjAAIeU (ORCPT ); Sun, 1 Jan 2023 03:34:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39344 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229446AbjAAIeT (ORCPT ); Sun, 1 Jan 2023 03:34:19 -0500 Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 306EB2AC5 for ; Sun, 1 Jan 2023 00:34:18 -0800 (PST) Received: by mail-pf1-x441.google.com with SMTP id z7so11297105pfq.13 for ; Sun, 01 Jan 2023 00:34:18 -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=qrujwH51Ce0LjiV0lKl9NSqEeF9GoNHU8d2fm/eTaQY=; b=pzYXsgt1O4WgyDzxJ7X3b8F/CShjrLvaW1lIg8aifLw91XXvxwUszbGgHFJjCv9Eja s/dT6mluiLdaRAVe23DZlIjgz+wIqtsIzDzqZl+upBkKhKmDt9djip1si9cnHX7utLS7 dyIe4tyI1PzbwpkpQyF4KdpVf0lVdia23RpV1WNzY0CG5kpOSajOFUjdB6e9lwpY4AHz wk2LOqQsVBFLjgKs606RE6jRYafXpMuLtGz0cII2w7lTjGUXVfHgyfy/GOKlFi69Fw1N mZaghIwbESooZZviFlrNXv3xwGfhzyQQmL+vgTy53Vd70G55angYZRQhiBJYZgApuyUX yRPg== 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=qrujwH51Ce0LjiV0lKl9NSqEeF9GoNHU8d2fm/eTaQY=; b=r61/RQ5WltR/fYtguzdDKFkuD71LTbx2rMnItpPG5rD7UZ122UQsTn7LN3oFiro8ly ZyY9cdkGnxVLNzEXHCCKHsJcoobtiFvq9sfdnmmeTdGMgYNcxiJtLKaZJyce+QRQgVoU BV5vD11XmzZOSK5t6bzSAARrwg0zA9zqPDZGmpAMKvqnM0WIsSwnMijPB/RCiAI4SGmm 2ky6tAUvqxlQjzvoF0V2oyWRizmUOW4SbMqMGf2KgyhWE4sBLp7jkHepyXINJvILN0UQ 8MZj2qbwuf6s0ckWMpR3E5Algl/0bs/RFjwH+EB4ltW8plABMFARwiGakMPDeVDCuJUH d1dA== X-Gm-Message-State: AFqh2kq5ywyMpb+7Qgctu7cHJH47V4ZriuQcEOgTcqUDy1c/+4XztoxJ 85udFxaBVD3Nc0jkiy9m7Unq9VT/WOVrObTz X-Google-Smtp-Source: AMrXdXuerJz4xEOA7q7qM0dkPfn730fd7+0Kc5eFPnSqrm5lSbM3i36lBfF1RCnN7RUQ1G77ii5ZQg== X-Received: by 2002:a05:6a00:99d:b0:581:38bc:b5bc with SMTP id u29-20020a056a00099d00b0058138bcb5bcmr30457952pfg.1.1672562057373; Sun, 01 Jan 2023 00:34:17 -0800 (PST) Received: from localhost ([2405:201:6014:d8bc:6259:1a87:ebdd:30a7]) by smtp.gmail.com with ESMTPSA id 68-20020a620647000000b005756a67e227sm16831954pfg.90.2023.01.01.00.34.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Jan 2023 00:34:17 -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 v1 3/8] bpf: Fix partial dynptr stack slot reads/writes Date: Sun, 1 Jan 2023 14:03:57 +0530 Message-Id: <20230101083403.332783-4-memxor@gmail.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230101083403.332783-1-memxor@gmail.com> References: <20230101083403.332783-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=7339; i=memxor@gmail.com; h=from:subject; bh=p7RBaq+OXA6AXW4qlo40pbLXIWP1c21AGKhBd1Hitpc=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjsUV0turUjdqE/u1VgUR/z4OjyX0/cUy8QBrXV8vQ LNSsKh+JAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY7FFdAAKCRBM4MiGSL8RyhwND/ 4gRN1TyvC7ypwkrROwlrBHvzAbKxIP2bW8XRhkG9EBzZLffqGiRmjZpgVPfJJWgWMNSkVqPHDM0/cP 0vp6SkC+9ueJJjtpAfCKGpAaCH4+ULyfQfHAHuIPFnsNTLu5gPxqBEGHJoc9SBUqj3K91UVBG7UI0v jHzrUq92d01c8AeQhaw19gaWCoIXHl2orkkbXl+qBQ06C/98PesDEacJiEviDmFUbH8Hqf8JVyAjXA zaRtUUyZ8PJ7cJRQ/7PF6qujKM7fRM9ONIErBRJD6UXDvbcdmmCP3kwliGvycuQimzRFdZBPybX7ft cfbLjhX36TZeGSEvWNvzkp4DOpRDAo++eWZkR5+VaU/1RxpjFfw560aCwkK5z3KMNNn6dCCaVBAyuC N/2RXmJFpMgm0QwyfZhTJuhDIvbZGsh/XYwOMsagTImOWTUYybmoKtBCDgMm/RiGrwXv0Yri1Taegm 4WRIY8dspbunBqUf5TkPOq3K5WOZzv7xm+glvM+mViAGPxDcyzty0rntIjL5N3/MXxwRgzoVJQKIdX 9NvM+anKU7j2BKXbhyKkUI4VacJA6XYyeAQYNrpB5GxS3ePAAtl0IHKurFgjS4vYdsvrBiDsxSEgzV JXbKqZAi0tfRv+lqCxuQGzTxh/EAZTOt2Y2mJCteM+o39ceDTabq7Ls6nKKQ== 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 | 73 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index ca970f80e395..b985d90505cc 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 void destroy_stack_slots_dynptr(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) @@ -858,6 +860,44 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re return 0; } +static void destroy_stack_slots_dynptr(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; + /* Reposition spi to first slot */ + if (!state->stack[spi].spilled_ptr.dynptr.first_slot) + spi = spi + 1; + + 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; + } + + /* 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; +} + 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); @@ -3384,6 +3424,8 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env, env->insn_aux_data[insn_idx].sanitize_stack_spill = true; } + destroy_stack_slots_dynptr(env, state, spi); + mark_stack_slot_scratched(env, spi); if (reg && !(off % BPF_REG_SIZE) && register_is_bounded(reg) && !register_is_null(reg) && env->bpf_capable) { @@ -3497,6 +3539,13 @@ 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 slot, spi; + + slot = -i - 1; + spi = slot / BPF_REG_SIZE; + destroy_stack_slots_dynptr(env, state, spi); + } /* Variable offset writes destroy any spilled pointers in range. */ for (i = min_off; i < max_off; i++) { @@ -5524,6 +5573,30 @@ 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++) { + slot = -i - 1; + spi = slot / BPF_REG_SIZE; + /* raw_mode may write past allocated_stack */ + if (state->allocated_stack <= slot) + continue; + if (state->stack[spi].slot_type[slot % 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; From patchwork Sun Jan 1 08:33:58 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: 13086336 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 B4D93C4167B for ; Sun, 1 Jan 2023 08:34:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229446AbjAAIeX (ORCPT ); Sun, 1 Jan 2023 03:34:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39386 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229586AbjAAIeW (ORCPT ); Sun, 1 Jan 2023 03:34:22 -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 A45D662DB for ; Sun, 1 Jan 2023 00:34:21 -0800 (PST) Received: by mail-pj1-x1043.google.com with SMTP id v23so26954858pju.3 for ; Sun, 01 Jan 2023 00:34: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=wkER6R3nU+Ytefc0BYVC8Qo73Hz+RmjIOdmzqSommN0=; b=aVXZFrXR02jvb8yFiaA14lG0A9vd1KE1mNmV34huTC9MvLIWHq2VkfkZDGZ4b5w8dv kvjferWpxWMjTTGUJWlUYXfKcoMMYW1Nn9Z47+3dQX4x/8ELspSRKQM3nJRroww0DEo/ dkt0gEKbiE0VvOpZD88+pj2AE4EKY00qFWvMQY7e8AxjMOwIpTCXmQgU8Xju5WYdDWA8 ACcL6HTZbMAw9yWIJw3+ZMdS+fAIjMwRtOsNpfD7LW5JeM01MURmhBYlC9SreDCqnJH0 g2rSR2U0eBpSeUT2kQEBUGZ/P1h2grWi4zFGxcnEMKDx+PW/M1NxUoplGo4pdUdQFrwF 2yLA== 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=wkER6R3nU+Ytefc0BYVC8Qo73Hz+RmjIOdmzqSommN0=; b=1sYz86QC5P5mRFNMgtDgV8aX8e0iIkY08KDpqfovZ6z/zYba3v7bRQAbyXlmb4Chcr YlJJBuYn/M3r1oskLe9pM3QM0tPR8/1hvZ2OVnUcDO53eArvDxj4cz0GgQKEJ07ipw36 M4vhTSMnBekDmkmven8axmJMzUDW97+XfQAlCMdGzRRxoMysxvOzkEsHGBRKxrfw1Rpr 185WtbvZNnVLprdQeqe7ZcEO+r7LFVPMCoEdoUxFHk6ALYSyruHhFI4tF/059LxyyqNG OC0VxItNoxN/r6Zl2ud3mOf0o4ODinXRlnAJpIknOW6LCecuU/oK8+uuKcAivCJnTvUx lbSg== X-Gm-Message-State: AFqh2koUe41KoYuZIJwUkvtBu81765/wwodLu//RdrUQEvDkc9BpIkAG LzK8EqKOfR3n+FDUgmNsRloBIQxOWZ3RY6Fm X-Google-Smtp-Source: AMrXdXtq+AX0VUkVsuODCCFpPuBJwxx5hMEk08eUlkJ8sH+G9GqcYoVgeirZbx6sjUipIPpYRKe03Q== X-Received: by 2002:a17:903:40c8:b0:189:ab82:53f5 with SMTP id t8-20020a17090340c800b00189ab8253f5mr30458954pld.40.1672562060981; Sun, 01 Jan 2023 00:34:20 -0800 (PST) Received: from localhost ([2405:201:6014:d8bc:6259:1a87:ebdd:30a7]) by smtp.gmail.com with ESMTPSA id u6-20020a170902714600b00189c62eac37sm13159883plm.32.2023.01.01.00.34.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Jan 2023 00:34:20 -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 v1 4/8] bpf: Allow reinitializing unreferenced dynptr stack slots Date: Sun, 1 Jan 2023 14:03:58 +0530 Message-Id: <20230101083403.332783-5-memxor@gmail.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230101083403.332783-1-memxor@gmail.com> References: <20230101083403.332783-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2367; i=memxor@gmail.com; h=from:subject; bh=p9tJlATdM5FakqCpJKa6NnaquVsvZRS9oFRwkCo7zlg=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjsUV0Pj5El18NahWcUEIYLgaLO0npnAZeh4llskj/ TG2jWaGJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY7FFdAAKCRBM4MiGSL8RyvFXD/ 9tmzENivKcTiw5ZebT5O1AQj8CA8alzKcz58cgdT/nAJLMUDLIgqa4oGxKs4U0CWP80pBaO/9jeeVk 4Hdapzshz3xWSl26Nq6C2wBH2bLoRBdfMi6B93yGQGWVpK6/Wdh4Pw3E7R8oOrO5xxSWyu4ygmsDHd 3YMjD+qfJgAQGI0p1I0rNwli10HAp/f/YJ2LtKW0J6e8PQlXxSE2RhNTkmDk4JJRCmYeB9HcJ0vngG yqoVlbbFHHaKYcXeq2asSKFuWRABlPPzwwNr6O0tCyc/tHe8otP923JPONbcrzqiE7ecHf0MzU8t/9 0LvvTzW4MVT17WDgoocCQzAeYK+4RMG7nm4c4Yy9hUkqxyL1zLyPQ0Qj4qkvi2+Mjb/P+4rnW31P/X HDcRhJ6xLhFl5nm6i/0iA9z7KVV3fp8ZT4QUCAPWpvB6eqELZoB0rY2osr2XFH5FlViX4noOF0sWIG qK/iqqtIGV/CZxhrA+C4xtp4K9z0jgKKQwamhOiV+px1THTBPAlZC66bbOaVzN81mxrZcd9znvCnFQ 1Tvsfyp4WPUUYwRzf/15BXkfSVDjCGsz3rMVMBbuvk2N9KFvvHaLJio7FR7c0TDl6thLYPXNcg46A5 b67gwQyXyGP/LhqVW48Lbft7Npw2XR7ZK2iWqIXeDrGcwCf1uTfYOFx6hxLw== 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. Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index b985d90505cc..e85e8c4be00d 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -786,6 +786,9 @@ 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; + destroy_stack_slots_dynptr(env, state, spi); + destroy_stack_slots_dynptr(env, state, spi - 1); + 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; @@ -901,7 +904,7 @@ static void destroy_stack_slots_dynptr(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; @@ -914,12 +917,11 @@ 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 STACK_DYNPTR slots, see + * mark_stack_slots_dynptr which calls destroy_stack_slots_dynptr to + * ensure dynptr objects at the slots we are touching are completely + * destructed before we reinitialize them for a new one. + */ return true; } From patchwork Sun Jan 1 08:33:59 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: 13086337 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 BBA7DC4167B for ; Sun, 1 Jan 2023 08:34:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229626AbjAAIe0 (ORCPT ); Sun, 1 Jan 2023 03:34:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39426 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229586AbjAAIe0 (ORCPT ); Sun, 1 Jan 2023 03:34:26 -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 45B016246 for ; Sun, 1 Jan 2023 00:34:25 -0800 (PST) Received: by mail-pj1-x1043.google.com with SMTP id fy4so27009412pjb.0 for ; Sun, 01 Jan 2023 00:34:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=vTGrPnxdHr6DphV3iGM3VdIYy8slnPP3hxiIGDfb5OI=; b=hDAwqMuLNjsSSzmkbi54gWpa7GZ+UTmDJ0HuBAFMFlr4OukwbnZN9rTNldKuyBk1DR MJv77qS38pDOBi50cgPSGHU8xUsJeLMES2r4oRPeIEfSf793Y2V6OAgZwQ6cJ4R7yPcr Hs/ssMLKFjzQbEnpcH5L8RQVEkqYFcyKpdqseRtmpAY0ja7Q2ZyAdS2BToOY8egQ7nsb IRjc9zdxpGnflmxiWBlweQkdCCUGMki2qsGy/1ECLcXnHqaxsZO9SMY9SW3L419YCB6d NvGtwGJtZzqykJcOXw4lSZ+Q6FGYhGjPOwcAK5I3tExLSVkz41UNIxx/0mCTk0L1IXEx t11A== 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=vTGrPnxdHr6DphV3iGM3VdIYy8slnPP3hxiIGDfb5OI=; b=ikIsPwziFSoS4P/VQSOJBZtlb7EYjrfWbvv+mohX0lOXvFlGTJtMULFhPxbY/AN8Bh GZ+r5c+zSg3c4JLqfUoCnr9gk5/aTXA5gqP5HDyAzsej0TcsUfxgc/fVRdtzeAt8rnX1 FXDC4Vjlq0JH2W7J3EYnQJFyiD7e2pERNstFrWZp9hcX86YVfJvImLHw1F5gJLhK1xAZ sWn9pXDR63UMdzvRnkY5xz6rmC1E7MlQsU4hG9CHNdOn1KmtQxoXnsvtHSrZUP3TnkgL F4VUhFs6mY0qYIzKpeMi1yAQtPTcVp5PXuFGsWXZYaKJ/Tir2SaS+FPBaLWZGHb1IO1v cU0Q== X-Gm-Message-State: AFqh2kol8phsgaJLAxkE93SzLLIDJiCrMkrHsP3XkIPaWTQZh0igLgb2 hND0RNkV9Kf6Y0tdp5+yuBbnHhaewrDoXTbw X-Google-Smtp-Source: AMrXdXsLkk+R9mV7J7MPajHaZ7IkEIYMMykuUcgeJzsaHc0BDQICnStSzA4RXkXgvQrdi1L56cBYtQ== X-Received: by 2002:a17:902:ab1a:b0:192:9924:ce7e with SMTP id ik26-20020a170902ab1a00b001929924ce7emr13609924plb.55.1672562064496; Sun, 01 Jan 2023 00:34:24 -0800 (PST) Received: from localhost ([2405:201:6014:d8bc:6259:1a87:ebdd:30a7]) by smtp.gmail.com with ESMTPSA id jn18-20020a170903051200b00189393ab02csm9419249plb.99.2023.01.01.00.34.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Jan 2023 00:34:24 -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 v1 5/8] selftests/bpf: Add dynptr pruning tests Date: Sun, 1 Jan 2023 14:03:59 +0530 Message-Id: <20230101083403.332783-6-memxor@gmail.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230101083403.332783-1-memxor@gmail.com> References: <20230101083403.332783-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=4765; i=memxor@gmail.com; h=from:subject; bh=EjP8Xq5SudoQovN+R6iIEKmoCGjUc8QjuO48957Psgs=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjsUV0p9H75i8Qsw18by6rww8G0sbDD+WM3+IVaOjC 3C/FaHuJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY7FFdAAKCRBM4MiGSL8RyphcD/ 49c+HAdgLekQWgvnP+u6oL4kJYKW3gHaxGbP+SXtzHJwan+BeCAMyP9Wxu1pN5wb8I5g3JVuFHveyy jRJj3BzWQFcPRH0w1iBZZzn6qwM9wjQdjTkBw0DX2TOmk4Nj9eankQpACa9X5GBDC3dkLk1vnsXyO2 +eYy9vEx7MMnnxpKeu0maU00xWdhiOcHH59qDyCVs4iY1FMMkbC8UzgIs68uayc4whqqNkE/5ziXBt pNLwPjhHkVzOqy2D2NtHqx8lr4U+8VRAzVU6cFL8Q0RbU/UJjIK4ye9/zkHCVvVjBFWONGN8RHICIf R9iivBfPz65bgXuDwpxT5LAT5KHdWi0ac0i+WXERYSyJVThkJ2oW6aUEhPqy6N2YTTQxSJ1/WCzwgH pJ504yEGrHUI79erTKSBJqTtWiBFbPIiQHMR6ay8U8l4GzjqfpRATie0gFjv04Wa9HWu6Fo3GlahFr OFWzCL4ZfguXq8SzpSxbHeUwQveRDDYKA388nEcV6419ppsX0MULcPFcvNRF2nf4ldIxhjbTqLQJKA pS5Cj/3tFLtggoTc1wM67C+LMwfT+BleFQTZuTbvt+Nc2YT7UXIcYI/SNEvZ5blln3MUJNY10s4+Yx cLZXtvWf64LtnecebaR1m0lpBbwCJZbv030rSntuciKVwvK0tLFYXXc2SWiw== 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. Without the prior fixes, both of these bugs trigger with unprivileged BPF mode. Signed-off-by: Kumar Kartikeya Dwivedi --- tools/testing/selftests/bpf/verifier/dynptr.c | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 tools/testing/selftests/bpf/verifier/dynptr.c diff --git a/tools/testing/selftests/bpf/verifier/dynptr.c b/tools/testing/selftests/bpf/verifier/dynptr.c new file mode 100644 index 000000000000..798f4f7e0c57 --- /dev/null +++ b/tools/testing/selftests/bpf/verifier/dynptr.c @@ -0,0 +1,90 @@ +{ + "dynptr: rewrite dynptr slot", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_LD_MAP_FD(BPF_REG_6, 0), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_6), + BPF_MOV64_IMM(BPF_REG_2, 8), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -16), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_reserve_dynptr), + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1), + BPF_JMP_IMM(BPF_JA, 0, 0, 1), + BPF_ST_MEM(BPF_DW, BPF_REG_10, -16, 0xeB9F), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -16), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_discard_dynptr), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .fixup_map_ringbuf = { 1 }, + .result_unpriv = REJECT, + .errstr_unpriv = "unknown func bpf_ringbuf_reserve_dynptr#198", + .result = REJECT, + .errstr = "arg 1 is an unacquired reference", +}, +{ + "dynptr: type confusion", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_LD_MAP_FD(BPF_REG_6, 0), + BPF_LD_MAP_FD(BPF_REG_7, 0), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_6), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0), + BPF_MOV64_REG(BPF_REG_3, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, -24), + BPF_ST_MEM(BPF_DW, BPF_REG_10, -16, 0xeB9FeB9F), + BPF_ST_MEM(BPF_DW, BPF_REG_10, -24, 0xeB9FeB9F), + BPF_MOV64_IMM(BPF_REG_4, 0), + BPF_MOV64_REG(BPF_REG_8, BPF_REG_2), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_update_elem), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_6), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_8), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem), + BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1), + BPF_EXIT_INSN(), + BPF_MOV64_REG(BPF_REG_8, BPF_REG_0), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_7), + BPF_MOV64_IMM(BPF_REG_2, 8), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -16), + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_reserve_dynptr), + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 8), + /* pad with insns to trigger add_new_state heuristic for straight line path */ + BPF_MOV64_REG(BPF_REG_8, BPF_REG_8), + BPF_MOV64_REG(BPF_REG_8, BPF_REG_8), + BPF_MOV64_REG(BPF_REG_8, BPF_REG_8), + BPF_MOV64_REG(BPF_REG_8, BPF_REG_8), + BPF_MOV64_REG(BPF_REG_8, BPF_REG_8), + BPF_MOV64_REG(BPF_REG_8, BPF_REG_8), + BPF_MOV64_REG(BPF_REG_8, BPF_REG_8), + BPF_JMP_IMM(BPF_JA, 0, 0, 9), + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), + BPF_ST_MEM(BPF_DW, BPF_REG_10, -16, 0), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_8), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -16), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_dynptr_from_mem), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -16), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_discard_dynptr), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .fixup_map_hash_16b = { 1 }, + .fixup_map_ringbuf = { 3 }, + .result_unpriv = REJECT, + .errstr_unpriv = "unknown func bpf_ringbuf_reserve_dynptr#198", + .result = REJECT, + .errstr = "arg 1 is an unacquired reference", +}, From patchwork Sun Jan 1 08:34:00 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: 13086338 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 43143C4167B for ; Sun, 1 Jan 2023 08:34:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229623AbjAAIea (ORCPT ); Sun, 1 Jan 2023 03:34:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39458 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229586AbjAAIe3 (ORCPT ); Sun, 1 Jan 2023 03:34:29 -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 96A2A6246 for ; Sun, 1 Jan 2023 00:34:28 -0800 (PST) Received: by mail-pj1-x1044.google.com with SMTP id m7-20020a17090a730700b00225ebb9cd01so18031343pjk.3 for ; Sun, 01 Jan 2023 00:34:28 -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=9NdDIoiTNx/8nBOVqwD3u8E3YffJXgMuSkkETx6NZTg=; b=UpNMqfvp2AvlTHdU1HZqH6byI+aDb1TdAQ9QXKvSuGVFtCE/DMxNidJ9cLWUsOYr45 zwYGnUnOYCmgkqHgU4xpDUm+s+odTmTT6mMEVKBqy8QDRc0f3PdMoVJ7YuDn9D3O2KvM 22XIhoTL/LqJWtrzXsts9uP8Y6jMAgr75ZTc8n+WH5JHgekTJRSmgZMEWS19STo49H8G rpzr8L9cPY9Xtx+IssyrWGm2MYnhCUelea0eGZR58CQxEgfOmJiJdXNwjww39uS3PKoz tVNTnfVMuXFTBz/NsmM85w/pHtP289uYa/C+CLCNrOs4ryvx4HQ9GUMDsEccowYO5Gel 32hA== 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=9NdDIoiTNx/8nBOVqwD3u8E3YffJXgMuSkkETx6NZTg=; b=BIXO6Vp2P7QVzH3WWRNi3l+UKmykTZsi/FCiCvCHk4E/R8cfjDxKVsvBkTTBov6A0g CMQKJJgoMm3SPBwzb862eAE7KEV7Oox/HCRtWVdfhHeV5duli7mHvAwUEubuBqB/r8Dm 3gypypekPYoWzP5dWjnSXsyGUJLzph1eDiUM5in+z37aOHjMIihOzgxjjefb2PIy2JC2 aKzS8SJ9PpTvZhU5MzDNkSfPxGgpqOsgTquQyGiejBwSQk1vvh4jPKc+4OrSbHrTM1Hu 0VsS5oNVvzOzFAaVD8Wg3N9FrpvOS4LuSrF0pWc8knjV18HGI4Cl1Ud7O9LCBVZr9Ypc qPGQ== X-Gm-Message-State: AFqh2kp2XiC5nK6fwds3bfS/fseCNxs/8gtobYeAJ6/6uxCcqm+Hq4yX j8nq9mD6ImNBnIVD3Yo7CidMqIELLmyNwRo4 X-Google-Smtp-Source: AMrXdXvM0d0mRAFgEp1ox0ssPFJNt0GXKI+z2WQP+7ezKQHFsPTYCps/nZWc+2+WKvmg6/nmuG9niA== X-Received: by 2002:a05:6a20:4f86:b0:b0:3ff8:282e with SMTP id gh6-20020a056a204f8600b000b03ff8282emr22958606pzb.6.1672562068095; Sun, 01 Jan 2023 00:34:28 -0800 (PST) Received: from localhost ([2405:201:6014:d8bc:6259:1a87:ebdd:30a7]) by smtp.gmail.com with ESMTPSA id q24-20020a631f58000000b0043c732e1536sm15221837pgm.45.2023.01.01.00.34.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Jan 2023 00:34:27 -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 v1 6/8] selftests/bpf: Add dynptr var_off tests Date: Sun, 1 Jan 2023 14:04:00 +0530 Message-Id: <20230101083403.332783-7-memxor@gmail.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230101083403.332783-1-memxor@gmail.com> References: <20230101083403.332783-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2558; i=memxor@gmail.com; h=from:subject; bh=jAVWM2AMRTegvbN+p23/XBMDwp6jZopeYu2HCKwy6KY=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjsUV1jSkgOHzecyZaWzNkiMCMGj6SooC1TOJO3e+h ypYb0zSJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY7FFdQAKCRBM4MiGSL8RyhKFEA DDXPtmLxD+VHeRUIJVkehB94qjhpu9DUagT+1vJDu817M2xNiAsvoMQh7LkBV1XAc5NC/N6C6NbOBq 5/CkAN2cXypbQZelKQxLhvzNumZT/tubC5CsaSDu4+zBUCWB97KJ0rKipTyA0PJVq0WMzR1qQ/vnNp HqPAFp+zhcMOugurFxCpoeXI+qGNWCvNveJ0bmz9EOxXTacBCxrtxAj0zggMNDVGOQlq05oaGJZsYr v94wivB1bQbWb3DsK3qAv3J1OBghk3dt179qdaaGBbE6rlTZUropYk1bI95nxUJQX5hsyE/xvcb+KF cESKpxW4fB2XLq4UMxNirAuPMXPg2H/Sn8O/5XpFyuychLqnWZ4igI+0OjkvZVWWVcDDLCfzT5KgI3 ikYoZLNCzgtaAanb5f9DGwfQ8VzO2ZiG3BOxLvUZ1EyxVY6FdH4vW9EBEB7biEZ20FoENxetHZSu4O FA7inxhWXbRi13PrCq3yY0CEMPhg71DEtBSu0u842WTPSIT3xtuj5mk+3f2Ve/WXzmnc/KIGfJsohu 8LoCY/N8D97sjSwjig5d4bDApBykDfrSZCgfHxQNsBpjZj6I4z9iR7NvaScD7vfYWuZsdFgqsSQPmX eW7w6y9wrQ3u40e0JA5oSejaTRAJIYi26I8qL+q0jlTojUG34c49IiHcoo2w== 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 ensure that only constant var_off is allowed. Make sure that unprivileged BPF cannot use var_off for dynptr. Signed-off-by: Kumar Kartikeya Dwivedi --- tools/testing/selftests/bpf/verifier/dynptr.c | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/verifier/dynptr.c b/tools/testing/selftests/bpf/verifier/dynptr.c index 798f4f7e0c57..1aa7241e8a9e 100644 --- a/tools/testing/selftests/bpf/verifier/dynptr.c +++ b/tools/testing/selftests/bpf/verifier/dynptr.c @@ -1,5 +1,5 @@ { - "dynptr: rewrite dynptr slot", + "dynptr: rewrite dynptr slot (pruning)", .insns = { BPF_MOV64_IMM(BPF_REG_0, 0), BPF_LD_MAP_FD(BPF_REG_6, 0), @@ -26,7 +26,7 @@ .errstr = "arg 1 is an unacquired reference", }, { - "dynptr: type confusion", + "dynptr: type confusion (pruning)", .insns = { BPF_MOV64_IMM(BPF_REG_0, 0), BPF_LD_MAP_FD(BPF_REG_6, 0), @@ -88,3 +88,37 @@ .result = REJECT, .errstr = "arg 1 is an unacquired reference", }, +{ + "dynptr: rewrite dynptr slot (var_off)", + .insns = { + BPF_ST_MEM(BPF_W, BPF_REG_10, -4, 16), + BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_10, -4), + BPF_JMP_IMM(BPF_JGE, BPF_REG_8, 0, 2), + BPF_MOV64_IMM(BPF_REG_0, 1), + BPF_EXIT_INSN(), + BPF_JMP_IMM(BPF_JLE, BPF_REG_8, 16, 2), + BPF_MOV64_IMM(BPF_REG_0, 1), + BPF_EXIT_INSN(), + BPF_ALU64_IMM(BPF_AND, BPF_REG_8, 16), + BPF_LD_MAP_FD(BPF_REG_1, 0), + BPF_MOV64_IMM(BPF_REG_2, 8), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -32), + BPF_ALU64_REG(BPF_ADD, BPF_REG_4, BPF_REG_8), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_reserve_dynptr), + BPF_ST_MEM(BPF_DW, BPF_REG_10, -16, 0xeB9F), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -32), + BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_8), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_discard_dynptr), + BPF_MOV64_REG(BPF_REG_0, BPF_REG_8), + BPF_EXIT_INSN(), + }, + .fixup_map_ringbuf = { 9 }, + .result_unpriv = REJECT, + .errstr_unpriv = "R4 variable stack access prohibited for !root, var_off=(0x0; 0x10) off=-32", + .result = REJECT, + .errstr = "dynptr has to be at the constant offset", +}, From patchwork Sun Jan 1 08:34:01 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: 13086339 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 38FDAC4167B for ; Sun, 1 Jan 2023 08:34:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229676AbjAAIef (ORCPT ); Sun, 1 Jan 2023 03:34:35 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39494 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229586AbjAAIed (ORCPT ); Sun, 1 Jan 2023 03:34:33 -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 3C3136246 for ; Sun, 1 Jan 2023 00:34:32 -0800 (PST) Received: by mail-pj1-x1043.google.com with SMTP id o2so21444475pjh.4 for ; Sun, 01 Jan 2023 00:34:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=AhmoW+YWhQSTnzAriplvw885Int1P6BKhpLh33MyrW0=; b=q3c3Oy9CRDV44M/uwCnfZElSL4V+9OhsCsQCPtyGL+dJCA9qvSXV8XwxhXRIKdhJut CF7D8yXkDIr81eZXo03ZPWjcsB14JOWpuFzvm4XXlXdIh+2BiY0wGWe+tVH7dqCO/Ixc ppJij2/flb6DBGCxXsGtNKlMKVJ3nXrp+hMZrBoQZuXTbHGMe3So4Ef4qL/us7xCYZSQ JpUzMLi2FnA/nUISC9bTa47U8BbD51FhtTT/RPwofuTjpcge60mRx8tmF9DAAanDViXR IrCpJQiBz9hwLigM3wPLlTyfxO2acStgorEH9N63ZUaFJC9QuEvjHPeyjSdjfsZOJpzV WWTg== 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=AhmoW+YWhQSTnzAriplvw885Int1P6BKhpLh33MyrW0=; b=PBGCFJhkipxjSgMiWV3Jh+ncPIEs67UxBFOJ+GYjit3EL16++z1o/OvBY6a1t9asrL vxdOeSpItDpg+GxOwIvUHTNir6j32ffc0JyWj2NqHAwxlc4GbErLoL1PyXr/+H+B22vh KgLVchvlzVS4iG1GtbX+O4e277+0fo9NjOQBcMbYu6brZX67r/M6Ui6n0w9jHAZGjoC3 5MkBaw6sXrrTe5ZxC+8AwaQq6wb62gFpGuo+CFOZy64gjkEY4erp3THS66egoTEl9nvd +qBOtVwayScC8IEFiFvt+lBv2DISTiQv3YDh+ldMqx+ad5T4U5sEvRH6gMJgXJkF+tLN GQmA== X-Gm-Message-State: AFqh2kp4V6Jo4RrGChM425FQOAunTR2Bis5TN8I6BGQFnX5vD97SqIbz dZuhBpNJ5YuBBiQjXjoS22ATYiXW32hJOW2c X-Google-Smtp-Source: AMrXdXvbUMkThEY24D68Cps5WWhI98nDXwN8E9OC/Kt+2x/T8mvWabVUB7aLr9nwmlc5RxIZiluJzw== X-Received: by 2002:a05:6a20:c110:b0:a3:5a61:20ef with SMTP id bh16-20020a056a20c11000b000a35a6120efmr39171283pzb.61.1672562071783; Sun, 01 Jan 2023 00:34:31 -0800 (PST) Received: from localhost ([2405:201:6014:d8bc:6259:1a87:ebdd:30a7]) by smtp.gmail.com with ESMTPSA id w14-20020a62820e000000b005745635c5b5sm16285756pfd.183.2023.01.01.00.34.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Jan 2023 00:34:31 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet , Eduard Zingerman Subject: [PATCH bpf-next v1 7/8] selftests/bpf: Add dynptr partial slot overwrite tests Date: Sun, 1 Jan 2023 14:04:01 +0530 Message-Id: <20230101083403.332783-8-memxor@gmail.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230101083403.332783-1-memxor@gmail.com> References: <20230101083403.332783-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3165; i=memxor@gmail.com; h=from:subject; bh=eWmNvxGiF5Xaqm4y1ZtdaWfWN8WxTeCQ6gx8Y2rsDnk=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjsUV1/ixClzNpVpS5lkZgtI97gYut6AwoR6brsFg5 8MIWKjOJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY7FFdQAKCRBM4MiGSL8Ryu+TD/ 9eEE25fbu0nqt0g1gBmt1+uKkcNbWdf/CiJPCY1GYgXrt6Ds9l8B0a+BFNWo0xWJv72MB2IEPSIna0 4iA4h1OLIQpwlwAamY2BuUs6HXpTV4AQZMhdmMwNEfbhc/eZNv/2NP3lXaV769UBThWKYXOPJxqhLT guz7R86wsfM5/ZRCWTMQ2JvBVWlmZoohJAEANZmS13TZF50etQjfOCtzp3iQUU8Ks2g0lIzLRpKgLs OIrSyt2Scbix2divSyYc7sMpDFf4VACNbvlA+jZ+in+M5AieGJ3jPNmjYxMnF1SCsafKgiMLTYjfG4 /qpp6gI3YLzDYTUUszfzUb4xs8O5rgVbEON3qgA/zcSVcPIC2dwnfXJUTyJ0RbiwPm+DGXf537vfi4 qP1husrL8QmNrl/7UJFmbi065M7+F7+mNe2+/Juut9vR0SGK99B02wm8aMXopolTV/UAtaQ2O+wXcS jISVDWXyoSSsH1dSlz2hUCac0gONlkAgcJj4D1evPexakfPm0DWPDJ/Zv1mfjuaFu9mLZPekehBsUE g/jepnJzs+7nHUQ7vMIVdom37OSRMa4xfKYv/gLV2QuHUMucyJcsr6Z3a0BG5JiHInqL4GjfowZo5q 0ejH91BrO5H/T1lVtTZzNr/WwLpyONxxAV94SfYNMgOhe6mgScxui64TruQw== 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 --- tools/testing/selftests/bpf/verifier/dynptr.c | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tools/testing/selftests/bpf/verifier/dynptr.c b/tools/testing/selftests/bpf/verifier/dynptr.c index 1aa7241e8a9e..8c57bc9e409f 100644 --- a/tools/testing/selftests/bpf/verifier/dynptr.c +++ b/tools/testing/selftests/bpf/verifier/dynptr.c @@ -122,3 +122,61 @@ .result = REJECT, .errstr = "dynptr has to be at the constant offset", }, +{ + "dynptr: partial dynptr slot invalidate", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_LD_MAP_FD(BPF_REG_6, 0), + BPF_LD_MAP_FD(BPF_REG_7, 0), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_7), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0), + BPF_MOV64_REG(BPF_REG_3, BPF_REG_2), + BPF_MOV64_IMM(BPF_REG_4, 0), + BPF_MOV64_REG(BPF_REG_8, BPF_REG_2), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_update_elem), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_7), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_8), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem), + BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1), + BPF_EXIT_INSN(), + BPF_MOV64_REG(BPF_REG_7, BPF_REG_0), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_6), + BPF_MOV64_IMM(BPF_REG_2, 8), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -24), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_reserve_dynptr), + BPF_ST_MEM(BPF_DW, BPF_REG_10, -16, 0), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_7), + BPF_MOV64_IMM(BPF_REG_2, 8), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -16), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_dynptr_from_mem), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -512), + BPF_MOV64_IMM(BPF_REG_2, 488), + BPF_MOV64_REG(BPF_REG_3, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, -24), + BPF_MOV64_IMM(BPF_REG_4, 0), + BPF_MOV64_IMM(BPF_REG_5, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_dynptr_read), + BPF_MOV64_IMM(BPF_REG_8, 1), + BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1), + BPF_MOV64_IMM(BPF_REG_8, 0), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -24), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_discard_dynptr), + BPF_MOV64_REG(BPF_REG_0, BPF_REG_8), + BPF_EXIT_INSN(), + }, + .fixup_map_ringbuf = { 1 }, + .fixup_map_hash_8b = { 3 }, + .result_unpriv = REJECT, + .errstr_unpriv = "unknown func bpf_ringbuf_reserve_dynptr#198", + .result = REJECT, + .errstr = "Expected an initialized dynptr as arg #3", +}, From patchwork Sun Jan 1 08:34:02 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: 13086340 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 76D58C3DA7D for ; Sun, 1 Jan 2023 08:34:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229586AbjAAIeh (ORCPT ); Sun, 1 Jan 2023 03:34:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39530 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229631AbjAAIeg (ORCPT ); Sun, 1 Jan 2023 03:34:36 -0500 Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 043992AC5 for ; Sun, 1 Jan 2023 00:34:36 -0800 (PST) Received: by mail-pl1-x641.google.com with SMTP id d9so9567694pll.9 for ; Sun, 01 Jan 2023 00:34:35 -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=P+mjdVq/lm8o2u1+aLRQmVRqK+3LLxx6Bpmv9V0f4BU=; b=jk70oeVTZjk5L2lF6nDjRCl1vJNgLHeIHQM+zTFmVHSZFK9T67R/QMQvwBaOtdM75M J9Euflo94zXqeCqfu7vcGqW8EDYfzpVOSAUzQsgJHO9vpkqrg455mfggsacjx+ZrrUiE HUKpiqKAZdauuvIRTpMjrmp7mZLFo6MEFMjYlm3NsM4UUQL16D/ziSWchZ+txU8wgRAb H1Wq8CRVLZPSRjdRVQt91tTc8iB5sdUYDQB15N653Pi9hYMlI3ddX/ZTfOSW5Rp+hju3 JWq6GYWHqZe9YQuHSwRQWjwUJ7JJGue2vSeWRx4R9utVN2aTTW4ZcccSTWgE0IfEACoW eenQ== 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=P+mjdVq/lm8o2u1+aLRQmVRqK+3LLxx6Bpmv9V0f4BU=; b=JTWJwRcWCAA8yZdAdYePXP6ZuJzImS9rnLN6XvBqBqWocXRpjHF+qQGrjdaDWr4l2M cCOqeMJoxqu0gJoGiAliJFb9lO5DuoNGpxlP4w7Fz8RW6pE8sdBjNSoav+/R9Kc0E6Ok vPv9/l7G1ButpnQg7jtRauzmONT6U+T6HRIu4bBcEnkQnvlUWLqxfpXFACUsGeuAmsjn IFUx8qgl6GLGBv9TqCe0MaO+ktohIbjz+MUG0eYP8kfKEEm2Rx6esGJWAJlypSGqcGr2 9MF5WwQdNSJ1xSbiQc95fNEiAMugrsn2quqdpwJuoQkru1O9NV9II/nCL0tFNn9WASgP 3WXA== X-Gm-Message-State: AFqh2kpc5S1UV1+N+d8CJBBVXzPZm82S128ar0Yu1a+y2GfXazH696lt KGyXGqrBWHPPtN23SVKyIZR8JqG/306nB+f2 X-Google-Smtp-Source: AMrXdXv17V9mL++853ZZoja00kL5FxMBarCRVfC6A5fSQRcqlNnr3X7k5Eh5Lmc6L1Zc1Le/uLaP/Q== X-Received: by 2002:a17:902:7582:b0:192:4d6b:2311 with SMTP id j2-20020a170902758200b001924d6b2311mr38077442pll.46.1672562075316; Sun, 01 Jan 2023 00:34:35 -0800 (PST) Received: from localhost ([2405:201:6014:d8bc:6259:1a87:ebdd:30a7]) by smtp.gmail.com with ESMTPSA id d19-20020a170902f15300b00189fdadef9csm17934185plb.107.2023.01.01.00.34.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 01 Jan 2023 00:34:34 -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 v1 8/8] selftests/bpf: Add dynptr helper tests Date: Sun, 1 Jan 2023 14:04:02 +0530 Message-Id: <20230101083403.332783-9-memxor@gmail.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230101083403.332783-1-memxor@gmail.com> References: <20230101083403.332783-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2023; i=memxor@gmail.com; h=from:subject; bh=HbuEZDUs+WYocuqEBo6afen2xIUnVrhRd4cXTonJBa0=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjsUV1jkqMm9yl9KI8SazkU2R1Oan0Znmjaw6Ldc58 TWsVq4yJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY7FFdQAKCRBM4MiGSL8Ryr0cEA C3h6EGnHwpGhZi8ZB/xnacauXOnCuhm6qngfB5JtpiVQsXKeWKS191SmutrvUjGfeAZyFEFoTaKeIc ASWjmGytSPLxBiRlPYClFpR4xphJLtupoeX85W927+1D+SVtlPYQz8f4mwOslk+W8LYqrVifagNJbk upU4fjvnHf0rmHqYmu6PEZg1e+bCDpQe0RT8DhGcBLN9dpVHcFVDEAT3lr2vSArDRJ9LeQrlVM7CHO MywdT/7PguTk6KdqKOeCd6iDK/x2FXuMuym8uxpwHxToXWILMetYJr+CwWjplLfSSeqp9mfS5XXGmw rx1dFkJ89RR2d8x02IKW6WLjGF9PQvh5TwM2qbWMW7o+hESBD/X7Cb2imh47JyVSsKkFCKJCswCI92 1VmkXZzM8aHVJFewSjH0n4zxQZD1ZpKrfDQ22wwK5+4JvV8vOvovoIne4K8S9h6Wmh22WcR4grka4+ CQaRrEDwpdaIBDK5VUkRJC8hE7NzOGTJjzxNXARkMi0CQdNdIm2Ug59yKVAgBQKvnWZiyJJQFzSiSs 8ZOL2FXV+3Hejxckc9MK7XM+t+c6A+EvkooTmvijd0amhDpzhuEZksE75PIc/diuIQJF/iYcOlElYB +zPvOf01lR60wC2EyNFxpTIooJwpsiBPaap38bSDoghFH1+GZ5YJheu0XLZw== 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. 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 | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index 32df3647b794..73ae93dedaba 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -653,3 +653,65 @@ int dynptr_from_mem_invalid_api(void *ctx) return 0; } + +SEC("?raw_tp") +__success +int dynptr_overwrite_unref(void *ctx) +{ + struct bpf_dynptr ptr; + + get_map_val_dynptr(&ptr); + get_map_val_dynptr(&ptr); + get_map_val_dynptr(&ptr); + + return 0; +} + +SEC("?raw_tp") +__failure __msg("Unreleased reference") +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; +}