From patchwork Tue Oct 18 13:59:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010560 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 E2191C433FE for ; Tue, 18 Oct 2022 13:59:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231304AbiJRN7i (ORCPT ); Tue, 18 Oct 2022 09:59:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48424 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231327AbiJRN7g (ORCPT ); Tue, 18 Oct 2022 09:59:36 -0400 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 CAAC7BF4A for ; Tue, 18 Oct 2022 06:59:33 -0700 (PDT) Received: by mail-pj1-x1044.google.com with SMTP id x1-20020a17090ab00100b001fda21bbc90so17428919pjq.3 for ; Tue, 18 Oct 2022 06:59:33 -0700 (PDT) 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=wCy36PN3wJ1LB153aSf6Ng+oi283CCJUdsT0Es6SCzo=; b=ZrFsTvRd5SwJFEjV0t0qMx0eI303YJmwcFcR3NisDqm3MlJ9xXSB2+x4OIW4pT+TuV Ukv6kbCJmptBcEm1sZd0otZYBapAYNqhMOfoNnmaAxbNi/DjZ73nuRCaguSLEoH/SvwX 6n3UCQ4JGzBMBS5J2BnmLFTfnTTjXmQ8C9dtQebD6WsTjRlAs890Rhp9/nChKhp6OBJ3 fmMKXGgk7cfebi9FQIvS/c4pa7F0VLB0cOQtBAoE3P+xi26mmv+CIamohGPlFkqOyYnr g3tNA7rSvXD8eh/MvQlbAnSiTHEur/7M/2Dq1qgxe31+P0UnURnhmBE9miT7jR0+h4wJ DdfQ== 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=wCy36PN3wJ1LB153aSf6Ng+oi283CCJUdsT0Es6SCzo=; b=jf4gTTLJdcQ67nH1OjauyKPb6/Clf8Kbz+OrX6MpA1Er9IHyGT5rNtfWbx9rUjMaRJ x6BldxiGbUXC2oVNJEbeOiDQ9UQ+Ve+cC0f5E4oQ3GFV8JfS4ncyTkmv9WVLtV25zail IxQbVa4IrPoSDtoiToGiLqPC5rtYn2L7fEJn5/eaiqDV4B0ZhLwgLzyfGIN9jXfuCVv7 mFfu+2GcOhOpcxLhLVuisV69LK0PE0p6MOXbCuyfXvzhGmqYtT9dIVaUcDidY5XDJi5U bd4RN5XrSETSgOWDUcJ0cX7BWJvEiyTwsaBXaJU+2ot4NCIk6RPzeWLPPcwjWJS4jVEP m1QQ== X-Gm-Message-State: ACrzQf3Sb3ly52X7g//GZ7R5IA86r4qb77WsEpdn+6rKXWH5C6H+pdGB NEbPMB5uJus+YqpY+OpL0/vIzAC4sesUqw== X-Google-Smtp-Source: AMsMyM5mIggOs5YGzln41VTc6IIse0SwzpY60ofFsbZ73zqjBteJu24zvgPEc2hjXAnDViEWfBArKg== X-Received: by 2002:a17:90a:c258:b0:20b:23d5:8eb2 with SMTP id d24-20020a17090ac25800b0020b23d58eb2mr3666601pjx.85.1666101572705; Tue, 18 Oct 2022 06:59:32 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id o3-20020a17090a678300b001f94d25bfabsm11694076pjj.28.2022.10.18.06.59.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 06:59:32 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 01/13] bpf: Refactor ARG_PTR_TO_DYNPTR checks into process_dynptr_func Date: Tue, 18 Oct 2022 19:29:08 +0530 Message-Id: <20221018135920.726360-2-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9595; i=memxor@gmail.com; h=from:subject; bh=BHwXyoAI/Z+OBPhsYJkkO3GcDz+jXjL23gceoL8/+D0=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEig35tYAR1x2XKHvJuVSQjh+eu5P+WzupQPP03 JnNeWP6JAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8RymZ9EA CiyH+fjeAz3S+zNA8x/zOUxWqlbffHQy+60VFJQbB6usoueBnPwu1AOzd3mHU/U9jF3TNUUGEILDQ9 oYhph7ZrRN2WGdBQLbOb8zmibqk35XaTO3muDlMMPLVFDVpIKkE+dxdYEZMjSp7ems81XNsJCiu4+W fpXovKCjblUTdcXu4fX9zxvJ2dXVFSMDp8VTDeRN3Dy9hhYRtWn5DBePCT/Iv/7OQbNFeEGThqgE71 tJPHBnw2mZmTbzBIBVrC+t07/fj8O4GWYWps8j9MsSwAXIq3oCw126179WtTNnUS6HaZEM+U3Xki1P jJBwcO1+ZzM9UVPiHpxK0wuqWwKoq+h/KThTuCuVbyD+t8KFDdBFl3DDa0AgoAbKKGH6oOR7YG5jEt eKR6pox3zEmL/8MZo32xxSAeoXsMwrtRgkoqLKmWqxOygHBiSmAKaj0naLV57Pvhf/m5bITCxGmlar pZJHZ7+3k4rccNJL6k5FpuSnewLo22NKZFNKff8ldg1XmqT2JHoT+WeAsPvKFdTG4sSxWK2Ubhx3rV qr1UxgvVAtSjLMUjDPRlPOGK7NrQFMjwnUX6T0g/n+pfRaeqJKdnTXZpE4vmpOq5DRCBXcDRD4iUXs AajT9y0xwo+Ld7Q63VVTVQNsJ3Q2xZPiqKqP6tcI1BM47QSLX8hYOYIoNsew== 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 ARG_PTR_TO_DYNPTR is akin to ARG_PTR_TO_TIMER, ARG_PTR_TO_KPTR, where the underlying register type is subjected to more special checks to determine the type of object represented by the pointer and its state consistency. Move dynptr checks to their own 'process_dynptr_func' function so that is consistent and in-line with existing code. This also makes it easier to reuse this code for kfunc handling. To this end, remove the dependency on bpf_call_arg_meta parameter by instead taking the uninit_dynptr_regno by pointer. This is only needed to be set to a valid pointer when arg_type has MEM_UNINIT. Then, reuse this consolidated function in kfunc dynptr handling too. Note that for kfuncs, the arg_type constraint of DYNPTR_TYPE_LOCAL has been lifted. Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: David Vernet --- include/linux/bpf_verifier.h | 8 +- kernel/bpf/btf.c | 17 +-- kernel/bpf/verifier.c | 115 ++++++++++-------- .../bpf/prog_tests/kfunc_dynptr_param.c | 5 +- .../bpf/progs/test_kfunc_dynptr_param.c | 12 -- 5 files changed, 69 insertions(+), 88 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 9e1e6965f407..a33683e0618b 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -593,11 +593,9 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state u32 regno); int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, u32 regno, u32 mem_size); -bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, - struct bpf_reg_state *reg); -bool is_dynptr_type_expected(struct bpf_verifier_env *env, - struct bpf_reg_state *reg, - enum bpf_arg_type arg_type); +int process_dynptr_func(struct bpf_verifier_env *env, int regno, + enum bpf_arg_type arg_type, int argno, + u8 *uninit_dynptr_regno); /* this lives here instead of in bpf.h because it needs to dereference tgt_prog */ static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog, diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index eba603cec2c5..1827d889e08a 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6486,23 +6486,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, return -EINVAL; } - if (!is_dynptr_reg_valid_init(env, reg)) { - bpf_log(log, - "arg#%d pointer type %s %s must be valid and initialized\n", - i, btf_type_str(ref_t), - ref_tname); + if (process_dynptr_func(env, regno, ARG_PTR_TO_DYNPTR, i, NULL)) return -EINVAL; - } - - if (!is_dynptr_type_expected(env, reg, - ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL)) { - bpf_log(log, - "arg#%d pointer type %s %s points to unsupported dynamic pointer type\n", - i, btf_type_str(ref_t), - ref_tname); - return -EINVAL; - } - continue; } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 6f6d2d511c06..31c0c999448e 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -782,8 +782,7 @@ static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_ return true; } -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) { struct bpf_func_state *state = func(env, reg); int spi = get_spi(reg->off); @@ -802,9 +801,8 @@ bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, return true; } -bool is_dynptr_type_expected(struct bpf_verifier_env *env, - struct bpf_reg_state *reg, - enum bpf_arg_type arg_type) +static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg_state *reg, + enum bpf_arg_type arg_type) { struct bpf_func_state *state = func(env, reg); enum bpf_dynptr_type dynptr_type; @@ -5573,6 +5571,65 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno, return 0; } +int process_dynptr_func(struct bpf_verifier_env *env, int regno, + enum bpf_arg_type arg_type, int argno, + u8 *uninit_dynptr_regno) +{ + struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; + + /* We only need to check for initialized / uninitialized helper + * dynptr args if the dynptr is not PTR_TO_DYNPTR, as the + * assumption is that if it is, that a helper function + * initialized the dynptr on behalf of the BPF program. + */ + if (base_type(reg->type) == PTR_TO_DYNPTR) + return 0; + if (arg_type & MEM_UNINIT) { + if (!is_dynptr_reg_valid_uninit(env, reg)) { + verbose(env, "Dynptr has to be an uninitialized dynptr\n"); + return -EINVAL; + } + + /* We only support one dynptr being uninitialized at the moment, + * which is sufficient for the helper functions we have right now. + */ + if (*uninit_dynptr_regno) { + verbose(env, "verifier internal error: multiple uninitialized dynptr args\n"); + return -EFAULT; + } + + *uninit_dynptr_regno = regno; + } else { + if (!is_dynptr_reg_valid_init(env, reg)) { + verbose(env, + "Expected an initialized dynptr as arg #%d\n", + argno + 1); + return -EINVAL; + } + + if (!is_dynptr_type_expected(env, reg, arg_type)) { + const char *err_extra = ""; + + switch (arg_type & DYNPTR_TYPE_FLAG_MASK) { + case DYNPTR_TYPE_LOCAL: + err_extra = "local"; + break; + case DYNPTR_TYPE_RINGBUF: + err_extra = "ringbuf"; + break; + default: + err_extra = ""; + break; + } + verbose(env, + "Expected a dynptr of type %s as arg #%d\n", + err_extra, argno + 1); + return -EINVAL; + } + } + return 0; +} + static bool arg_type_is_mem_size(enum bpf_arg_type type) { return type == ARG_CONST_SIZE || @@ -6086,52 +6143,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, err = check_mem_size_reg(env, reg, regno, true, meta); break; case ARG_PTR_TO_DYNPTR: - /* We only need to check for initialized / uninitialized helper - * dynptr args if the dynptr is not PTR_TO_DYNPTR, as the - * assumption is that if it is, that a helper function - * initialized the dynptr on behalf of the BPF program. - */ - if (base_type(reg->type) == PTR_TO_DYNPTR) - break; - if (arg_type & MEM_UNINIT) { - if (!is_dynptr_reg_valid_uninit(env, reg)) { - verbose(env, "Dynptr has to be an uninitialized dynptr\n"); - return -EINVAL; - } - - /* We only support one dynptr being uninitialized at the moment, - * which is sufficient for the helper functions we have right now. - */ - if (meta->uninit_dynptr_regno) { - verbose(env, "verifier internal error: multiple uninitialized dynptr args\n"); - return -EFAULT; - } - - meta->uninit_dynptr_regno = regno; - } else if (!is_dynptr_reg_valid_init(env, reg)) { - verbose(env, - "Expected an initialized dynptr as arg #%d\n", - arg + 1); - return -EINVAL; - } else if (!is_dynptr_type_expected(env, reg, arg_type)) { - const char *err_extra = ""; - - switch (arg_type & DYNPTR_TYPE_FLAG_MASK) { - case DYNPTR_TYPE_LOCAL: - err_extra = "local"; - break; - case DYNPTR_TYPE_RINGBUF: - err_extra = "ringbuf"; - break; - default: - err_extra = ""; - break; - } - verbose(env, - "Expected a dynptr of type %s as arg #%d\n", - err_extra, arg + 1); - return -EINVAL; - } + if (process_dynptr_func(env, regno, arg_type, arg, &meta->uninit_dynptr_regno)) + return -EACCES; break; case ARG_CONST_ALLOC_SIZE_OR_ZERO: if (!tnum_is_const(reg->var_off)) { 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 c210657d4d0a..fc562e863e79 100644 --- a/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c @@ -18,10 +18,7 @@ static struct { const char *expected_verifier_err_msg; int expected_runtime_err; } kfunc_dynptr_tests[] = { - {"dynptr_type_not_supp", - "arg#0 pointer type STRUCT bpf_dynptr_kern points to unsupported dynamic pointer type", 0}, - {"not_valid_dynptr", - "arg#0 pointer type STRUCT bpf_dynptr_kern must be valid and initialized", 0}, + {"not_valid_dynptr", "Expected an initialized dynptr as arg #1", 0}, {"not_ptr_to_stack", "arg#0 pointer type STRUCT bpf_dynptr_kern not to stack", 0}, {"dynptr_data_null", NULL, -EBADMSG}, }; diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c index ce39d096bba3..f4a8250329b2 100644 --- a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c +++ b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c @@ -32,18 +32,6 @@ int err, pid; char _license[] SEC("license") = "GPL"; -SEC("?lsm.s/bpf") -int BPF_PROG(dynptr_type_not_supp, int cmd, union bpf_attr *attr, - unsigned int size) -{ - char write_data[64] = "hello there, world!!"; - struct bpf_dynptr ptr; - - bpf_ringbuf_reserve_dynptr(&ringbuf, sizeof(write_data), 0, &ptr); - - return bpf_verify_pkcs7_signature(&ptr, &ptr, NULL); -} - SEC("?lsm.s/bpf") int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size) { From patchwork Tue Oct 18 13:59:09 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010561 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 6CD8DC4332F for ; Tue, 18 Oct 2022 13:59:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231283AbiJRN7m (ORCPT ); Tue, 18 Oct 2022 09:59:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48778 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231337AbiJRN7j (ORCPT ); Tue, 18 Oct 2022 09:59:39 -0400 Received: from mail-pg1-x542.google.com (mail-pg1-x542.google.com [IPv6:2607:f8b0:4864:20::542]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EA62B6276 for ; Tue, 18 Oct 2022 06:59:36 -0700 (PDT) Received: by mail-pg1-x542.google.com with SMTP id b5so13384499pgb.6 for ; Tue, 18 Oct 2022 06:59:36 -0700 (PDT) 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=E2PHPXhpAuJEPOETNHp23e+7fBIW1mr8BiqDC16XPmw=; b=iyUNhhzTFw9mORBjMVNQoi2Xz9aF0GLljmSP/1I3Mfu1UbVx6roAA1sCnKnSCCqw5K fqSCC3j6wPCzPa6DR3BAfpTqoUYzWCAfg95qz3A4Uq5LRNkaK1gGmYkwuPgQWkk30dJN HFR11OZXmrFiMgy1c645z63KtaIWUBwWk6IHn12d2vXgxMToXCffrI7oj2uKi4d1F+zq fPp7fpzStzKbJlkn/eWcqgjPH23m50o77mKudvRTbobHty/5vg4+Hu/yT1l7itg5E+nM Fo+dtInQfPyINLk5rJooEYYBGI6w7GXVgKuyR5uZvLN2HOJqtB/q3oV9W+qSRTvkSsyH 66Ag== 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=E2PHPXhpAuJEPOETNHp23e+7fBIW1mr8BiqDC16XPmw=; b=149yav7b2x+5tfZfiwzurO3u3m22xegE1vj0DShbNHaob5oIwLG/adKm8I5nqLr7Q8 CroCSpszhAOlYzvrZiJP+haS9I+xR9CMDEu25p41ocgBgbrqPdwbTfQN7zL/D3JDleSP K5qtV54Hs8JUR5jUmGNA/d+NnTWgFeYCzYR7tgNSyP2BDpyp//SEO4mCMTZUa9X9/cjq 9HrSbrImWIVDwqwXtadyodd8i6hSR6BJDOsUF/BlBx1eE/R5avWsF54roLcsgP7fx7Fh dgqUzVNKwN50SblLiNMRDb5jd5IBu3Tue3oaFKBkDQaRg4Ke3Oh9fyW7MvtK+9yNrAUG PLvg== X-Gm-Message-State: ACrzQf1rLk7uU+JTSKmLAoX6DFIs0zgW6jrpmLckI6NBdJsFxNYoBwl9 BkDMXZQpzUUy616f+UH1clLF/qA7YJQYnA== X-Google-Smtp-Source: AMsMyM5JNaHiqQXeJxdYPC1YkDl24jEdurrWgDTIcu0+Aql4AS++fz3My6k9Ndn6uwa/3mykqdiz9Q== X-Received: by 2002:a63:464d:0:b0:441:5968:cd0e with SMTP id v13-20020a63464d000000b004415968cd0emr2827884pgk.595.1666101575910; Tue, 18 Oct 2022 06:59:35 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id g19-20020aa796b3000000b00528a097aeffsm9289819pfk.118.2022.10.18.06.59.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 06:59:35 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 02/13] bpf: Rework process_dynptr_func Date: Tue, 18 Oct 2022 19:29:09 +0530 Message-Id: <20221018135920.726360-3-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=27742; i=memxor@gmail.com; h=from:subject; bh=+YQU9nIozIV1W0Qg3WMdBYzp7Eip+Fp7mUm5J+auNZM=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEiqtfNZAX77LoMYo5YYfrFEyS8zZaqgs3MaNdf xYGEZjWJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8RyuNXD/ 0SMt3J5g37rW8FhcWTJxDHISLUKfNaR+XmWrPHNdef0zztx94/DzccjuCrAO9xczlSs2ACXmodKUGM VKsM1eAKHqFH6YeV+CCgvxDL92KK2BxWLVCNci5voBpRcZ1W0FqlHKjvXex7SKcFgOZBr6JNs9bPn9 qFdEedo+LUtfs8rdiOZa5f+dPm7xdpP+bhVO4R/w8tria+BRWjppAHFH+IiWZ4A2lTNp+Q+T59q6ru s3A3FfMtZHgzuEI9TRs1LPPPbpLiBUvThBTsqWStc0IyG5nAlIkLcO4LowzDOoaC6JMRZajf7MwhYi ISFS5oJKFQRE+FJOplG6QPv8XpzmfRglY5zKoHjTn4lZx6UpS719hvi7K078J2gtiWDZQesXkFz1wg 6ZRVBTi82cw0fDXvbwRS60Zne9AceXHgC02xv9QxUW1QyA92+gC1hfUnx7nqWAlYTZdslX0c3TWiXW 7lo8KZwX3a7J9NbgCfnftGxZfG6kfUfVIFqpAIQ68sCQSwWpMK/WxU8uI+GA4bg2TpFXFVP4hH8vCC P1AWvbl6G1+r6U744DoB2pnmzvmDNT3CwpEeBZ8cuOV3GADH4V/pq78ZAfLOeZyH8OZwHVFlVNlMO0 pLjlMMEGwRbq8zJ5FLyDPJm/YKy4g0IFUfmCcq83Bk37Nuipru+6NZ3X66rA== 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 Recently, user ringbuf support introduced a PTR_TO_DYNPTR register type for use in callback state, because in case of user ringbuf helpers, there is no dynptr on the stack that is passed into the callback. To reflect such a state, a special register type was created. However, some checks have been bypassed incorrectly during the addition of this feature. First, for arg_type with MEM_UNINIT flag which initialize a dynptr, they must be rejected for such register type. Secondly, in the future, there are plans to dynptr helpers that operate on the dynptr itself and may change its offset and other properties. In all of these cases, PTR_TO_DYNPTR shouldn't be allowed to be passed to such helpers, however the current code simply returns 0. The rejection for helpers that release the dynptr is already handled. For fixing this, we take a step back and rework existing code in a way that will allow fitting in all classes of helpers and have a coherent model for dealing with the variety of use cases in which dynptr is used. First, for ARG_PTR_TO_DYNPTR, it can either be set alone or together with a DYNPTR_TYPE_* constant that denotes the only type it accepts. Next, helpers which initialize a dynptr use MEM_UNINIT to indicate this fact. To make the distinction clear, use MEM_RDONLY flag to indicate that the helper only operates on the memory pointed to by the dynptr, not the dynptr itself. In C parlance, it would be equivalent to taking the dynptr as a point to const argument. When either of these flags are not present, the helper is allowed to mutate both the dynptr itself and also the memory it points to. Currently, the read only status of the memory is not tracked in the dynptr, but it would be trivial to add this support inside dynptr state of the register. With these changes and renaming PTR_TO_DYNPTR to CONST_PTR_TO_DYNPTR to better reflect its usage, it can no longer be passed to helpers that initialize a dynptr, i.e. bpf_dynptr_from_mem, bpf_ringbuf_reserve_dynptr. A note to reviewers is that in code that does mark_stack_slots_dynptr, and unmark_stack_slots_dynptr, we implicitly rely on the fact that PTR_TO_STACK reg is the only case that can reach that code path, as one cannot pass CONST_PTR_TO_DYNPTR to helpers that don't set MEM_RDONLY. In both cases such helpers won't be setting that flag. The next patch will add a couple of selftest cases to make sure this doesn't break. Fixes: 205715673844 ("bpf: Add bpf_user_ringbuf_drain() helper") Signed-off-by: Kumar Kartikeya Dwivedi --- include/linux/bpf.h | 4 +- include/uapi/linux/bpf.h | 8 +- kernel/bpf/btf.c | 7 +- kernel/bpf/helpers.c | 18 +- kernel/bpf/verifier.c | 203 ++++++++++++++---- scripts/bpf_doc.py | 1 + tools/include/uapi/linux/bpf.h | 8 +- .../selftests/bpf/prog_tests/user_ringbuf.c | 10 +- 8 files changed, 185 insertions(+), 74 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 9e7d46d16032..13c6ff2de540 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -656,7 +656,7 @@ enum bpf_reg_type { PTR_TO_MEM, /* reg points to valid memory region */ PTR_TO_BUF, /* reg points to a read/write buffer */ PTR_TO_FUNC, /* reg points to a bpf program function */ - PTR_TO_DYNPTR, /* reg points to a dynptr */ + CONST_PTR_TO_DYNPTR, /* reg points to a const struct bpf_dynptr */ __BPF_REG_TYPE_MAX, /* Extended reg_types. */ @@ -2689,7 +2689,7 @@ void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data, enum bpf_dynptr_type type, u32 offset, u32 size); void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr); int bpf_dynptr_check_size(u32 size); -u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr); +u32 bpf_dynptr_get_size(const struct bpf_dynptr_kern *ptr); #ifdef CONFIG_BPF_LSM void bpf_cgroup_atype_get(u32 attach_btf_id, int cgroup_atype); diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 17f61338f8f8..2b490bde85a6 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -5282,7 +5282,7 @@ union bpf_attr { * Return * Nothing. Always succeeds. * - * long bpf_dynptr_read(void *dst, u32 len, struct bpf_dynptr *src, u32 offset, u64 flags) + * long bpf_dynptr_read(void *dst, u32 len, const struct bpf_dynptr *src, u32 offset, u64 flags) * Description * Read *len* bytes from *src* into *dst*, starting from *offset* * into *src*. @@ -5292,7 +5292,7 @@ union bpf_attr { * of *src*'s data, -EINVAL if *src* is an invalid dynptr or if * *flags* is not 0. * - * long bpf_dynptr_write(struct bpf_dynptr *dst, u32 offset, void *src, u32 len, u64 flags) + * long bpf_dynptr_write(const struct bpf_dynptr *dst, u32 offset, void *src, u32 len, u64 flags) * Description * Write *len* bytes from *src* into *dst*, starting from *offset* * into *dst*. @@ -5302,7 +5302,7 @@ union bpf_attr { * of *dst*'s data, -EINVAL if *dst* is an invalid dynptr or if *dst* * is a read-only dynptr or if *flags* is not 0. * - * void *bpf_dynptr_data(struct bpf_dynptr *ptr, u32 offset, u32 len) + * void *bpf_dynptr_data(const struct bpf_dynptr *ptr, u32 offset, u32 len) * Description * Get a pointer to the underlying dynptr data. * @@ -5403,7 +5403,7 @@ union bpf_attr { * Drain samples from the specified user ring buffer, and invoke * the provided callback for each such sample: * - * long (\*callback_fn)(struct bpf_dynptr \*dynptr, void \*ctx); + * long (\*callback_fn)(const struct bpf_dynptr \*dynptr, void \*ctx); * * If **callback_fn** returns 0, the helper will continue to try * and drain the next sample, up to a maximum of diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 1827d889e08a..b6cd91c23a27 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6479,14 +6479,15 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, } if (arg_dynptr) { - if (reg->type != PTR_TO_STACK) { - bpf_log(log, "arg#%d pointer type %s %s not to stack\n", + if (reg->type != PTR_TO_STACK && + reg->type != CONST_PTR_TO_DYNPTR) { + bpf_log(log, "arg#%d pointer type %s %s not to stack or dynptr\n", i, btf_type_str(ref_t), ref_tname); return -EINVAL; } - if (process_dynptr_func(env, regno, ARG_PTR_TO_DYNPTR, i, NULL)) + if (process_dynptr_func(env, regno, ARG_PTR_TO_DYNPTR | MEM_RDONLY, i, NULL)) return -EINVAL; continue; } diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index a6b04faed282..0a4017eb3616 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1398,7 +1398,7 @@ static const struct bpf_func_proto bpf_kptr_xchg_proto = { #define DYNPTR_SIZE_MASK 0xFFFFFF #define DYNPTR_RDONLY_BIT BIT(31) -static bool bpf_dynptr_is_rdonly(struct bpf_dynptr_kern *ptr) +static bool bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr) { return ptr->size & DYNPTR_RDONLY_BIT; } @@ -1408,7 +1408,7 @@ static void bpf_dynptr_set_type(struct bpf_dynptr_kern *ptr, enum bpf_dynptr_typ ptr->size |= type << DYNPTR_TYPE_SHIFT; } -u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr) +u32 bpf_dynptr_get_size(const struct bpf_dynptr_kern *ptr) { return ptr->size & DYNPTR_SIZE_MASK; } @@ -1432,7 +1432,7 @@ void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr) memset(ptr, 0, sizeof(*ptr)); } -static int bpf_dynptr_check_off_len(struct bpf_dynptr_kern *ptr, u32 offset, u32 len) +static int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr, u32 offset, u32 len) { u32 size = bpf_dynptr_get_size(ptr); @@ -1477,7 +1477,7 @@ static const struct bpf_func_proto bpf_dynptr_from_mem_proto = { .arg4_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL | MEM_UNINIT, }; -BPF_CALL_5(bpf_dynptr_read, void *, dst, u32, len, struct bpf_dynptr_kern *, src, +BPF_CALL_5(bpf_dynptr_read, void *, dst, u32, len, const struct bpf_dynptr_kern *, src, u32, offset, u64, flags) { int err; @@ -1500,12 +1500,12 @@ static const struct bpf_func_proto bpf_dynptr_read_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_UNINIT_MEM, .arg2_type = ARG_CONST_SIZE_OR_ZERO, - .arg3_type = ARG_PTR_TO_DYNPTR, + .arg3_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, }; -BPF_CALL_5(bpf_dynptr_write, struct bpf_dynptr_kern *, dst, u32, offset, void *, src, +BPF_CALL_5(bpf_dynptr_write, const struct bpf_dynptr_kern *, dst, u32, offset, void *, src, u32, len, u64, flags) { int err; @@ -1526,14 +1526,14 @@ static const struct bpf_func_proto bpf_dynptr_write_proto = { .func = bpf_dynptr_write, .gpl_only = false, .ret_type = RET_INTEGER, - .arg1_type = ARG_PTR_TO_DYNPTR, + .arg1_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY, .arg2_type = ARG_ANYTHING, .arg3_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg4_type = ARG_CONST_SIZE_OR_ZERO, .arg5_type = ARG_ANYTHING, }; -BPF_CALL_3(bpf_dynptr_data, struct bpf_dynptr_kern *, ptr, u32, offset, u32, len) +BPF_CALL_3(bpf_dynptr_data, const struct bpf_dynptr_kern *, ptr, u32, offset, u32, len) { int err; @@ -1554,7 +1554,7 @@ static const struct bpf_func_proto bpf_dynptr_data_proto = { .func = bpf_dynptr_data, .gpl_only = false, .ret_type = RET_PTR_TO_DYNPTR_MEM_OR_NULL, - .arg1_type = ARG_PTR_TO_DYNPTR, + .arg1_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY, .arg2_type = ARG_ANYTHING, .arg3_type = ARG_CONST_ALLOC_SIZE_OR_ZERO, }; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 31c0c999448e..87d9cccd1623 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -563,7 +563,7 @@ static const char *reg_type_str(struct bpf_verifier_env *env, [PTR_TO_BUF] = "buf", [PTR_TO_FUNC] = "func", [PTR_TO_MAP_KEY] = "map_key", - [PTR_TO_DYNPTR] = "dynptr_ptr", + [CONST_PTR_TO_DYNPTR] = "dynptr", }; if (type & PTR_MAYBE_NULL) { @@ -697,6 +697,27 @@ static bool dynptr_type_refcounted(enum bpf_dynptr_type type) return type == BPF_DYNPTR_TYPE_RINGBUF; } +static void __mark_dynptr_regs(struct bpf_reg_state *reg1, + struct bpf_reg_state *reg2, + enum bpf_dynptr_type type); + +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, + struct bpf_reg_state *sreg2, + enum bpf_dynptr_type type) +{ + __mark_dynptr_regs(sreg1, sreg2, type); +} + +static void mark_dynptr_cb_reg(struct bpf_reg_state *reg1, + enum bpf_dynptr_type type) +{ + __mark_dynptr_regs(reg1, NULL, type); +} + + 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) { @@ -718,9 +739,8 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ if (type == BPF_DYNPTR_TYPE_INVALID) return -EINVAL; - state->stack[spi].spilled_ptr.dynptr.first_slot = true; - state->stack[spi].spilled_ptr.dynptr.type = type; - state->stack[spi - 1].spilled_ptr.dynptr.type = type; + mark_dynptr_stack_regs(&state->stack[spi].spilled_ptr, + &state->stack[spi - 1].spilled_ptr, type); if (dynptr_type_refcounted(type)) { /* The id is used to track proper releasing */ @@ -728,8 +748,8 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ if (id < 0) return id; - state->stack[spi].spilled_ptr.id = id; - state->stack[spi - 1].spilled_ptr.id = id; + state->stack[spi].spilled_ptr.ref_obj_id = id; + state->stack[spi - 1].spilled_ptr.ref_obj_id = id; } return 0; @@ -751,25 +771,23 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re } /* Invalidate any slices associated with this dynptr */ - if (dynptr_type_refcounted(state->stack[spi].spilled_ptr.dynptr.type)) { - release_reference(env, state->stack[spi].spilled_ptr.id); - state->stack[spi].spilled_ptr.id = 0; - state->stack[spi - 1].spilled_ptr.id = 0; - } - - state->stack[spi].spilled_ptr.dynptr.first_slot = false; - state->stack[spi].spilled_ptr.dynptr.type = 0; - state->stack[spi - 1].spilled_ptr.dynptr.type = 0; + if (dynptr_type_refcounted(state->stack[spi].spilled_ptr.dynptr.type)) + WARN_ON_ONCE(release_reference(env, state->stack[spi].spilled_ptr.ref_obj_id)); + __mark_reg_not_init(env, &state->stack[spi].spilled_ptr); + __mark_reg_not_init(env, &state->stack[spi - 1].spilled_ptr); 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); - int spi = get_spi(reg->off); - int i; + int spi, i; + if (reg->type == CONST_PTR_TO_DYNPTR) + return false; + + spi = get_spi(reg->off); if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) return true; @@ -785,9 +803,14 @@ 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 = get_spi(reg->off); + int spi; int i; + /* This already represents first slot of initialized bpf_dynptr */ + if (reg->type == CONST_PTR_TO_DYNPTR) + return true; + + spi = get_spi(reg->off); if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS) || !state->stack[spi].spilled_ptr.dynptr.first_slot) return false; @@ -806,15 +829,21 @@ static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg { struct bpf_func_state *state = func(env, reg); enum bpf_dynptr_type dynptr_type; - int spi = get_spi(reg->off); + int spi; + /* Fold MEM_RDONLY, caller already checked it */ + arg_type &= ~MEM_RDONLY; /* ARG_PTR_TO_DYNPTR takes any type of dynptr */ if (arg_type == ARG_PTR_TO_DYNPTR) return true; dynptr_type = arg_to_dynptr_type(arg_type); - - return state->stack[spi].spilled_ptr.dynptr.type == dynptr_type; + if (reg->type == CONST_PTR_TO_DYNPTR) { + return reg->dynptr.type == dynptr_type; + } else { + spi = get_spi(reg->off); + return state->stack[spi].spilled_ptr.dynptr.type == dynptr_type; + } } /* The reg state of a pointer or a bounded scalar was saved when @@ -1317,9 +1346,6 @@ static const int caller_saved[CALLER_SAVED_REGS] = { BPF_REG_0, BPF_REG_1, BPF_REG_2, BPF_REG_3, BPF_REG_4, BPF_REG_5 }; -static void __mark_reg_not_init(const struct bpf_verifier_env *env, - struct bpf_reg_state *reg); - /* This helper doesn't clear reg->id */ static void ___mark_reg_known(struct bpf_reg_state *reg, u64 imm) { @@ -1382,6 +1408,25 @@ static void mark_reg_known_zero(struct bpf_verifier_env *env, __mark_reg_known_zero(regs + regno); } +static void __mark_dynptr_regs(struct bpf_reg_state *reg1, + struct bpf_reg_state *reg2, + enum bpf_dynptr_type type) +{ + /* 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. + */ + __mark_reg_known_zero(reg1); + reg1->type = CONST_PTR_TO_DYNPTR; + reg1->dynptr.type = type; + reg1->dynptr.first_slot = true; + if (!reg2) + return; + __mark_reg_known_zero(reg2); + reg2->type = CONST_PTR_TO_DYNPTR; + reg2->dynptr.type = type; + reg2->dynptr.first_slot = false; +} + static void mark_ptr_not_null_reg(struct bpf_reg_state *reg) { if (base_type(reg->type) == PTR_TO_MAP_VALUE) { @@ -5571,19 +5616,62 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno, return 0; } +/* Implementation details: + * + * 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. + * + * In both cases we deal with the first 8 bytes, but need to mark the next 8 + * bytes as STACK_DYNPTR in case of PTR_TO_STACK. In case of + * CONST_PTR_TO_DYNPTR, we are guaranteed to get the beginning of the object. + * + * Mutability of bpf_dynptr is at two levels, one is at the level of struct + * bpf_dynptr itself, i.e. whether the helper is receiving a pointer to struct + * bpf_dynptr or pointer to const struct bpf_dynptr. In the former case, it can + * mutate the view of the dynptr and also possibly destroy it. In the latter + * case, it cannot mutate the bpf_dynptr itself but it can still mutate the + * memory that dynptr points to. + * + * The verifier will keep track both levels of mutation (bpf_dynptr's in + * reg->type and the memory's in reg->dynptr.type), but there is no support for + * readonly dynptr view yet, hence only the first case is tracked and checked. + * + * This is consistent with how C applies the const modifier to a struct object, + * where the pointer itself inside bpf_dynptr becomes const but not what it + * points to. + * + * Helpers which do not mutate the bpf_dynptr set MEM_RDONLY in their argument + * type, and declare it as 'const struct bpf_dynptr *' in their prototype. + */ int process_dynptr_func(struct bpf_verifier_env *env, int regno, enum bpf_arg_type arg_type, int argno, u8 *uninit_dynptr_regno) { struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; - /* We only need to check for initialized / uninitialized helper - * dynptr args if the dynptr is not PTR_TO_DYNPTR, as the - * assumption is that if it is, that a helper function - * initialized the dynptr on behalf of the BPF program. + if ((arg_type & (MEM_UNINIT | MEM_RDONLY)) == (MEM_UNINIT | MEM_RDONLY)) { + verbose(env, "verifier internal error: misconfigured dynptr helper type flags\n"); + return -EFAULT; + } + + /* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to a + * ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*): + * + * MEM_UNINIT - Points to memory that is an appropriate candidate for + * constructing a mutable bpf_dynptr object. + * + * Currently, this is only possible with PTR_TO_STACK + * pointing to a region of atleast 16 bytes which doesn't + * contain an existing bpf_dynptr. + * + * MEM_RDONLY - Points to a initialized bpf_dynptr that will not be + * mutated or destroyed. However, the memory it points to + * may be mutated. + * + * None - Points to a initialized dynptr that can be mutated and + * destroyed, including mutation of the memory it points + * to. */ - if (base_type(reg->type) == PTR_TO_DYNPTR) - return 0; if (arg_type & MEM_UNINIT) { if (!is_dynptr_reg_valid_uninit(env, reg)) { verbose(env, "Dynptr has to be an uninitialized dynptr\n"); @@ -5597,9 +5685,14 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, verbose(env, "verifier internal error: multiple uninitialized dynptr args\n"); return -EFAULT; } - *uninit_dynptr_regno = regno; } else { + /* 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"); + return -EINVAL; + } + if (!is_dynptr_reg_valid_init(env, reg)) { verbose(env, "Expected an initialized dynptr as arg #%d\n", @@ -5607,6 +5700,7 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, return -EINVAL; } + arg_type &= ~MEM_RDONLY; if (!is_dynptr_type_expected(env, reg, arg_type)) { const char *err_extra = ""; @@ -5762,7 +5856,7 @@ static const struct bpf_reg_types kptr_types = { .types = { PTR_TO_MAP_VALUE } } static const struct bpf_reg_types dynptr_types = { .types = { PTR_TO_STACK, - PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL, + CONST_PTR_TO_DYNPTR, } }; @@ -5938,12 +6032,15 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, return __check_ptr_off_reg(env, reg, regno, fixed_off_ok); } -static u32 stack_slot_get_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg) +static u32 dynptr_ref_obj_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { struct bpf_func_state *state = func(env, reg); - int spi = get_spi(reg->off); + int spi; - return state->stack[spi].spilled_ptr.id; + if (reg->type == CONST_PTR_TO_DYNPTR) + return reg->ref_obj_id; + spi = get_spi(reg->off); + return state->stack[spi].spilled_ptr.ref_obj_id; } static int check_func_arg(struct bpf_verifier_env *env, u32 arg, @@ -6007,11 +6104,17 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, if (arg_type_is_release(arg_type)) { if (arg_type_is_dynptr(arg_type)) { struct bpf_func_state *state = func(env, reg); - int spi = get_spi(reg->off); + int spi; - if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS) || - !state->stack[spi].spilled_ptr.id) { - verbose(env, "arg %d is an unacquired reference\n", regno); + if (reg->type == PTR_TO_STACK) { + spi = get_spi(reg->off); + 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); + return -EINVAL; + } + } else { + verbose(env, "cannot release unowned const bpf_dynptr\n"); return -EINVAL; } } else if (!reg->ref_obj_id && !register_is_null(reg)) { @@ -6946,11 +7049,10 @@ static int set_user_ringbuf_callback_state(struct bpf_verifier_env *env, { /* bpf_user_ringbuf_drain(struct bpf_map *map, void *callback_fn, void * callback_ctx, u64 flags); - * callback_fn(struct bpf_dynptr_t* dynptr, void *callback_ctx); + * callback_fn(const struct bpf_dynptr_t* dynptr, void *callback_ctx); */ __mark_reg_not_init(env, &callee->regs[BPF_REG_0]); - callee->regs[BPF_REG_1].type = PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL; - __mark_reg_known_zero(&callee->regs[BPF_REG_1]); + mark_dynptr_cb_reg(&callee->regs[BPF_REG_1], BPF_DYNPTR_TYPE_LOCAL); callee->regs[BPF_REG_2] = caller->regs[BPF_REG_3]; /* unused */ @@ -7328,6 +7430,10 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn regs = cur_regs(env); + /* This can only be set for PTR_TO_STACK, as CONST_PTR_TO_DYNPTR cannot + * be reinitialized by any dynptr helper. Hence, mark_stack_slots_dynptr + * is safe to do. + */ if (meta.uninit_dynptr_regno) { /* we write BPF_DW bits (8 bytes) at a time */ for (i = 0; i < BPF_DYNPTR_SIZE; i += 8) { @@ -7346,6 +7452,10 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn if (meta.release_regno) { err = -EINVAL; + /* This can only be set for PTR_TO_STACK, as CONST_PTR_TO_DYNPTR cannot + * be released by any dynptr helper. Hence, unmark_stack_slots_dynptr + * is safe to do. + */ if (arg_type_is_dynptr(fn->arg_type[meta.release_regno - BPF_REG_1])) err = unmark_stack_slots_dynptr(env, ®s[meta.release_regno]); else if (meta.ref_obj_id) @@ -7428,11 +7538,10 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn return -EFAULT; } - if (base_type(reg->type) != PTR_TO_DYNPTR) - /* Find the id of the dynptr we're - * tracking the reference of - */ - meta.ref_obj_id = stack_slot_get_id(env, reg); + /* Find the id of the dynptr we're + * tracking the reference of + */ + meta.ref_obj_id = dynptr_ref_obj_id(env, reg); break; } } diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py index c0e6690be82a..2865f2b22eca 100755 --- a/scripts/bpf_doc.py +++ b/scripts/bpf_doc.py @@ -750,6 +750,7 @@ class PrinterHelpers(Printer): 'struct bpf_timer', 'struct mptcp_sock', 'struct bpf_dynptr', + 'const struct bpf_dynptr', 'struct iphdr', 'struct ipv6hdr', } diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 17f61338f8f8..2b490bde85a6 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -5282,7 +5282,7 @@ union bpf_attr { * Return * Nothing. Always succeeds. * - * long bpf_dynptr_read(void *dst, u32 len, struct bpf_dynptr *src, u32 offset, u64 flags) + * long bpf_dynptr_read(void *dst, u32 len, const struct bpf_dynptr *src, u32 offset, u64 flags) * Description * Read *len* bytes from *src* into *dst*, starting from *offset* * into *src*. @@ -5292,7 +5292,7 @@ union bpf_attr { * of *src*'s data, -EINVAL if *src* is an invalid dynptr or if * *flags* is not 0. * - * long bpf_dynptr_write(struct bpf_dynptr *dst, u32 offset, void *src, u32 len, u64 flags) + * long bpf_dynptr_write(const struct bpf_dynptr *dst, u32 offset, void *src, u32 len, u64 flags) * Description * Write *len* bytes from *src* into *dst*, starting from *offset* * into *dst*. @@ -5302,7 +5302,7 @@ union bpf_attr { * of *dst*'s data, -EINVAL if *dst* is an invalid dynptr or if *dst* * is a read-only dynptr or if *flags* is not 0. * - * void *bpf_dynptr_data(struct bpf_dynptr *ptr, u32 offset, u32 len) + * void *bpf_dynptr_data(const struct bpf_dynptr *ptr, u32 offset, u32 len) * Description * Get a pointer to the underlying dynptr data. * @@ -5403,7 +5403,7 @@ union bpf_attr { * Drain samples from the specified user ring buffer, and invoke * the provided callback for each such sample: * - * long (\*callback_fn)(struct bpf_dynptr \*dynptr, void \*ctx); + * long (\*callback_fn)(const struct bpf_dynptr \*dynptr, void \*ctx); * * If **callback_fn** returns 0, the helper will continue to try * and drain the next sample, up to a maximum of diff --git a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c index 02b18d018b36..39882580cb90 100644 --- a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c +++ b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c @@ -668,13 +668,13 @@ static struct { const char *expected_err_msg; } failure_tests[] = { /* failure cases */ - {"user_ringbuf_callback_bad_access1", "negative offset dynptr_ptr ptr"}, - {"user_ringbuf_callback_bad_access2", "dereference of modified dynptr_ptr ptr"}, - {"user_ringbuf_callback_write_forbidden", "invalid mem access 'dynptr_ptr'"}, + {"user_ringbuf_callback_bad_access1", "negative offset dynptr ptr"}, + {"user_ringbuf_callback_bad_access2", "dereference of modified dynptr ptr"}, + {"user_ringbuf_callback_write_forbidden", "invalid mem access 'dynptr'"}, {"user_ringbuf_callback_null_context_write", "invalid mem access 'scalar'"}, {"user_ringbuf_callback_null_context_read", "invalid mem access 'scalar'"}, - {"user_ringbuf_callback_discard_dynptr", "arg 1 is an unacquired reference"}, - {"user_ringbuf_callback_submit_dynptr", "arg 1 is an unacquired reference"}, + {"user_ringbuf_callback_discard_dynptr", "cannot release unowned const bpf_dynptr"}, + {"user_ringbuf_callback_submit_dynptr", "cannot release unowned const bpf_dynptr"}, {"user_ringbuf_callback_invalid_return", "At callback return the register R0 has value"}, }; From patchwork Tue Oct 18 13:59:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010562 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 F21B0C433FE for ; Tue, 18 Oct 2022 13:59:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231268AbiJRN7o (ORCPT ); Tue, 18 Oct 2022 09:59:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48980 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231298AbiJRN7l (ORCPT ); Tue, 18 Oct 2022 09:59:41 -0400 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 71B742D75E for ; Tue, 18 Oct 2022 06:59:39 -0700 (PDT) Received: by mail-pl1-x643.google.com with SMTP id z20so13883951plb.10 for ; Tue, 18 Oct 2022 06:59:39 -0700 (PDT) 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=FHeaOKnzGoZgXbP3flsb1cpZRfmFWpFjAC5Iww6rckA=; b=P1XV9l8XAEp+7/zbKAF6Le9iEnoNimZFdixJfwS7oGJnXqWt3oSPw2N7MF5lrvb6Gr TIKHA2cUriprV/d+fJNx7Q5W8gwOnzqjrzXojneM3qyRRtpMP1uIiCt22Tf635xyZ9hK A3jYa4x0Nj+E3arGGfjHYBrwwipUH9sMK2Zrc+jB9Jo6kJ9/Dy01e+Y2OeJKpof98oW/ ET2VU4bmbDZiUQQI77gETKFchCczMY6JIUvpeZxQ4WGFjg21hB1MO0I1tZ9G3nS6Agba 8T+QV0XLZqiCr9XDan2eRtshsol6F1Dr8EhN7xMnntAnago9JnleCvi3MXyQA54ft74/ SfzA== 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=FHeaOKnzGoZgXbP3flsb1cpZRfmFWpFjAC5Iww6rckA=; b=y/1591Fsljq4Z59r6wljhemSR8f1STWadNSS0L7R4aqfrwVQJeBydmWx+hbIPzOG9i APORKeVmh+DkpuSU+aQDytbkNb11G+tF6cN1HtIsRYuqdyMseSdlsja7Hf8Ms+akYUQf hLBdRJaGzSRFgV8Ds1S9+b+y6UlvQnMHA5MMQS3buLqFWutm7xzy1TkY0JBK329sOwo6 5vxcUAJesblmVig3jvmJYusLxVcmmlbZbmzxE2muHAKWJqLa/t7m50BMFn2V9LbUWBp1 VYpfaOCTB93ONqREh1k5MpA4TEOqPUOWcpR3mwGIMWhAaaVZZLBlrXP4FwCXe7nxtSx+ AtLA== X-Gm-Message-State: ACrzQf2HBGAo25ksK2CFDmqclJeTWXHVRVCbE3oeZAa30DcWTtBMXIS4 9RzuxrqnNxuWqbraKHcDFTfodSDeB8qDJA== X-Google-Smtp-Source: AMsMyM4y2WiI7JJdi1uyUH63clJUi6hIaSNX9ma5LFCSDubqN64p1tkAt/6glPCIJNtS71hXFMM43w== X-Received: by 2002:a17:90a:4983:b0:20a:9509:8347 with SMTP id d3-20020a17090a498300b0020a95098347mr39285481pjh.101.1666101578707; Tue, 18 Oct 2022 06:59:38 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id v8-20020a654608000000b0044046aec036sm8001750pgq.81.2022.10.18.06.59.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 06:59:38 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 03/13] bpf: Rename confusingly named RET_PTR_TO_ALLOC_MEM Date: Tue, 18 Oct 2022 19:29:10 +0530 Message-Id: <20221018135920.726360-4-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2709; i=memxor@gmail.com; h=from:subject; bh=le0Ej6FdJB+3fi73W83TfWykvTTpArlDPioaZJRU8tQ=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEirKpN5wM8xfd6FPqOxy/B3+aOx+ViiEdORfDh rmYgw5aJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8RyoFVD/ 9UdZe3ZQ04reRW+/HQ0T4Y02EtjrXzipKYwnjJ537Z8rmz04jEHyMlfc6EdkEqwpd967GFvYZNyynu Vj60K67lBdy2AbdWpBMsJPlNzwT5eO2Xyi9Ql88kxQsRPo/GtteTzpNq5Qs1c1p/IzB2SG6qw7yLXH CtKcKtLiXUXjHByBjgemujL27ca4VG1cAcGD1rKtSW8d0NLFfOmB9aOqNyCY6s7vQFo1oaoAxR7GUJ f3gvDp3gXdrHYFifwD2gD6C4ykA1IMrnMwtF0i3HzltSjP+7tEASOdV5MiOa2JII+7n0o+xjI3jade kswz+G3hOAKBS9GZSAE/lUBGjvm378kyOUn1NJd1dxO9YiGypYwUhUCiwF/W81rXn6ifmfCOTub8MO J3EU43NJ9JoZiNhbG3J1WebR478E8fvlDhuJj+VTDlo7BGeKy6/IwI9XZh1G3A7moYGHB2RpPIr63X SAa9vsWLhn1XGz+gNaMMAkHc+ClXWSo7h0f7IpBbfhbxiW66yJ5sWGzS+L+iWWUEwtDfUZwvnSp0NV rIK+ZmqQQDMlrTZCAX9wniPKOmjQRXvQMdcWtdiyM5wIUM0hcv8HGDm5kY8AZbfrW5BamOvMs5il0D 1ZjJakC7kPP5xWy2fbBx23F4smeL8ErCZNDP6WBdsZ60V7R+b0xY17BTmx7g== 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 verifier has two return types, RET_PTR_TO_ALLOC_MEM, and RET_PTR_TO_ALLOC_MEM_OR_NULL, however the former is confusingly named to imply that it carries MEM_ALLOC, while only the latter does. This causes confusion during code review leading to conclusions like that the return value of RET_PTR_TO_DYNPTR_MEM_OR_NULL (which is RET_PTR_TO_ALLOC_MEM | PTR_MAYBE_NULL) may be consumable by bpf_ringbuf_{submit,commit}. Rename it to make it clear MEM_ALLOC needs to be tacked on top of RET_PTR_TO_MEM. Signed-off-by: Kumar Kartikeya Dwivedi --- include/linux/bpf.h | 6 +++--- kernel/bpf/verifier.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 13c6ff2de540..834276ba56c9 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -538,7 +538,7 @@ enum bpf_return_type { RET_PTR_TO_SOCKET, /* returns a pointer to a socket */ RET_PTR_TO_TCP_SOCK, /* returns a pointer to a tcp_sock */ RET_PTR_TO_SOCK_COMMON, /* returns a pointer to a sock_common */ - RET_PTR_TO_ALLOC_MEM, /* returns a pointer to dynamically allocated memory */ + RET_PTR_TO_MEM, /* returns a pointer to dynamically allocated memory */ RET_PTR_TO_MEM_OR_BTF_ID, /* returns a pointer to a valid memory or a btf_id */ RET_PTR_TO_BTF_ID, /* returns a pointer to a btf_id */ __BPF_RET_TYPE_MAX, @@ -548,8 +548,8 @@ enum bpf_return_type { RET_PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCKET, RET_PTR_TO_TCP_SOCK_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_TCP_SOCK, RET_PTR_TO_SOCK_COMMON_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCK_COMMON, - RET_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | MEM_ALLOC | RET_PTR_TO_ALLOC_MEM, - RET_PTR_TO_DYNPTR_MEM_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_ALLOC_MEM, + RET_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | MEM_ALLOC | RET_PTR_TO_MEM, + RET_PTR_TO_DYNPTR_MEM_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_MEM, RET_PTR_TO_BTF_ID_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_BTF_ID, /* This must be the last entry. Its purpose is to ensure the enum is diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 87d9cccd1623..a49b95c1af1b 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -7612,7 +7612,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn mark_reg_known_zero(env, regs, BPF_REG_0); regs[BPF_REG_0].type = PTR_TO_TCP_SOCK | ret_flag; break; - case RET_PTR_TO_ALLOC_MEM: + case RET_PTR_TO_MEM: mark_reg_known_zero(env, regs, BPF_REG_0); regs[BPF_REG_0].type = PTR_TO_MEM | ret_flag; regs[BPF_REG_0].mem_size = meta.mem_size; From patchwork Tue Oct 18 13:59:11 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010563 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 61A60C4332F for ; Tue, 18 Oct 2022 13:59:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229935AbiJRN7s (ORCPT ); Tue, 18 Oct 2022 09:59:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49168 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231332AbiJRN7q (ORCPT ); Tue, 18 Oct 2022 09:59:46 -0400 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 70F2D6314 for ; Tue, 18 Oct 2022 06:59:43 -0700 (PDT) Received: by mail-pf1-x441.google.com with SMTP id f140so14161416pfa.1 for ; Tue, 18 Oct 2022 06:59:43 -0700 (PDT) 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=7P53Tm+dK7GmyEk+HnF9ZWyrUeeMfEERVdJscIZTe1s=; b=oemsODqSOJVGwPXy0JRcH//O5Po5XzavztZbzwVzUdRpJ/xKh8RcPAGxicP2QzR2sA nyarQreyqaq2s5msLrCTJyN6jXhEb2t1iPXgZzXRiSlAFoDMkiu2EploRo78mPfTaQBp FaTGNBC/BDnr+UhzN/HS9L9GJ8UhqGKxeqZmx6bFxugmBPT0BAn5h+jozYKM5g++oydf FB/iR3EHs7VWaIjhMIV40yav4xs3sAf++eJnlTsQlhS0YDuiN7PZz4P83Li9jdkXFRNg KK2LqPtI+/r+OvVFjT3PFFggIrMyugNDEJk0hRcXMiEudznZVJWRORlNtIwgWeVPiAWA wuWg== 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=7P53Tm+dK7GmyEk+HnF9ZWyrUeeMfEERVdJscIZTe1s=; b=Iia1K7N/vLPriAHsJuW3qdOyMKo8mayC3FsduC+pNj5QVrz3ZwNpNu06XCn90gkKfc 0KgNfznc/1U54U8ou8jU1E2qZIvcPIdIBvmNDwneLUnWkjRF2cp39vpFMniMOYH5yjkL X//LgPNZs8o6kBvdKIbtE+8KKAhpmvo1dJ2Hm1S14kWzeHJlNBLsThIFy1fZQh+QR0Y+ +4FNVehroQplygaFEVbGZN8M4bdGxBVEWKTDKxbr2w27OCyLYxJ9uzOWoKy8kMToP4V9 OuM7QH1ttBG1+9Ua7VohDt9FTSiATu9tPPnEypeCerd+K5cpAyIS3qtGmrHwRC2xYPPg c88w== X-Gm-Message-State: ACrzQf0pe6yN7Gb/8xRAeQR3iNzeExbDW4Wt2eu03nlvK8k7tjTrLvfV IfjczcUkzp3ypzMhzD8aCOB1IkmjbF3Gww== X-Google-Smtp-Source: AMsMyM4YqBYfI0eajUxB+bBIIIxdXTuGJLfMfKEyy5WegfI/gLm6bQEyjEyyvUV4sI9XRyVCEe//Cw== X-Received: by 2002:aa7:9614:0:b0:562:b07b:ad62 with SMTP id q20-20020aa79614000000b00562b07bad62mr3377173pfg.79.1666101582192; Tue, 18 Oct 2022 06:59:42 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id x32-20020a634a20000000b00456891e1dacsm7938022pga.68.2022.10.18.06.59.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 06:59:41 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 04/13] bpf: Rework check_func_arg_reg_off Date: Tue, 18 Oct 2022 19:29:11 +0530 Message-Id: <20221018135920.726360-5-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=5989; i=memxor@gmail.com; h=from:subject; bh=iGBUAdNuqJJP0N6+hvkkSz4lq1kmz+JLsrg6rQs+HAk=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEiTpGfzukt7E/RSn9qZoBlrAabWRhqTT+EGtU/ yjAA+ZOJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8RypdRD/ 9a7hwUyeZfEZLwF6l3IphOo3asWNPHkU4aaj94ebrsoh2dyzZemW+BCOKSm8T8lFmcutuahC0IC5FX iyah4jvY/rcGZIWVtnbR6RYqcfZsfq30VQVi9HhQ+niCEqI/aE+v47UV5ydCN/H7A9R44DZG1K11Dg q95YOoP9KysLBF1NcT/tRm7N/aAjWMspcJBTnZDVRqaoNF/bjsHvfQz127SgjNELxCGGIQKNpfuvhg f0iCSUlTen2PJmRYpCUXXJzfscQdQvfCidEgdh/wkXg03+16hzYnSnGPmzLfgCOTSBNRFCGxapivct mBSz+bJ6jILlE6AdlK2W2oKODINfrJNEjVHi2CZloXphrNc/LN5nz+oRQ+c7z6aQJvTch6WiTOBFsH xBs0zR4KkM3N544TRUO/GumRT4ql9ZOJhItD5R+W7V85zC+9v0Z61rvkRHyyxoy8OFSvxaut1TcKWc y1T48lKldbYUt+Fcc+yP/BEFxanRHeDKaJp3/Q8HFF/Oh4IQ4Ha3VIBx4lhKQyNx4YuikupalHR2Lh aAwTQa5Ipf9Z0PYebtAMmg+2jpuSZ6uk9jtQYMKdcVF9SDcdvegHOpQm89yrM2YihF2a5+QDaIX+Cx s6HhPyDnCHPJP0BP+eANLbGOJ8ms6esH2Qh7dWVkqXhvHMj4xvRSrNkaaESg== 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 While check_func_arg_reg_off is the place which performs generic checks needed by various candidates of reg->type, there is some handling for special cases, like ARG_PTR_TO_DYNPTR, OBJ_RELEASE, and ARG_PTR_TO_ALLOC_MEM. This commit aims to streamline these special cases and instead leave other things up to argument type specific code to handle. This is done primarily for two reasons: associating back reg->type to its argument leaves room for the list getting out of sync when a new reg->type is supported by an arg_type. The other case is ARG_PTR_TO_ALLOC_MEM. The problem there is something we already handle, whenever a release argument is expected, it should be passed as the pointer that was received from the acquire function. Hence zero fixed and variable offset. There is nothing special about ARG_PTR_TO_ALLOC_MEM, where technically its target register type PTR_TO_MEM | MEM_ALLOC can already be passed with non-zero offset to other helper functions, which makes sense. Hence, lift the arg_type_is_release check for reg->off and cover all possible register types, instead of duplicating the same kind of check twice for current OBJ_RELEASE arg_types (alloc_mem and ptr_to_btf_id). Finally, for the release argument, arg_type_is_dynptr is the special case, where we go to actual object being freed through the dynptr, so the offset of the pointer still needs to allow fixed and variable offset and process_dynptr_func will verify them later for the release argument case as well. Finally, since check_func_arg_reg_off is meant to be generic, move dynptr specific check into process_dynptr_func. Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 55 +++++++++++++++---- .../testing/selftests/bpf/verifier/ringbuf.c | 2 +- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index a49b95c1af1b..a8c277e51d63 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5654,6 +5654,14 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, return -EFAULT; } + /* CONST_PTR_TO_DYNPTR has fixed and variable offset as zero, ensured by + * check_func_arg_reg_off, so this is only needed for PTR_TO_STACK. + */ + if (reg->off % BPF_REG_SIZE) { + verbose(env, "cannot pass in dynptr at an offset\n"); + return -EINVAL; + } + /* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to a * ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*): * @@ -5672,6 +5680,7 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, * destroyed, including mutation of the memory it points * to. */ + if (arg_type & MEM_UNINIT) { if (!is_dynptr_reg_valid_uninit(env, reg)) { verbose(env, "Dynptr has to be an uninitialized dynptr\n"); @@ -5983,14 +5992,37 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, enum bpf_reg_type type = reg->type; bool fixed_off_ok = false; - switch ((u32)type) { - /* Pointer types where reg offset is explicitly allowed: */ - case PTR_TO_STACK: - if (arg_type_is_dynptr(arg_type) && reg->off % BPF_REG_SIZE) { - verbose(env, "cannot pass in dynptr at an offset\n"); + /* When referenced register is passed to release function, it's fixed + * offset must be 0. + * + * We will check arg_type_is_release reg has ref_obj_id when storing + * meta->release_regno. + */ + if (arg_type_is_release(arg_type)) { + /* ARG_PTR_TO_DYNPTR is a bit special, as it may not directly + * point to the object being released, but to dynptr pointing + * to such object, which might be at some offset on the stack. + * + * In that case, we simply to fallback to the default handling. + */ + if (arg_type_is_dynptr(arg_type) && type == PTR_TO_STACK) + goto check_type; + /* Going straight to check will catch this because fixed_off_ok + * is false, but checking here allows us to give the user a + * better error message. + */ + if (reg->off) { + verbose(env, "R%d must have zero offset when passed to release func\n", + regno); return -EINVAL; } - fallthrough; + goto check; + } +check_type: + switch ((u32)type) { + /* Pointer types where both fixed and variable reg offset is explicitly + * allowed: */ + case PTR_TO_STACK: case PTR_TO_PACKET: case PTR_TO_PACKET_META: case PTR_TO_MAP_KEY: @@ -6001,12 +6033,7 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, case PTR_TO_BUF: case PTR_TO_BUF | MEM_RDONLY: case SCALAR_VALUE: - /* Some of the argument types nevertheless require a - * zero register offset. - */ - if (base_type(arg_type) != ARG_PTR_TO_ALLOC_MEM) - return 0; - break; + return 0; /* All the rest must be rejected, except PTR_TO_BTF_ID which allows * fixed offset. */ @@ -6023,12 +6050,16 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, /* For arg is release pointer, fixed_off_ok must be false, but * we already checked and rejected reg->off != 0 above, so set * to true to allow fixed offset for all other cases. + * + * var_off always must be 0 for PTR_TO_BTF_ID, hence we still + * need to do checks instead of returning. */ fixed_off_ok = true; break; default: break; } +check: return __check_ptr_off_reg(env, reg, regno, fixed_off_ok); } diff --git a/tools/testing/selftests/bpf/verifier/ringbuf.c b/tools/testing/selftests/bpf/verifier/ringbuf.c index b64d33e4833c..92e3f6a61a79 100644 --- a/tools/testing/selftests/bpf/verifier/ringbuf.c +++ b/tools/testing/selftests/bpf/verifier/ringbuf.c @@ -28,7 +28,7 @@ }, .fixup_map_ringbuf = { 1 }, .result = REJECT, - .errstr = "dereference of modified alloc_mem ptr R1", + .errstr = "R1 must have zero offset when passed to release func", }, { "ringbuf: invalid reservation offset 2", From patchwork Tue Oct 18 13:59:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010564 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 D86A8C4332F for ; Tue, 18 Oct 2022 13:59:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231295AbiJRN7y (ORCPT ); Tue, 18 Oct 2022 09:59:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49676 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231298AbiJRN7v (ORCPT ); Tue, 18 Oct 2022 09:59:51 -0400 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 D595231343 for ; Tue, 18 Oct 2022 06:59:46 -0700 (PDT) Received: by mail-pj1-x1043.google.com with SMTP id 70so14056138pjo.4 for ; Tue, 18 Oct 2022 06:59:46 -0700 (PDT) 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=hXRgOgNBUr5iOiUQ7IQEqWKoapMJ63yhRh4wTj6vkOk=; b=qw+DyjjwOfI/3SD2dvboNm9gr/rIp/HBYbWpREJduYOaRTwg/+AThs8Pu4H74qxxti iWFUcEnzWYN/s5YEK46F2ULjSpmw9V8fxATFn4ZO55PI8pz9HoH/48sh10DXPw79sQkw XBM9QQiQuWtinOcEt8rqv5LTvq2I3TugAD0/fgWYCfI2+eh2fOjgIRuxTMNLs15rFOFH OpbT2xfJBnav1IadtkH5y1g1czmJBgtfujU+YUn+EytuI6AFtrBDz8HNcafe1Ilg6mP5 4U0kJMtyEpgoNSgMgC3lTRlaZeGg+AHVPVc+zxE9Mjjb3ko0HLgL8BscUR5Upcuo9HP9 P6zg== 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=hXRgOgNBUr5iOiUQ7IQEqWKoapMJ63yhRh4wTj6vkOk=; b=nJApVy/S5XKlHWbut0Z8CXJeU8YR46jY8idHFVxPP38UCX2ZTSFDXhUfaZaQ25vkRW uaQZIuI/1yA4bcVYGcGA93JJqR3nuSsJIjHkg/9hrL+KiJGf9Fp3VgsakUz+Sl0oLZ19 VStJPMRAPaFxYwQue+8LINbOasR2y0iFCAY5Wk9wLr4u0B+mGts7NAaqlD6pxfMhD0MX u5XqAR8YtX5bj4HnCSg4aHg14BV/8mNlVuh9keRlTBrQetC+pkKEEq2mP4XB1cXTup08 OqeEgKITSQh8/d3/5tl4+ubU8uLxC/c9Yksr6KcNL930/KaO4kKNHuGHjl8AdmNHQUOF Rspw== X-Gm-Message-State: ACrzQf1poAutwPSKj8oxYNAlEDsSrql8JkWw/ivXvjDf7bBuLlsDfzmx Q+gG/zB22GPfga5VgTKksWgUJyqASOR5Zg== X-Google-Smtp-Source: AMsMyM7qCejzuvITG0pspG1DV/qO+ShHmQ07vJRqjA5BqrzBULNNfRGZ9veNhEXkkxMjXA8BevpmPQ== X-Received: by 2002:a17:90a:e7ce:b0:20a:cb3f:faf0 with SMTP id kb14-20020a17090ae7ce00b0020acb3ffaf0mr38805962pjb.148.1666101585254; Tue, 18 Oct 2022 06:59:45 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id r16-20020a63ec50000000b0045ccc30e469sm8029511pgj.25.2022.10.18.06.59.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 06:59:44 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 05/13] bpf: Fix state pruning for STACK_DYNPTR stack slots Date: Tue, 18 Oct 2022 19:29:12 +0530 Message-Id: <20221018135920.726360-6-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9159; i=memxor@gmail.com; h=from:subject; bh=78q+shD1c9FVwOu63DoS5Gf+7vZBOe5czpymXzjEIQo=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEiuuOfo2rIN5czdU7t52wfU5pZKA3DsB65kIfL WhXP0YWJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8Rylu/EA DAq11CLLfNp6pmAjSCpmZ6uMqrb63hAIiH32JFFaduGplUk/q0e2xDXcBoz28oi6iW+l7d90UjfHyl 8eVsxjNFDhW8t4t8yUOjwjquGvsdvi0SVYzoOq/976YJa6ZHZ58XJtgwQvvJ52lxw7Q+cAvDgJ7hnJ 9y0k7mTHaTzkdzb9WArw4m+r0XtW30kWUu6Y4P0yquRYd4BT74Pva3HwqfzRnrpRUeC+lIwZL8CK1W e1EzMlPFE5oqgD/eGVjY2XB5OcvcCHU9CgrR7b/YbJK+fFaJgKkKkAHtERikovSdBeCjGWsPqZKEi0 LNE+jRsk1jzH4rmS68KRX5x/ozmbXRqEocanhMfiKQhRH6sRoI+wCmoVdei/PY7asjmbs9PQUdVxmL FVgI+CDw4cJTjqOQ7iSxDsUgy4OTRlPlZLO/WTzV82/N/7DfHI7QKRKVPkLBSuz8PI61+DC2trBICk AHMUVvj7ZhbhKNrPTw3iA4T8b6oVQ+YZOzgMrdpahSi3OfzWZeTJpAKApR1hakzkiuf+tq5nEl2KrA +WARRe3hNim8z0PVfKbNiCcSv12k6gMcEBOaWb6tAjTY0tLBfYxJMP5M9x84z6ZKnSrlBCHZyf2a5R 3Nzax77diPRQe57dY+UDvNaGyOig8YZGHXolvQe2LX2AjSflneSD9odJuqUA== 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 --- kernel/bpf/verifier.c | 73 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index a8c277e51d63..8f667180f70f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -752,6 +752,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; } @@ -776,6 +779,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; } @@ -2354,6 +2377,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. @@ -5648,6 +5695,7 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, u8 *uninit_dynptr_regno) { struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; + int err; if ((arg_type & (MEM_UNINIT | MEM_RDONLY)) == (MEM_UNINIT | MEM_RDONLY)) { verbose(env, "verifier internal error: misconfigured dynptr helper type flags\n"); @@ -5729,6 +5777,10 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, err_extra, argno + 1); return -EINVAL; } + + err = mark_dynptr_read(env, reg); + if (err) + return err; } return 0; } @@ -11793,6 +11845,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 Tue Oct 18 13:59:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010565 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 5AE39C433FE for ; Tue, 18 Oct 2022 14:00:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231332AbiJROAA (ORCPT ); Tue, 18 Oct 2022 10:00:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50172 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230477AbiJRN74 (ORCPT ); Tue, 18 Oct 2022 09:59:56 -0400 Received: from mail-pf1-x442.google.com (mail-pf1-x442.google.com [IPv6:2607:f8b0:4864:20::442]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CEEF03ECE9 for ; Tue, 18 Oct 2022 06:59:49 -0700 (PDT) Received: by mail-pf1-x442.google.com with SMTP id p14so14168448pfq.5 for ; Tue, 18 Oct 2022 06:59:49 -0700 (PDT) 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=9cHKgfMxfRcBLnSN/RhbiQD5k8u+SrqxjXlNV53Xio4=; b=WFHb42vw3HzG0YWxgVjeO7IWyQZ7vOI+ygUYphNhp+T2tH/sc/kdAXH3OVGjrXPZbV ql4GS9YPUOmW88KNIckigq7YsNggo9dnyknTGHbm3ibSBhb90dcWvgyWoUq/KDShrZ2B NtsOOPyKf2yEPa3y4ee2CHEFMKcIHbbhfZD/kMxhx+Zz36cdAtB27zo8WUksJ7nDacu4 pU4c86uy2O5m6Nl1rbdUM5Bo7kHb/0pasFavftOMoaxWdE+lM5bTwwfuv8HTRf8dBnoT 71XIWtKcHsljq9JLch+poFCZ1OVrLlYL2PNrh6XhjcWfkhbJiVqaRsV1UH1GZ3lNH/s4 2xHA== 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=9cHKgfMxfRcBLnSN/RhbiQD5k8u+SrqxjXlNV53Xio4=; b=cL+P747MkkMMVPpAkXAugVpntNevcwWliUTJgRrXIqY3dsEziOB4XUuViqKz2QNXmv LbLzs2ktI1JyOoXiyjmLGsjqTKstn16q6oLRzxMzNluKCYqihe3X5E7SkCuXCG+8hbup h4AYdO7WdtPTTF3hzQioMWAhKGlDCEJ/ixMvSh+0HyfV4WxYOazjn6GDU3tGu6vDj1kD CG4sE5rMrSrXSRewESnaeqtsUEMbFIFPlQEydAlwUflnnzeRIye258zgkSOQkzNb68kB eRZj3wZnHwfCRFnYYoM/oFF+k+M9hFIThqrfT1zKo4qBZPZTbSDpHJSPfCiR/ur1tzjL uPzg== X-Gm-Message-State: ACrzQf0B2TuANfjXtGRdvQK6EEFX1fpcG3tXrvxSltK0s7y/MygGmrlm DzN/2gX8LuX5C38NS45dj0d3Nfhi+9ZfKg== X-Google-Smtp-Source: AMsMyM6rXgNMJ4NZ3TTPanLF/bsEENTrpZIWtMLVAj5A3hifUMEjVYp4/ktG4HdGXCYWl4o3WdLD1A== X-Received: by 2002:a05:6a00:21c6:b0:563:9169:3a4c with SMTP id t6-20020a056a0021c600b0056391693a4cmr3464056pfj.56.1666101589012; Tue, 18 Oct 2022 06:59:49 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id a188-20020a6366c5000000b00460ea630c1bsm8160387pgc.46.2022.10.18.06.59.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 06:59:48 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 06/13] bpf: Fix missing var_off check for ARG_PTR_TO_DYNPTR Date: Tue, 18 Oct 2022 19:29:13 +0530 Message-Id: <20221018135920.726360-7-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9846; i=memxor@gmail.com; h=from:subject; bh=HP2s1vl4++9YP8xLPS535iwMoswch8EjwABHeVDtj3Q=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEi6cwODdyDzmVuM4xhVFKgMSxvT7vFYpH1MoSa dIUucxuJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8RygHlEA CCdgwaouAYKU8ko/gGj9NM2lsKSgJVn/1inhwywE0RTO/fkxaCZjITDnKSMD2MoVnR3iogpqOJlbXR FJhgHmLoV2nGTFmqjaDfgqeK3NcCQrlr/9EbvHQ8inGvZxgpovrbYW6F7/vCVhYFTn07bm70hyjmQy wUUxeEJ2oWBOZx+LIfwbXaFn+n/OA+URRwB2roenNSub2ITetCVYpPj5jdjGa7XjqN+ATrhYShGZyt Tfy8MYntGadmvuGg3kbcs5RjBumVYSNlSKjb46hzPQA/DFflCEkugQ9F/sMSyJ4SPwFDGhNxFgbvtm eKA8V2f2Max1JRZFUCZUd0vrZzf8o2ZOqSgM1A5PDORj+rWQ7BqxtY41s20CijPvZY37/0RxEhNj+3 wJE54CvQTu2eo6yfjMQ+XBJ6NsHbyLAGL8M65n+CiZMWGx18RTyWlRN0EFgOHDXGhlcR+RgM/6TrEb C/KKdJ6rI5r35YjeRjnZ6k7YKNN/9hr9ShJirAP63JrUWxkT7RuKiNhr+5auFEmSqUjL6IMILfAGF9 ZtrFJWQBLn8+uzEd2Cuu6UXaWlfqXioXarBhQOm+rBBaFzc6lsJcifGx1tFwxict6oHODIHP3EmFHz K5ZJJ12JJrv24gNhVc1vhpoV7e6ZdVESWouvrnqIzk9yTh3ic9llss/L97AA== 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 | 80 +++++++++++++++---- .../testing/selftests/bpf/prog_tests/dynptr.c | 6 +- .../bpf/prog_tests/kfunc_dynptr_param.c | 2 +- 3 files changed, 67 insertions(+), 21 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 8f667180f70f..0fd73f96c5e2 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -610,11 +610,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 spi; + + if (reg->off % BPF_REG_SIZE) { + verbose(env, "cannot pass in dynptr at an offset=%d\n", reg->off); + return -EINVAL; + } + + if (!tnum_is_const(reg->var_off)) { + verbose(env, "dynptr has to be at the constant offset\n"); + return -EINVAL; + } + + spi = __get_spi(reg->off + reg->var_off.value); + if (spi < 1) { + verbose(env, "cannot pass in dynptr at an offset=%d\n", + (int)(reg->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; @@ -725,7 +748,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; @@ -763,7 +788,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; @@ -810,7 +837,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; @@ -826,14 +857,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; @@ -864,7 +896,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; } } @@ -2388,7 +2422,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. @@ -5663,6 +5699,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; +} + /* Implementation details: * * There are two register types representing a bpf_dynptr, one is PTR_TO_STACK @@ -5710,6 +5751,13 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, return -EINVAL; } + /* Additional check for PTR_TO_STACK offset */ + if (reg->type == PTR_TO_STACK) { + err = dynptr_get_spi(env, reg); + if (err < 0) + return err; + } + /* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to a * ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*): * @@ -5728,7 +5776,6 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, * destroyed, including mutation of the memory it points * to. */ - if (arg_type & MEM_UNINIT) { if (!is_dynptr_reg_valid_uninit(env, reg)) { verbose(env, "Dynptr has to be an uninitialized dynptr\n"); @@ -5791,11 +5838,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; @@ -6122,7 +6164,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; } @@ -6190,7 +6234,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, int spi; 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/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c index 8fc4e6c02bfd..947126d217bd 100644 --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c @@ -27,16 +27,16 @@ static struct { {"data_slice_missing_null_check1", "invalid mem access 'mem_or_null'"}, {"data_slice_missing_null_check2", "invalid mem access 'mem_or_null'"}, {"invalid_helper1", "invalid indirect read from stack"}, - {"invalid_helper2", "Expected an initialized dynptr as arg #3"}, + {"invalid_helper2", "cannot pass in dynptr at an offset=-8"}, {"invalid_write1", "Expected an initialized dynptr as arg #1"}, {"invalid_write2", "Expected an initialized dynptr as arg #3"}, - {"invalid_write3", "Expected an initialized dynptr as arg #1"}, + {"invalid_write3", "arg 1 is an unacquired reference"}, {"invalid_write4", "arg 1 is an unacquired reference"}, {"invalid_read1", "invalid read from stack"}, {"invalid_read2", "cannot pass in dynptr at an offset"}, {"invalid_read3", "invalid read from stack"}, {"invalid_read4", "invalid read from stack"}, - {"invalid_offset", "invalid write to stack"}, + {"invalid_offset", "cannot pass in dynptr at an offset=0"}, {"global", "type=map_value expected=fp"}, {"release_twice", "arg 1 is an unacquired reference"}, {"release_twice_callback", "arg 1 is an unacquired reference"}, 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 fc562e863e79..e4b970bc2d3f 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 pointer type STRUCT bpf_dynptr_kern not to stack", 0}, {"dynptr_data_null", NULL, -EBADMSG}, }; From patchwork Tue Oct 18 13:59:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010566 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 E5107C4332F for ; Tue, 18 Oct 2022 14:00:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231339AbiJROAC (ORCPT ); Tue, 18 Oct 2022 10:00:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50702 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231344AbiJRN75 (ORCPT ); Tue, 18 Oct 2022 09:59:57 -0400 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 31AF3558C4 for ; Tue, 18 Oct 2022 06:59:53 -0700 (PDT) Received: by mail-pj1-x1042.google.com with SMTP id g8-20020a17090a128800b0020c79f987ceso17418345pja.5 for ; Tue, 18 Oct 2022 06:59:53 -0700 (PDT) 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=YEas3sHBE0VIucB962Ttze9x3m2xbgs7+/0A1P4FfmI=; b=P3pV10JyNUT3/Isn+Vhb93GBZ1yRJAMXRzfCEyOgcJe/tQJXmAesq64oy7MZPPZEB8 bK/mebiozuuYI4W7POy+hlvKFoK7K0AKBt8bNIVX4xznWv+/Lv+TSPrwg0LVhQnG1aDa CSREr9JUYqBJXbxAUwWbsh9d0bhyFbktcKmGMbRQkjZwG5Md1Ih2IdCaZ0pV29/x7hWK /OqgPImMW+yqPVdNmVl75xvvm5kbBL4dWiFZnaupr11Ig+RZgPtRXb2CQHDAb2Nm23NL mymf5smdrsLUJo5FjdEl+s5FaOYMC9tLfKnEgBHeSs8Z74tsv3NOi7LC01X2n0tQwg5S iF/Q== 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=YEas3sHBE0VIucB962Ttze9x3m2xbgs7+/0A1P4FfmI=; b=5DepkpC9ygbfsYmdFDckeiKjxTIV+YcLnPfaE9fyBBn9itpjsugXFWYDO6+DIxBwlr sQjstJnDgtOsmgApJsg/+JQe/fYeVh9Gy9RQTPH0UCbz/G6R7gCdKYO4vxWSe7OUIeuv Wh+L0B2VX87usOJn5X4/catjGG9egAdb9/DeKWt+fBiWXi19k4ZGFvGrlFHWH5GaQuZA gdjDKjlL0HTZU2hinYCB8DNcA4S6Lr8uweu8XjKNRDgy0uVPJ2fRgwNC7tT9GRLWGORs BmKkWDVI6DZcMq4dfAA1MWuJxmJCCjWufSzDRtEIIvJpN6/fb6/VgIPhqWERUce9b7Bo J1ag== X-Gm-Message-State: ACrzQf2Fbl8U5QLyQDY7pUPHTk2gmHlwCMxVprfS4gTGIXApVsC7+u7x 1pfQCBK0mjQPsk6JBrsQ7pEkzJo8SfsMjQ== X-Google-Smtp-Source: AMsMyM7ON8iZwf1/1fIQpd42FhlCKOLQbLhCBhL7syha7Z5m2+xq4QKTL8suxS8xxLqEWb4hRJqc8w== X-Received: by 2002:a17:903:2307:b0:17f:78a5:5484 with SMTP id d7-20020a170903230700b0017f78a55484mr3240136plh.15.1666101592799; Tue, 18 Oct 2022 06:59:52 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id i6-20020a628706000000b00553b37c7732sm9218888pfe.105.2022.10.18.06.59.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 06:59:52 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 07/13] bpf: Fix partial dynptr stack slot reads/writes Date: Tue, 18 Oct 2022 19:29:14 +0530 Message-Id: <20221018135920.726360-8-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=7778; i=memxor@gmail.com; h=from:subject; bh=RJRVR1fT/oS1JF7iDoYDuwVKzXdYOHNg/YrQkETwO1Q=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEidPPx4svIURqVZQdU2pvTS+nRVXs10MVGhkn4 nz802m+JAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8RysClEA CjGxB4JARJLDCziUOf1Q5/ON3o4hsvYB6V8V6LDWw8cHrY75PYTsPmFPJSYi0+B9Fq9fT9eZtCteGn JLQOPHAM7mD1wi1Qwg297cL2nuMLFnk/THvkpC6IIYNn5ITAg4K0pqyFPmgeALWB5bNWmJ/5kwN1Ur OdWnbOXe00/hqWGyk1Wh1mH9pMbQ3JSpRarzrVjBEwsz0bUrM1ygVT6zX4fD0iF7akQ5upWGPRc25C 1NOFu8DK0mqdWDbS+9t+L0c/1q2m0cnBUMCpsZClPoovzx0VnsSgPYWHdROJHHIiUjASX2o+QsA1DG 1b0Cse5XwFCoEJ7qz3lNgmiKw4cR9PTGLuxpPtRSPE1SMIPx0dpEsZWy28HqqD09gKAFDMhq9MH2w9 rfyTNGZzJ//AyKox7xgfAED9TlFiUBoCPiRqs/ABjHuD8xfZxy7ldwcJLMTeTOle/iF9nVsDokWuJV 5nUXqUyK9jBMTLOPa4k7XrBJ5MgSz7U+SyW5STLRINRQcKB3HlEwdbKZOBZeSzNiRyuT9Uj84JiUrU B2HlcD0q28E7iIFp+doiQ5sxrlRIKxs2iolMVGePMpY2kDixWICN+CmjnUkilKTY6QwOQIhgVx+ppu ATTvpJ79hSmeoefvqLWEPBJ9zEZIR0tA3mEVJmPuPoUWqfaJLrUOuaqN+/Rg== 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 | 76 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 0fd73f96c5e2..89ae384ea6a7 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -740,6 +740,8 @@ static void mark_dynptr_cb_reg(struct bpf_reg_state *reg1, __mark_dynptr_regs(reg1, NULL, type); } +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) @@ -755,6 +757,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; @@ -829,6 +834,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); @@ -3183,6 +3226,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) { @@ -3296,6 +3341,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++) { @@ -5257,6 +5309,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 Tue Oct 18 13:59:15 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010567 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 C1F07C433FE for ; Tue, 18 Oct 2022 14:00:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229901AbiJROAE (ORCPT ); Tue, 18 Oct 2022 10:00:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50018 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230110AbiJROAA (ORCPT ); Tue, 18 Oct 2022 10:00:00 -0400 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 5C00B67CAA for ; Tue, 18 Oct 2022 06:59:56 -0700 (PDT) Received: by mail-pl1-x643.google.com with SMTP id c24so13934901plo.3 for ; Tue, 18 Oct 2022 06:59:56 -0700 (PDT) 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=Ab69KN2/e44z8xRbQAhKsu+NTn2Sox1wt5+f8LQTZSU=; b=aM63/vCM1YqQjxumzUMwmtgft2MBDFGAfHXJIF/D7vkRItgXzeMRMmHSx2QiCxgxM8 RJaslAsw3amJOvfciJV5ig9m0nZPPr9PJzf+45zEN4gU/fY/fD10M8bHbX11WCKaBqSI p1XRJvAEVScgvil7bKtq4gtE454PP0BftCZa+6dHQZIxDVvQhXVpHHBqRUWz45TNg5le e4vgI/F4oRsKOCJCgZKW7xLvJnLlZlT0PXqJqqXr+/vt922u/JOCB9HboMqf+R6c8Y82 cdhlFUnVyfjXOZhavbagNI1AJfnzdGF0FX83zowIYJMGPYj3xmF1Q5WzG8ZqkX9gWL/1 iM3Q== 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=Ab69KN2/e44z8xRbQAhKsu+NTn2Sox1wt5+f8LQTZSU=; b=p9PEPKEec+XDHj1VfR8JnX93nMwHJtuZEA1tcDraUlB2q58ppxPNARHuvkpX5czT4F 67TCKU/bGOsrRXY38ysXx2xOhJyU1/oeqM0N7TeGkZw4J4hfKKbnCDWzTBTZnR1lxKg/ K6IehZP+9PJQyRwzM2ezQIG4dN3J/rKBlCe5+G1OaYxlDYMPyxSAA6aA57/tsc4BiWSS +tSFVBwAgk3YzLiC5xR4gQcY0TKXXpTZV8GMA011M7dOAa/mibkJxvvG/gwSG4P+8B8F OXa4o59SXzMCZUsE62LAdgnxztmhbmBKP/LPP0r72Md1/9PaM25bF5ZPD6+zcQHM57dH N0hA== X-Gm-Message-State: ACrzQf2TH0JhnjHsLUHfBvJ3fpyBDOqzONApzf8XlQwxTfowaWsqaWrh T194gPe82SHw7WyEq0d1p4I/BH+hxvhobg== X-Google-Smtp-Source: AMsMyM65Nha+HkNct1B+6xzhZHg05h3cr2C54u78Uz0/spGHTudQ3FS4ZfRJ7hx2p4uFZO9Pc2lQ5w== X-Received: by 2002:a17:90a:65c7:b0:20f:8385:cc18 with SMTP id i7-20020a17090a65c700b0020f8385cc18mr3443147pjs.235.1666101595586; Tue, 18 Oct 2022 06:59:55 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id s90-20020a17090a2f6300b0020a8e908dc8sm11448603pjd.4.2022.10.18.06.59.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 06:59:55 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 08/13] bpf: Use memmove for bpf_dynptr_{read,write} Date: Tue, 18 Oct 2022 19:29:15 +0530 Message-Id: <20221018135920.726360-9-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1215; i=memxor@gmail.com; h=from:subject; bh=yy0R7WMEdJOFW1d2cPFR+SghjmgeriaKeEiD8vgQa48=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEiI8CDi+t5HXehhdsBZK48eu18pbylvZrWKgzQ tpcCmG6JAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8RyvrqEA C8FzhtAAD0ruo9ZiapgZZtBSyxKC87I8qLEgYnWACofz6GvXbDy1Y6C4vK5NWy84m8ZUdOE2SWRtn5 WyPUDsYIRqbYiiLkgDWWuebCcd/SXVq/GKK/R3q9UDxXSeCOkZe3KmzfSMmnL3YTM4bdLxH9amj74t l0ggn9WVNotAgjrv5fa/n42LygMmnLt8f9f0DxfTG9B5up4ERCB2HTy6YqdPFrg8hg0b0Iw/RAIJUn rKaLMTv5N/IiZyprc/1KRuJsvq0DumLwXrs0x8FT8sG4dTluATPLTg27ru23oYQu4kYVK1+HYy0uCX 6As98zzcMGvlNLybGaJ2U8jhTjFZ+WF1rSBYKcZahFxb8YnxzgC+R+6FJ3HltmI9CZRTwXZMN063gT dVmrfJODoPCRGXr4BcLxrtx1sZhYTnCz6ZL8iPigRLfgNGK6vEyaOd/SRX9MdPxsrv/4TsGEEwz4sk P74pb7ZsI8hVbCv2m//X7xGUoPP7uOb3cL+z0JHPIy2UhHFMELJCScekiUKjfPGcTM38ttLKBNsSme 8XwKnKBUdDJ+nxD1gpI3ULjjMT78ejkaq1yXML4/EYVfx6wv98jZAo6+ST3q9eSgLrfdft3Kt1ecc8 46QRCeWTvvJXwo//u6tYECDR1CmeCOxT12a9nytPg6hUQExC9otfeRtyZ7Ng== 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 It may happen that destination buffer memory overlaps with memory dynptr points to. Hence, we must use memmove to correctly copy from dynptr to destination buffer, or source buffer to dynptr. This actually isn't a problem right now, as memcpy implementation falls back to memmove on detecting overlap and warns about it, but we shouldn't be relying on that. Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: Joanne Koong --- kernel/bpf/helpers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 0a4017eb3616..2dc3f5ce8f9b 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1489,7 +1489,7 @@ BPF_CALL_5(bpf_dynptr_read, void *, dst, u32, len, const struct bpf_dynptr_kern if (err) return err; - memcpy(dst, src->data + src->offset + offset, len); + memmove(dst, src->data + src->offset + offset, len); return 0; } @@ -1517,7 +1517,7 @@ BPF_CALL_5(bpf_dynptr_write, const struct bpf_dynptr_kern *, dst, u32, offset, v if (err) return err; - memcpy(dst->data + dst->offset + offset, src, len); + memmove(dst->data + dst->offset + offset, src, len); return 0; } From patchwork Tue Oct 18 13:59:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010568 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 2453DC433FE for ; Tue, 18 Oct 2022 14:00:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231330AbiJROAJ (ORCPT ); Tue, 18 Oct 2022 10:00:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51384 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231358AbiJROAD (ORCPT ); Tue, 18 Oct 2022 10:00:03 -0400 Received: from mail-pf1-x444.google.com (mail-pf1-x444.google.com [IPv6:2607:f8b0:4864:20::444]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A8830696FB for ; Tue, 18 Oct 2022 07:00:00 -0700 (PDT) Received: by mail-pf1-x444.google.com with SMTP id h13so14154605pfr.7 for ; Tue, 18 Oct 2022 07:00:00 -0700 (PDT) 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=Bg4TpE7gOTDrWPR4AW73VePXXzUIdnV9jgo5mQEobvg=; b=kdiMy+SZGbl4STSi7Kc5JMdBwXgar53vpaYb486izQmZOcGK6rfAg4z54UwGHCTlk5 q3t3vuwiRfD19NeOiXIqihfeB/jlybfTInXtz3Gnht45NkZ+2dJOYec9u0cGLBFwmhw9 M6JEOr9WqGbPSAfrTQZIm++Ftq5GnXnDDo0QOAlmrz7MmoFsBcn5S+XiV+6z1hwFjzMq A1yV7cdnxfuCFJuLMpzzT0aND+lIPglaDmdci0hfXrTuEP/ge6qY9qpK3tTn4lc/r0s6 VAQgKr1cR2UwkC8IF5wSSxn+YQn1AQvjedU81XKA947cjled7yXgWVTNhRHu8124eMae yn2g== 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=Bg4TpE7gOTDrWPR4AW73VePXXzUIdnV9jgo5mQEobvg=; b=v3VeCUpPVMfiYJbObvu23ivkMlP3HiSFFHMNgAHUfQ4snTa7nyOG4j5SLX5g3HMRjm 0ASrbIMrtkhzJnACuuyfH1o29SMgHvb7mlF4cp6vnFl2mmTl3gqQACH3kltIIS+9VjbC o/T+6fq/CHAmEUfm7vKdU+vdRZju1JKzM+1LjtdnXgBNcWlsHiZAczZugpxfOvH4k9mM nxUOdzbWcnew3cOSG2whUWBArPnVQeF7+ZjN6r/el2/soka9XItHtb35A5wo0JlqpSb1 6PE1O9QIVnRq6SVgM/++KlEw7esUBZOo3YCmutXD396/1lbc0h4JF9S4agVKrkuIyLOh uq3A== X-Gm-Message-State: ACrzQf1TDtlvY4Kn/srN7CcYIwAvvCN7fTzZ9wRQkjxU9bFN4GfGT9BX eoEsDFd9/nzjqqhL+qhMAtD/8F8Nt6oXiQ== X-Google-Smtp-Source: AMsMyM5uUZMjENYIa27pCLr5iCoOuNLZRsW3JXdMixuNSaukt7ia+vmsgc/U5XnRKCElIJEqM+6qOw== X-Received: by 2002:a65:580a:0:b0:439:befc:d2b0 with SMTP id g10-20020a65580a000000b00439befcd2b0mr2722649pgr.302.1666101598971; Tue, 18 Oct 2022 06:59:58 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id u14-20020a62790e000000b0055fe65e9203sm9361430pfc.0.2022.10.18.06.59.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 06:59:58 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 09/13] selftests/bpf: Add test for dynptr reinit in user_ringbuf callback Date: Tue, 18 Oct 2022 19:29:16 +0530 Message-Id: <20221018135920.726360-10-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3027; i=memxor@gmail.com; h=from:subject; bh=w+dQmpzIhiJvX6EB6pYHQqoDf3V0oUCaweyNMpUT0Sc=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEi9gqtf+Uuyer0SG0n6yUuPNBIZyyL3mFccsND mYhr822JAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8RyqLdEA DDCzIrZz9SjzmTJJVlpCZQgSecOaCYI1Xn0kIuhl4CCf+WmPGGiAY47lPwuHcGwzYt3foOo5PdpBVH D79Xs8JTjDWj7P3y9KKwVs6GomRHskbiEx/aWxIG9H2GVahoDRMW4bwMv2M3NaJSTF50GFU09uWxNI fiXzmUgdME7IsaVqwRQ5E8d7Uu9a09F2a4TFSpsi3PK9X3w0V4+RV5ewfsN8SXV3rzrNizDOSqy+xV ImAc3QMN4pdLu/ckxuuG1H3VPe4WZjIGirO/Q4bTqTXwvfOvI/baieeQNExfozCG4yAU4ejhI/BgYz o3qI0Ko0HOoIV7iXPrdsvA5/F3syoF5M6OI5ERAXWFHjdjas+JIm8/A1Uy0XRsHWBLOq4SInl7bJwm A30PcaQEDWhZzSoIdeZ0YKLads69oHHcwEfhd7vY5+0PuPndCMKB+xJABNVXXih/0wmXaRo1LUNkeJ u7hyalkOF+7jCjI81LnqNy+SHFlPNdgUxapFgkYGd2IBtOww3o9I7yyiKlV7RhtsazD7Lal1+uJQJ2 LscD8cLO+dwMVncJ2TV2AWqPWb2cDG0r1CoSLTboHiiez5WlAHHEriNyMv9+1RgvrgyxyoGhUWqumB 4HTfWiNrHAk54g1ztFvRTsCZngaxE/dartWrd+6l9gko8vd74sPd/0gs0Vbw== 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 original support for bpf_user_ringbuf_drain callbacks simply short-circuited checks for the dynptr state, allowing users to pass PTR_TO_DYNPTR (now CONST_PTR_TO_DYNPTR) to helpers that initialize a dynptr. This bug would have also surfaced with other dynptr helpers in the future that changed dynptr view or modified it in some way. Include test cases for all cases, i.e. both bpf_dynptr_from_mem and bpf_ringbuf_reserve_dynptr, and ensure verifier rejects both of them. Without the fix, both of these programs load and pass verification. Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: David Vernet --- .../selftests/bpf/prog_tests/user_ringbuf.c | 2 ++ .../selftests/bpf/progs/user_ringbuf_fail.c | 35 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c index 39882580cb90..500a63bb70a8 100644 --- a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c +++ b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c @@ -676,6 +676,8 @@ static struct { {"user_ringbuf_callback_discard_dynptr", "cannot release unowned const bpf_dynptr"}, {"user_ringbuf_callback_submit_dynptr", "cannot release unowned const bpf_dynptr"}, {"user_ringbuf_callback_invalid_return", "At callback return the register R0 has value"}, + {"user_ringbuf_callback_reinit_dynptr_mem", "Dynptr has to be an uninitialized dynptr"}, + {"user_ringbuf_callback_reinit_dynptr_ringbuf", "Dynptr has to be an uninitialized dynptr"}, }; #define SUCCESS_TEST(_func) { _func, #_func } diff --git a/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c b/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c index 82aba4529aa9..7730d13c0cea 100644 --- a/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c +++ b/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c @@ -18,6 +18,13 @@ struct { __uint(type, BPF_MAP_TYPE_USER_RINGBUF); } user_ringbuf SEC(".maps"); +struct { + __uint(type, BPF_MAP_TYPE_RINGBUF); + __uint(max_entries, 2); +} ringbuf SEC(".maps"); + +static int map_value; + static long bad_access1(struct bpf_dynptr *dynptr, void *context) { @@ -175,3 +182,31 @@ int user_ringbuf_callback_invalid_return(void *ctx) return 0; } + +static long +try_reinit_dynptr_mem(struct bpf_dynptr *dynptr, void *context) +{ + bpf_dynptr_from_mem(&map_value, 4, 0, dynptr); + return 0; +} + +static long +try_reinit_dynptr_ringbuf(struct bpf_dynptr *dynptr, void *context) +{ + bpf_ringbuf_reserve_dynptr(&ringbuf, 8, 0, dynptr); + return 0; +} + +SEC("?raw_tp/sys_nanosleep") +int user_ringbuf_callback_reinit_dynptr_mem(void *ctx) +{ + bpf_user_ringbuf_drain(&user_ringbuf, try_reinit_dynptr_mem, NULL, 0); + return 0; +} + +SEC("?raw_tp/sys_nanosleep") +int user_ringbuf_callback_reinit_dynptr_ringbuf(void *ctx) +{ + bpf_user_ringbuf_drain(&user_ringbuf, try_reinit_dynptr_ringbuf, NULL, 0); + return 0; +} From patchwork Tue Oct 18 13:59:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010569 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 CDF8CC4332F for ; Tue, 18 Oct 2022 14:00:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231374AbiJROAS (ORCPT ); Tue, 18 Oct 2022 10:00:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51532 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230389AbiJROAF (ORCPT ); Tue, 18 Oct 2022 10:00:05 -0400 Received: from mail-pl1-x644.google.com (mail-pl1-x644.google.com [IPv6:2607:f8b0:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F29687E00C for ; Tue, 18 Oct 2022 07:00:02 -0700 (PDT) Received: by mail-pl1-x644.google.com with SMTP id i6so13880702pli.12 for ; Tue, 18 Oct 2022 07:00:02 -0700 (PDT) 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=kMwTFgRUP580axj5yBnU9+zFqX7iQ7qQm+gg72pN00U=; b=nrfJvUfqAZmFHEqcOJqzxVeuQ1/tnemles2fBA10V3SS931SoqGBuHAnqCk444sMN8 RDmLkMElB4tVXtUt+iM7qMKHajn/71tuFZwvKpx80jusuN5mQSPa/tg7Tc62aeHXTTig F0P0+0Ba957jAcAuIXv7k6nKdQV5jiVQGzyFwBpMcBbKBFl+7N/eEA0+SF47AwTXyKXX UT24q2xBrDmAkSLhVj9a+fZsOOSANKlq7ZqQ7Db8Hz3dMG5CpTdYYBQMUeZyK1FihFBh d/rPYzPmaiB/q2EGyhdIR9+TQUvVprYJHtg/UMMGx8gW4Xrp0RhW4A2WzebudGkoR/ii Sy5w== 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=kMwTFgRUP580axj5yBnU9+zFqX7iQ7qQm+gg72pN00U=; b=iOy/YiAybF+CyyhDKa+KdJ319gCns2t5ZF4wSaDv9SxoJznlOCZ45v51odk0a+NA43 Boa5XMt2ZAYaQgaWMXfWsAL9CMAkbyRXq+MafCQpNR4+YuAsKW57xayes9khIR1ZfNfZ cYGFNrB+HUWQ2UjljK4XAjIsoIln8RdR/6ZWHrH4vol0+HWJjzCzrd7fkGDz80O/WFvU IYhiQJkcWedejtB6IMT1Xq5fudKptFvCrkNfvbwuUi/ohwNp/tEJpunpLjCsLfmZvPf+ 9ILUu/L1YdhxwvC+aEf7nCeEGR+u8jD9MY9xifkH8Zer65ykipvDav3cmSoph/h64CZ2 WfPQ== X-Gm-Message-State: ACrzQf3Embiyf0mZmi3up+eurYX9i1O85bT6k0E1iWu6U1BS8fdOhb55 Pib1Xv1EZVGJhUlkZ8wdcx7r7suYdzTI9w== X-Google-Smtp-Source: AMsMyM6MgBo2+79EGOSN/NlitzAShAboW79nTch2GkGhoQVGUiu0fQ6+oaQ2AiUyVgFpE/SSnN69gw== X-Received: by 2002:a17:902:ec91:b0:182:8eca:ade2 with SMTP id x17-20020a170902ec9100b001828ecaade2mr3150125plg.12.1666101601770; Tue, 18 Oct 2022 07:00:01 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id g8-20020a631108000000b00462612c2699sm8139942pgl.86.2022.10.18.07.00.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 07:00:01 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 10/13] selftests/bpf: Add dynptr pruning tests Date: Tue, 18 Oct 2022 19:29:17 +0530 Message-Id: <20221018135920.726360-11-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-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=1+xaYPFvhSVh1B1HYsq6/v3Prh+5k7T7cXmAvII84mA=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEiPAEB6C2K8pH+r5ktTXNziLVqwMCWX1BDetV3 c3M3LRmJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8Ryn1GD/ 44pHjkwfn3UAXiOH29FsqaJ45G0uxh5OAwKC35TpaeqNxgEP6j17k0Z3ruDC1D6eahSvEB47SIFxRW hPEVS/HQX0ZnYMisc/WvYH9/x5nslF3/mOev3JHXC0hR4xkp0tsNm6cV7gQ0UQ9MfzWb9T/b1u2qYt ekX/pkbo677GUZhM5DZNbKRvyUUE7zR8qIs43gmVX0GnFTk1tMv2GplQQd/osbp072WToOp3dL49Kc dGixqhkHmfouXxc2ehH6Vojvme1yesxS0I7ppoaF8V6uiihNuHdee4J2hDrcoXDaCgHoNUXctZf3SP W6chb7eAEQsP4FOU6JCea5RIDPZnI8OkwOAErC863Y13zd1/8DVgdV6wjRXSGsYmSn0QCaakChIRcC 1LRz1TbE9CJNHNTVfP/xwCpLMTbnYf285Uu/2wPpe265LJ61E9KDCulpTMCQUMK41QmyTj7xwr7JXH ZiWSowSnkDZUt+yeaF42dC0lb5WvTIPsSosGDHexqVhn4dcaSIrkwI0LQcC63WL5Y+8LcptoSkp0Jj utfhWHUkQclG+Z6gtZmam0F/OgO11JD7KRFsj5P8H3PErMwXfXjjFheLLl/zyn8TfX+5Q9y/AROA8D EWvqbjejbfjTvHEWv0NGjfaSNPgj+iqdn4pNXO4kuPGOBD7lkqx5OSv7/90w== 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 Tue Oct 18 13:59:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010570 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 C5A8CC433FE for ; Tue, 18 Oct 2022 14:00:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229998AbiJROAT (ORCPT ); Tue, 18 Oct 2022 10:00:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51776 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231298AbiJROAG (ORCPT ); Tue, 18 Oct 2022 10:00:06 -0400 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 780127C30B for ; Tue, 18 Oct 2022 07:00:05 -0700 (PDT) Received: by mail-pg1-x544.google.com with SMTP id s196so12085676pgs.3 for ; Tue, 18 Oct 2022 07:00:05 -0700 (PDT) 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=fPT/xLfKFwrrU4SwJxKlx5kwl9p+8LOa5HNWJZWwq8Y=; b=oe9RVtZAyBl7MVuan/eUuIBqvHxSc6t9XvxK2aNTv4YQ0Bxa19I9h9mcDg1IZHzQXi 6yVeMw0pmpoDp8gpE20OKMjJ6lL7a+VGcefi91D7AP8S7ipiK2FBXVFZhCWTxJpnd7R9 44+5utvdbcdcQoWxTcFfwRA+dHysyxvmkC1/Wb8xxlDQDjRToaodspQKv5K4oNTJ51xh 67imcKRp4mNwZVUwB2JBwpn/c+Ad9MM9L8eCoNGICi4M0tLtoplK7k4yN//KiW/+7l/E Mv24+Jjs+PeL0ndeedyhvARbqZMAI8/E9GAWLUxFjOAmIBdwCOLpZ1P+G9tuBPermqq+ OVRA== 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=fPT/xLfKFwrrU4SwJxKlx5kwl9p+8LOa5HNWJZWwq8Y=; b=rlQgONBKhri0dcSu+vtvlqt8lLKi9c5qT3qMDLq+vo6Fzs70inNS5illGv/6PZ7EBf oAc9MgNZTr7eGvdwtG0vWFDe9QJwwm8+0a2mMxAbjlQjLYMUbPlhvK2o4BoSPvLf3zw7 BCoz95+nuwJAjWvwdISRbWJzgTQFLrpYiQBCzpKqg42p0GZ9DytaeC1sla8omE4sKy6R uL/NMZ/tNYn+AiajRfkbz57Q4QMkhJc2Ll0Q3zSULEN1Czv3U+raSFTpCRm78x5QG413 IW886iE8aBtFZcsPBKOYnvt055Xrn0C6P0CEvZPQmLuYpO23WI3RKPO9gvjgc+G0714m HFKg== X-Gm-Message-State: ACrzQf36ucVVBFr9x5urr+/kTnW3k+Sx5h/JciYg3u3HXaJYHTZkMNey e+NbT6z2aICKwR29nEn8l+niZ8Q2fUtKfg== X-Google-Smtp-Source: AMsMyM56e5asRNUc/4hEuXI8Yu/eXQ+57HxgB/CWxlxAID/aRFotrnl761wTa1wIBi/z6+2t6lVNDg== X-Received: by 2002:aa7:85d7:0:b0:562:6079:3481 with SMTP id z23-20020aa785d7000000b0056260793481mr3047782pfn.77.1666101604747; Tue, 18 Oct 2022 07:00:04 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id i2-20020aa796e2000000b00560e5da42d5sm9136600pfq.201.2022.10.18.07.00.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 07:00:04 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 11/13] selftests/bpf: Add dynptr var_off tests Date: Tue, 18 Oct 2022 19:29:18 +0530 Message-Id: <20221018135920.726360-12-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-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=lWtxzrANAZxyKY18Vsxv4Bbip2ZoyfaVfH5dUk1VWJk=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEi9zSGeX57gcGd1S6tZ6TcqBjRE2nBrQm2pnqw lmHI1JOJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8Ryn3HD/ 0RkvB7V2Thg++DgspbVjkbOlQ4MAMt9aBkPHujK/6qXjEeg2Y8yTWJ0FolfW1xmI0R2kIwYHbERp32 uCO/AwYAL6B8ddBU0ueX0s4dptw7RZeq5oOi0inryrDhXQcXZ9pdJV5ab/UH4wIA1TyBRqMMJTzNGk k4htyRyAbSMFac2olvR1bp2OuOzDfEYvoIZUBv18E1CPice2q+Urx/4IxqUVv6mS4tKj6wz8OcEdSo xJOPxxCRvrDtvuy8cNjmh8z3ixk2Hkg0mQSWSP+cMZlmhXRMaZVDXi4RSejtfZwY97Fp1Km8e+O5jf UiYMYn5ZNpnofTkzrgY8VKo22ksgjIBnaOUWLmSJNSQbQgtSxy0U58MxEjx9mQIB8oLkixBvEShfBv pT5vVMYKUKMH9q2MzOeE/6LdA+QqcNKbjqvh+84is4gdQdjKApSIUpjfkZ6y7gbRWalUeyxKJVB3PE wyX+k7GXkM2CXk63ZYIooHxagrEJZEyTjGIxhjIePwKnC0XCFiaSMu5clA/PhtyxB0OQLUP2aKlK9/ MrdkSesN1jA3gTNy1B6F56T1c5ae9iXEaPcVl8B9L0/eeC78gpOtMczyqUG09GzIc3eEgTJQiMi2jm g/X6QCL2HMM1UmS4OB6IHEnD09rKXsl9phz/aboIp9CxhLZ9S1czoipdlXVg== 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 Tue Oct 18 13:59:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010571 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 3D249C4332F for ; Tue, 18 Oct 2022 14:00:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231348AbiJROAY (ORCPT ); Tue, 18 Oct 2022 10:00:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52734 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231362AbiJROAN (ORCPT ); Tue, 18 Oct 2022 10:00:13 -0400 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 8F03989917 for ; Tue, 18 Oct 2022 07:00:09 -0700 (PDT) Received: by mail-pl1-x643.google.com with SMTP id o21so11502936ple.5 for ; Tue, 18 Oct 2022 07:00:09 -0700 (PDT) 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=Ag5Q1CLOoCiOe4E6jcNn0D/qd7Ptx/xklL9ZKfmmK+A=; b=qhUjJu+zD1mkiVxwGYIonMV5bFwn5kce1chAoBi4/BD7rWjQExtrcuI1Ab/JFYHFzx 8Myj40S1zxPqPMdHbMviwOOCmtcFzkWx9PFfiDGOZh82ZwbEiGaacbnc3EqtAme/BASm YtWYikKTyILD4rE3eScqJjkecvj5t+XBdsGZy/THcoEJBrSv9P2HSE6klvwg1SDFuyqO OxsBtW8JZtV4gFASocwwK5iZmAHQhEZXY8JXU8QDTX17bSTiobiIDHc3TeerWs6s090j Ry9aLxjni+Dpcy8HWGIF9SrGjIxdowjFVEBT9Kt5v5nvqFBb+dDs2uQ8OPXyZFOjC7dj WPag== 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=Ag5Q1CLOoCiOe4E6jcNn0D/qd7Ptx/xklL9ZKfmmK+A=; b=HA7UdhHgdtB+LAQ5D/pOK9s2e6ehVbi+LRUqW+C0qfmV7u4CTLiDRRlAVGJk8wqt88 PDZRhxwSScgNb9Sm2kCGILUPWkeMXZju0yt7TFXmE69rDIrEdPvuqqa6wcNCcV6nYVKP k4jPpJMI87sjHTeG1bG7yNjDdDTiZyr1QJKmzyISeQ1t9/mZVHGc+dWeq9w8BUy0gaj8 0xExHMcQ56r9CVfuT6lmq8LMsyqEy1qn7cWgCaIF96y21WDusMjwJ6LZWbpfDd4LfycO S5tMIgnvTS/rVcD/UF30zTHeHBtuLIRaR/6x18c8KEccO/V44Xwe7da8CUlP6PZeAjht gzYg== X-Gm-Message-State: ACrzQf0Sh/lZ6x8Hu01iHk//ag+E/v6okf2hkpjvUeYx5iKaMEp0d4bl dd3ZEESgkXFpBfAxwhTh0U7NaphBCjbKSQ== X-Google-Smtp-Source: AMsMyM701Bvaj0M+7dOsyvshR6cbSw/3T3zyDuY3p4d/4fMvQh4O4Xye/MyXjgNWdsnfaUXBO9RXQw== X-Received: by 2002:a17:902:70c4:b0:17c:f9fe:3200 with SMTP id l4-20020a17090270c400b0017cf9fe3200mr3181546plt.1.1666101608156; Tue, 18 Oct 2022 07:00:08 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id y17-20020a634951000000b0046a1c832e9fsm7984183pgk.34.2022.10.18.07.00.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 07:00:07 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 12/13] selftests/bpf: Add dynptr partial slot overwrite tests Date: Tue, 18 Oct 2022 19:29:19 +0530 Message-Id: <20221018135920.726360-13-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-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=n6R+Kc1fES01TnB4BRh9DyPo8cyoSpbfFQ92Z+ogpQs=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEisz4QmcCQCAZzWRRCitFtyC0TyN/Ktw/4AGj1 M8MjLPCJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIgAKCRBM4MiGSL8RygqLD/ 9QfwP93D/WXJFoPCG3bJPrZyXwpZ/DgkKrS87ShMMX16qX8rFaKRL6SlKZ/+HNj0sguzTVGFZSX1Qj vKHTJufxj6kKnwIn8TwCQakxRcDqiLR9909TqGxrN2/lBbxrDTO/RcGjwdWmbjkgWwUBQJuKlJVI6s UCqoTnkJ4Aqt0mUmaZ08lOo2ajw9tXRhSMTDa/G2O5OUJZdqYmk4Lhsomawg2kdHMJcNEsquoQY6at Gg+wjcfFQE+in1+n7U2/dz7FXB9LozMPMvrIYe+hpzZpEaFVoxXZHy9Yc7fM1Zze4iTWkvObFGLA5M Z4GTox0VBSQ1a6AXP3eYFrtUXEdR9mXK+of8XpMPa5/7nrQ6K7eAQhX5RLVM+kH6FKJf6H/JCKGsXQ V6QUAkHISdFaIkZLqdMen8lvOjfjR/mfS6j3nZiyRD9nKKmAZ6cN6wlhPlWWMeOd9CfY1Lanx16T47 cvZ7ik0WleotXZHINejIn0YcaNhE2GDG81b8e7mHOfnNsSMHMwjbFP21vg0Tq3TFHxWN8iCBP57rvM eQXBpjK8Ej/h+ZlXRaOAZeP22JTCReQ0YZUFqVhC3Yk1dtiLcy4GVJhNRnOa2p3j9b0ww7ilcn7Ozc CgqMJmJF0QL9pCDFJPvH87rjROSCx2g3jOWJyVImKfcMI+HTnrBuB/kUt9JQ== 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 Tue Oct 18 13:59:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13010572 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 6A4FBC4332F for ; Tue, 18 Oct 2022 14:00:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230185AbiJROA1 (ORCPT ); Tue, 18 Oct 2022 10:00:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51544 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230359AbiJROAS (ORCPT ); Tue, 18 Oct 2022 10:00:18 -0400 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 9B1268A1F0 for ; Tue, 18 Oct 2022 07:00:12 -0700 (PDT) Received: by mail-pj1-x1041.google.com with SMTP id t10-20020a17090a4e4a00b0020af4bcae10so14061347pjl.3 for ; Tue, 18 Oct 2022 07:00:11 -0700 (PDT) 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=C5hKkta8jKAlESTBm7K0poJIgy7JPyEhWZPuv6W2TV8=; b=MurWAKhh2HwRuHRHx/Em6wUlGCZszsZIvRkcjSu3Q2RWApzNe5IKedPthMwrf9L8vq l1f8u8FzF37DZCjwbYlehPXABariYTzVDhKqAjNSMCdudNkPQw2CBWvYDeZZfk8YI3L9 e7khW4ur1+opT4ER4Tb0RDgN0rsEF+EeZA4FW2A5I7YuG43Mf3ldL7q9ZKkGxmJO9RrO /Jp9rTdIlL3SyM7FoFJDTHdBpJOPHcdCaKKIPqB+J1GwoNNFkSb4QH9I/GTvF9F57WVY 2UXKj/rTzhC6Emv644V/tdZpHTFYyyFM7Zd6SqwSs3UGFvGEOWNYrGV3XS60hQpG1Pqu AQKw== 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=C5hKkta8jKAlESTBm7K0poJIgy7JPyEhWZPuv6W2TV8=; b=KHJuBkSZ3CRlhJtZJZtNFtXKfi+/dr3k6rEMTFCsLK5gpIO+vJ2CR43yzIvoOEU7BI wIF0FLF+OXtnb6grIuJFiVlK6dTtRrAbJDHAgIXldmND5lSSbAz0A5Ff5Vh01Dq5ugyW wqyUFmdFoTs151blN0OgL2m7yBN/MbiLRGJcBY0Fgfbt0B8dFks+PQ7uQ2a7IA/SLVa2 TJvusUS/yoE9p6wkPjciaCi3uU1zdRvnuB+oM/05so0pUlCf+Gab3mkufza94BK/o61t xUUomWC982dGhMaF4HyvuMLHGNmhdC28R/vAMrdCjH2yM+dzwAodEBJrd45erF4ci0zX ViBA== X-Gm-Message-State: ACrzQf0tUBc42kPZoEKZkGiSIM6Di2lyBgn8o1LgAgUXBO/fgVFujPpj /Bh5aJtrpve39Y/2OAyj6VIYB6iTElM9AA== X-Google-Smtp-Source: AMsMyM7tUOdAo5BDodjJRSUVGsalAuAh/zV/enKFCnRQNjR5eRGM4/ulgV1oczr8Titd3MTmLDa9aQ== X-Received: by 2002:a17:90b:2d85:b0:20a:d20e:a5fe with SMTP id sj5-20020a17090b2d8500b0020ad20ea5femr3704508pjb.96.1666101611016; Tue, 18 Oct 2022 07:00:11 -0700 (PDT) Received: from localhost ([103.4.222.252]) by smtp.gmail.com with ESMTPSA id l9-20020a170903244900b001754e086eb3sm8818986pls.302.2022.10.18.07.00.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Oct 2022 07:00:10 -0700 (PDT) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 13/13] selftests/bpf: Add dynptr helper tests Date: Tue, 18 Oct 2022 19:29:20 +0530 Message-Id: <20221018135920.726360-14-memxor@gmail.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221018135920.726360-1-memxor@gmail.com> References: <20221018135920.726360-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3180; i=memxor@gmail.com; h=from:subject; bh=eMjtMuXO5rdwYTSgrRC3E52QI3/PPyZSAswoySqEGZY=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjTrEjlt03T3P4YR/mKsk4bZzlJtwLybR0v8Ar5UOw J+/BBcuJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY06xIwAKCRBM4MiGSL8Ryt93D/ 46OavxunKYHWf+txRq39+musyJGIXzYeehyvZBL4NgIGIIxx4M3/ACmT77OdQinSQtvqh1PRo+vYnc f9WGPJuuAPcXzv2ZuKSrKF4fF46AgnKX2cJgBuN2DIe4ne+4o/oEBvSJG8GVe+K7Ra8JXyTXslpPZZ JGoFnFyDwQTKSxZh306B+69xXRvrVVam2kFjCt8XOGSMwWsZzWka/1nsEJTxfqIHIT7D558TmwX4Lz 9lbLa5Basjzrl1amAcSLWSRG+bOeI9HsMLSjzj+ol5JTvPoEt8zbexH4XS6FjI8exauruKxwMZUKpC MlThyDMD6i7p1FFvw3WZKErin3cU2LGv25J4jRYx2KSpOXikbL39sWqx1ceakXgem0nXWcHsAEGkPA QxbaCo7qALDSJRwgclBENfeTm/oTgqDiLB2uqNr75G6I2/e870J/YK948R+sNLDajq+dPdS0y359sA 9G23isOJYc2ZIuzpG2hxTtuiNSmJXxNp0GGXrn6EVPZR3KOC5mfrfN7KEfdQioJ79ej/hddPdyBS/7 KlLzAp/AuJTMoxg1onkrwoW15Kn9kabGe5j5W1uspiqyxJGyf46Znz2CcFpMLB7soGzSFqiWDetwyv zkhSee4PEDCf9jss1ypCRXp+6t3Jo+8vNF1ALqunRNGvQ72GylB6qd4X9JAw== 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 Test that MEM_UNINIT doesn't allow writing dynptr stack slots. Next, also add a test triggering the memmove case for dynptr_read and dynptr_write. Signed-off-by: Kumar Kartikeya Dwivedi --- .../testing/selftests/bpf/prog_tests/dynptr.c | 3 ++ .../testing/selftests/bpf/progs/dynptr_fail.c | 35 +++++++++++++++++++ .../selftests/bpf/progs/dynptr_success.c | 20 +++++++++++ 3 files changed, 58 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c index 947126d217bd..20910598a0a6 100644 --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c @@ -42,11 +42,14 @@ static struct { {"release_twice_callback", "arg 1 is an unacquired reference"}, {"dynptr_from_mem_invalid_api", "Unsupported reg type fp for bpf_dynptr_from_mem data"}, + {"dynptr_read_into_slot", "potential write to dynptr at off=-16"}, + {"uninit_write_into_slot", "potential write to dynptr at off=-16"}, /* success cases */ {"test_read_write", NULL}, {"test_data_slice", NULL}, {"test_ringbuf", NULL}, + {"test_overlap", NULL}, }; static void verify_fail(const char *prog_name, const char *expected_err_msg) diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index b0f08ff024fb..43a0ed3736a9 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -622,3 +622,38 @@ int dynptr_from_mem_invalid_api(void *ctx) return 0; } + +/* Reject writes to dynptr slot from bpf_dynptr_read */ +SEC("?raw_tp") +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") +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; +} diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c index a3a6103c8569..401e924b15a0 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_success.c +++ b/tools/testing/selftests/bpf/progs/dynptr_success.c @@ -162,3 +162,23 @@ int test_ringbuf(void *ctx) bpf_ringbuf_discard_dynptr(&ptr, 0); return 0; } + +SEC("tp/syscalls/sys_enter_nanosleep") +int test_overlap(void *ctx) +{ + struct bpf_dynptr ptr; + void *p; + + if (bpf_get_current_pid_tgid() >> 32 != pid) + return 0; + bpf_ringbuf_reserve_dynptr(&ringbuf, 16, 0, &ptr); + p = bpf_dynptr_data(&ptr, 0, 16); + if (!p) { + err = 1; + goto done; + } + bpf_dynptr_read(p + 1, 8, &ptr, 0, 0); +done: + bpf_ringbuf_discard_dynptr(&ptr, 0); + return 0; +}