From patchwork Tue Nov 15 00:01:24 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: 13042984 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 A0E47C433FE for ; Tue, 15 Nov 2022 00:01:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231790AbiKOABk (ORCPT ); Mon, 14 Nov 2022 19:01:40 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54580 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230415AbiKOABi (ORCPT ); Mon, 14 Nov 2022 19:01:38 -0500 Received: from mail-pl1-x642.google.com (mail-pl1-x642.google.com [IPv6:2607:f8b0:4864:20::642]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D93C4F3F for ; Mon, 14 Nov 2022 16:01:36 -0800 (PST) Received: by mail-pl1-x642.google.com with SMTP id w23so5175453ply.12 for ; Mon, 14 Nov 2022 16:01:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=kpg5Qz+6fWOX2DvxSu0D8LB5VPcvtrAWoVCahsyCIi8=; b=XK9ZG1XO2+KITQqwyxZlfpC0xpPekKzVgOBP4OVxFja1dSZPfwBKE9S8eVtVLt1pbg mI4se1u3d4s22yeuAD+R71k8V2g7kgOLmPkOm1D6A2Ix3fu9e4FVibEBrOGq1MaKDUv+ dbuBsB00Gd1RFdcqe9v3MiPejY0cqEAHMWSXiI2SYs0jf0ETpwwUytA0ablB1LsD9u7s Co6t5iG29SDsXJe9WDsIZ+18UxP4UI3VohpQh5YfyEHq6TR0yasVlYtOEh/p12BzrMK9 4UxPt8PfYBIpRNKNxX8/pDHipfl9bt2YCf7NZpKU4/JzCY7q7EqhA5op5V2da2IcbjoX 45DA== 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=kpg5Qz+6fWOX2DvxSu0D8LB5VPcvtrAWoVCahsyCIi8=; b=6L2EWTD78D7x4GJivdjWte/7QABtV0yt2gNjX/OaYK7pfbRVR03HSQ4DfRkjaleOOE BOTWnzzovn4ajZX/Y+dIfAWtieiWfNDnFilc6tmpD5IZg+OoAABUnHUzdcVxebDBlS1w qoT0x51kfzs+tyHLZRct9f80KkJA/o7WRQTN6a29FVD/OJXDfGCp8euNZylcKLDhx3x5 +L+xgX8ICnLxkU2atOlgZEM/z1/50eiPc4sLl638uQKPi1VtRaXRgnEjdGy1C3QhaXgj Uf4605JPljkb4Mc4yZxxeDzmxlZRjuvB9CfIhV5MVjXuoJblVprSEAV7FyaxzHRsExOg gc9w== X-Gm-Message-State: ANoB5pnESCrfFVXDs7UL5sTj25dW/Sxjl1NNYtCT1jJZNy6KYLG8QN5L ZHcuTOGkXl894qKMZ0xby453yybCGZ3vFA== X-Google-Smtp-Source: AA0mqf4T95fp0TCT7/7qkPKynmPcK/XOeekuH2LZsVtZ0eOnBT/lx47kYwWNvgJbo0H3IrqdCWBr3A== X-Received: by 2002:a17:90a:55c5:b0:213:4f59:9646 with SMTP id o5-20020a17090a55c500b002134f599646mr16105895pjm.116.1668470496073; Mon, 14 Nov 2022 16:01:36 -0800 (PST) Received: from localhost ([59.152.80.69]) by smtp.gmail.com with ESMTPSA id b129-20020a62cf87000000b0056bad6ff1b8sm7292624pfg.101.2022.11.14.16.01.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Nov 2022 16:01:35 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: David Vernet , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong Subject: [PATCH bpf-next v1 1/7] bpf: Refactor ARG_PTR_TO_DYNPTR checks into process_dynptr_func Date: Tue, 15 Nov 2022 05:31:24 +0530 Message-Id: <20221115000130.1967465-2-memxor@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221115000130.1967465-1-memxor@gmail.com> References: <20221115000130.1967465-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9457; i=memxor@gmail.com; h=from:subject; bh=5P0Dxb9qxhITlYE11NEHLrjKbsu1NP1KfSCZ54b+Fz8=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjctaC8TbYxYNI/U6La7PdXe/Py873iQSsOoyNBk25 ohjAKkKJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY3LWggAKCRBM4MiGSL8RyrjbD/ 9cZa7Q1ACXMafgVANipfJnHf62udr//8K7diWbey8F9U0TcaCMuVtwUWa2BnmA16qi+D06YBEF6EXQ CJ9uEo9PrloDqwICgWYvidlANH3Ev9YBxqz7/oGPr7zOGfBgRH83fHUoPRJg+8b9PvufRYNbSCeEG4 u7vq0BzMnHZQyAl3cZX0b5q3Mj72v28PCAPCaqTRSg3mshKyMGFA4nrjfXy2g2YgUMswtbY/U8UEFp gk9jhzSviBegnjO/XTgegg1wgRs9XfJpKbiRq75daSsHFmUJ3h7GEqK8ym5CAZC1UlraY2LwbLMC9h +rlSj3GZ0rUOkP4njNqNzgEEnxt+xkZiztYh+aK7Icxwz7jRFHEsNlWuX6ghKmFJQYNXV+FxFUuApH E6vkaKTC7PJDy8+3SUyHlg0Axf7lq6BX+YEcKt3ySZwOVnFdHtI9smFJYUq9OGWlsy2SU3+1j0sOS7 AtwqJIBoi0pvtIp6u7Dsve9Lk1LQNDj6TYmVK4/E4a7xG4hLq+asRbdTRdaDlke4InfnXfVckaSLIU 0D14k5+X8GS3fduPzQx9OddLNz/u1uAaQR2TY7YPDS+gnK6JEEc7d9w0Jdrnr7ehET0+Xp25muvs6t rj5UYT6JXKGOZlwU+Z2s6uGclnNpLDewp1c74aMtmBHa+9VycKqmrvpUGpAA== 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. 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. Acked-by: David Vernet Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: Joanne Koong --- include/linux/bpf_verifier.h | 8 +- kernel/bpf/btf.c | 17 +-- kernel/bpf/verifier.c | 116 ++++++++++-------- .../bpf/prog_tests/kfunc_dynptr_param.c | 5 +- .../bpf/progs/test_kfunc_dynptr_param.c | 12 -- 5 files changed, 70 insertions(+), 88 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 1a32baa78ce2..a69b6d49d40c 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); +struct bpf_call_arg_meta; +int process_dynptr_func(struct bpf_verifier_env *env, int regno, + enum bpf_arg_type arg_type, struct bpf_call_arg_meta *meta); /* 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 5579ff3a5b54..d02ae2f4249b 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6575,23 +6575,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, 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 07c0259dfc1a..56f48ab9827f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -784,8 +784,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); @@ -804,9 +803,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; @@ -5694,6 +5692,66 @@ 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, + struct bpf_call_arg_meta *meta) +{ + struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; + int argno = regno - 1; + + /* 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 (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", + 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 || @@ -6197,52 +6255,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, meta)) + 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 Nov 15 00:01:25 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: 13042985 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 0FD26C433FE for ; Tue, 15 Nov 2022 00:02:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237668AbiKOABz (ORCPT ); Mon, 14 Nov 2022 19:01:55 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54592 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230415AbiKOABk (ORCPT ); Mon, 14 Nov 2022 19:01:40 -0500 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AE9EC60ED for ; Mon, 14 Nov 2022 16:01:39 -0800 (PST) Received: by mail-pj1-x1041.google.com with SMTP id r61-20020a17090a43c300b00212f4e9cccdso15328759pjg.5 for ; Mon, 14 Nov 2022 16:01:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=bPS13dYNuF95/UnFyu3fB5t8wmLAjV6BId42u1eCjj8=; b=JDV7cNezr2C5AIeeS4jG81im9yafoikbj9wuqTCR4jgJSTEgwx9Nd84rjLwtsQHSbX m+2uzNtwnCJGq7s+44ETPHo/rA28QIHBHguBqncINxvOfVd+uwbOQHHGNncvzhMYV0N6 cDtL8RR3dqCG0RkpGh5O5z6IOm5naocrncke2m1SsrYm7heEYZuMMaoXvUeGlgK4bNzi sfcAEh4Nv5gxNQI9g5taErOhMT8sgq1xBX+RNe4m+qSB8WnJVSOf8sA9ty8TsIt+DL4X Oxln3TKy2L4EOhH02asl+Jgg6sNubVwxaoj07wyBm28bagj3ldiqOiVrPLzzKKWTj/xC kBUA== 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=bPS13dYNuF95/UnFyu3fB5t8wmLAjV6BId42u1eCjj8=; b=Pjt9oWbCqRxZiKbponavUVd00TEwau5p/vjWf2xjm+f+Q42gO2Utk+b+UPvlTbFXsI hG6YEhuBaC3TjiCAjP1AnFMTWTcABZfmlRaX3kUHtaZoMX/+lOCqutCzFU67ZBUeaD1S RaZ2ngW9mtvvmWmuO6CGq+MvbFQlFeCuNqkDq1eC8tfbJiuEAwxEcvWtBXGXEIxC6XQR dZuwtcIUQ0XdkeWNE72fWGyToAH8+/P1DIERCjiun8Fn2uMO2jAf/NQP2Cb/vZWjcT8o 5Y0i36QMq7gsKVQLwNZIN9zGplfJrnnG88cj4L2pvUlbg8Jd1I9u8ihXXGD7MXkXpuxn 3UiQ== X-Gm-Message-State: ANoB5pnM5NskqptFouLdQ4ND7rpQrkZurVkT89yKqQ4jHakP//G4hFys 810dKy535oKtU9jbt6O+Idw5fsuBdF0cBg== X-Google-Smtp-Source: AA0mqf4eYmwwpIsdzkwxDQYIDMNRZWAH4b3jdVA8q8DujnhsPX5flPdVvU+B1meZ7Vghuuc6oZ53gA== X-Received: by 2002:a17:90a:b894:b0:20a:7294:47dd with SMTP id o20-20020a17090ab89400b0020a729447ddmr15208720pjr.207.1668470498982; Mon, 14 Nov 2022 16:01:38 -0800 (PST) Received: from localhost ([59.152.80.69]) by smtp.gmail.com with ESMTPSA id t6-20020a1709027fc600b0018703bf3ec9sm8155152plb.61.2022.11.14.16.01.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Nov 2022 16:01:38 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 2/7] bpf: Propagate errors from process_* checks in check_func_arg Date: Tue, 15 Nov 2022 05:31:25 +0530 Message-Id: <20221115000130.1967465-3-memxor@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221115000130.1967465-1-memxor@gmail.com> References: <20221115000130.1967465-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2133; i=memxor@gmail.com; h=from:subject; bh=HjZ8n14OOOglk/GCqR2IboCzYEj0fP9YCcqR8T20538=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjctaC5wXp4g+aRZb0CeKDmgilfOm58S1Jtm41e5WG 3eiReXuJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY3LWggAKCRBM4MiGSL8RyqZ+D/ 9o09168Vw6GJdLfJ2KB7FseMuCHZQ4SAqyA2AC+xyYQQp2TJ70FIgqcud/b7rDB5AWJjaJG8W0IYNm aeD80aAnnK7MuNdftKxjzNyxDJrOhu0dUvfIevs0YWksaHN73GDIXIbVq+W7XYSGkrB7kDkhmriFrW E830acQd2T+Oss3BZ5zst7zW4IJtDEMVCzrVxyHUWUZXW/MbzgUnwFGxiwEEPVL+y/H9arBdw6w6kg bkT11TeMXrXe5n9hbfEir4Zunl22GnLxoKiyM/5W/olx9RI/GgZmIe7N9BVoX2RkAMmx1dRJnixatq VmLFsg0Sq9EcVt2wvv0WtLDlVFIy1iv3t3HWLuIb9MYaCdf3AtrjkLywTwBpawhPMKu40lCh0oO8G7 dtaThNUnUZC+kKoKP7al2zrvZLGYQ1lGBjl70MNNyiHZsK/Vm6V2O4Dmv6q3c5cUozt1hI/MsHju7b f/g4sEXmHM4tlvHthC5GOS9sAPxCmvA3LNivg63c2+kpukagzq+QgOhjc4X8SXwylEWNZU61alSt53 u8HwJ1yuwWhAaL0opthJ4U4cL+hN0D3PRZ+M0WP09aA8TJxU8j7E5vildM7EkGdciHZV0ysB5n2l2/ cVlWalU3uiOeg3Q6SNVrhl7knouUWG74rqjDejbPg84WbbfIvUPxsBSgOZdA== 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, we simply ignore the errors in process_spin_lock, process_timer_func, process_kptr_func, process_dynptr_func. Instead, bubble up storing and checking err variable. Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: Joanne Koong --- kernel/bpf/verifier.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 56f48ab9827f..41ef7e4b73e4 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6220,19 +6220,22 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, break; case ARG_PTR_TO_SPIN_LOCK: if (meta->func_id == BPF_FUNC_spin_lock) { - if (process_spin_lock(env, regno, true)) - return -EACCES; + err = process_spin_lock(env, regno, true); + if (err) + return err; } else if (meta->func_id == BPF_FUNC_spin_unlock) { - if (process_spin_lock(env, regno, false)) - return -EACCES; + err = process_spin_lock(env, regno, false); + if (err) + return err; } else { verbose(env, "verifier internal error\n"); return -EFAULT; } break; case ARG_PTR_TO_TIMER: - if (process_timer_func(env, regno, meta)) - return -EACCES; + err = process_timer_func(env, regno, meta); + if (err) + return err; break; case ARG_PTR_TO_FUNC: meta->subprogno = reg->subprogno; @@ -6255,8 +6258,9 @@ 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: - if (process_dynptr_func(env, regno, arg_type, meta)) - return -EACCES; + err = process_dynptr_func(env, regno, arg_type, meta); + if (err) + return err; break; case ARG_CONST_ALLOC_SIZE_OR_ZERO: if (!tnum_is_const(reg->var_off)) { @@ -6323,8 +6327,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, break; } case ARG_PTR_TO_KPTR: - if (process_kptr_func(env, regno, meta)) - return -EACCES; + err = process_kptr_func(env, regno, meta); + if (err) + return err; break; } From patchwork Tue Nov 15 00:01:26 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: 13042987 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 B4411C4332F for ; Tue, 15 Nov 2022 00:02:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229452AbiKOACA (ORCPT ); Mon, 14 Nov 2022 19:02:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54648 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236477AbiKOABp (ORCPT ); Mon, 14 Nov 2022 19:01:45 -0500 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9162CA456 for ; Mon, 14 Nov 2022 16:01:43 -0800 (PST) Received: by mail-pj1-x1041.google.com with SMTP id m14-20020a17090a3f8e00b00212dab39bcdso15368771pjc.0 for ; Mon, 14 Nov 2022 16:01:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=Fkh6ySGcR4CBB8NB2GAwRubxemytbXtW9jjYmPwh09A=; b=ZYx0hfVAhLKfam89AznvsHlmAKUMv+kxO6OQ/Cig8sRCodp27RQ66a4V9riamEC2xB jUjF2/vSdqvsoCmGe/SYeWDLk0XjWeliB2FHi9VU/oD9Upz+0xI8Wl7YuGmqwFSke5bs 87tJv1cwkqTJ5cZ67CJdqw2Noa675hrLMm33dNfC/x9Fnjjqg4/WcQpRIuqMHLC0byYO QFLAJkxa2UjnmueW99nm+MQq5gfjBZaUIZVUj72skDADUnMkcd40VvvjHwE/QNWnZr1B CF7s5QSkrjEikLfXLDm3DpWfwj35joU6WCYpsKZ8m/5sBGw72mJc1buh+r/0tsaZsHSw r2bg== 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=Fkh6ySGcR4CBB8NB2GAwRubxemytbXtW9jjYmPwh09A=; b=w/lThJ4pd7RjmF1NzapJnMVWvUP4A5v8REu520kWXw7bilKTMSSpXbA8DSLfgkPbbS DYXYTJQgzc0k8DP+8N9Ajgr7XJRn6zc60QqulCZTenhColyGgok9eLp5lU2ynSm8XG7j 0X28KSORqCKNs86y3e2DXDtfxV25kgv9NnfyOq41I9ylYlGACEgaY5hI38u2fqD5+zEz laonPzJxle0wgsNsKQtgRUpSfn4QVllhEn1NKdVmGuccecm74ZHq5UdLWvNisDRnlLZU /DURrfhEj72ugkrvCU0t7kEDzspR6IH2qD2f7SDgbTpOcgp3GYtZV965uymx8YgOZM03 a7VA== X-Gm-Message-State: ANoB5pnIeO0y58iPSM02DwYUbR9wmf6zZAHlRmwOia/6loRBCbT8tmDA ddVGNi6kG8aqH8VRRQxK56mxClDd2DxLKw== X-Google-Smtp-Source: AA0mqf7uQJwqwKbdds0GO8NAh9UYiwKi7FVQRnxDIZtmvsWxZJzCoEOzzSa9V04QIUcpEYU5COlu1A== X-Received: by 2002:a17:90a:988:b0:20a:cbb0:3c9b with SMTP id 8-20020a17090a098800b0020acbb03c9bmr15769795pjo.81.1668470501960; Mon, 14 Nov 2022 16:01:41 -0800 (PST) Received: from localhost ([59.152.80.69]) by smtp.gmail.com with ESMTPSA id i2-20020a17090332c200b0016d9b101413sm8231303plr.200.2022.11.14.16.01.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Nov 2022 16:01:41 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 3/7] bpf: Rework process_dynptr_func Date: Tue, 15 Nov 2022 05:31:26 +0530 Message-Id: <20221115000130.1967465-4-memxor@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221115000130.1967465-1-memxor@gmail.com> References: <20221115000130.1967465-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=28721; i=memxor@gmail.com; h=from:subject; bh=urKKUCVnaot5/lcalotJRR5WRf3YQcioHwEYAmwhVtg=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjctaCDTTgamAFcbAJZh/WCrUyjxJFbibVg48cIQOU IIu05TSJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY3LWggAKCRBM4MiGSL8RyrQfEA C/UTtp/xlGpZCiBUEQ5Yzxlz3mObqCs6VMvjP7iybc7182miqf0t2YtWTqiUFCG9disG+83I1f7LWN LAtKjSMYoO76ZIqynxb+zSqgyOFtP5IbY8V6z/6mYmlixVN/Rc7L15Fbylr5MER1g7iEBOAk4DthgV efwaXP3LPO0D2PQLoy1BajwMnXuS3YfmnaIDmtgYZdjtdZak9J937gyYk4Ol8MdrfpxkEaWe+KGfSF a8Z01IKGJuZhb2j2r+8T+yQbJni96pbGSQiTY/hluWExgdiFpJmxjnA5rY0xRVktfGlJa+A5cdHj7C YXedxTcS7e5UitmJ1TQMs7PcDKz0G9IXZ0NnwhCQLchlS+/LVEs8UZHKuEldpiYkskn5xTIDn30kXw 7V7gXhQ7eEgSE2CPG/biquU/YrrKlLVTbJR4yK/B6HaXl353km2UYJVyr3T5H/67z1LlEa6cdss6zR WTc4DYj/Pu6PSpamRSIHOe7Lamfsw0DHfmcUBquHTgqsBZ+vo3qhmLY4WmJEe4+ATLw0FsnMxI6rfB Jh0Vsb2DwmafWaYjP3SwVFPM2QcrnmwR6lMJZs7lT7G/c8ovN8mK7EQzMipUC7Bu0f5V5yyohhntG5 hdteEIAwY0Wg7/2kNVFl4x/MmhQfmOI2qJiCAICyFWAxqYc2VBeSsjaZ2cBQ== 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 add 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 Acked-by: Joanne Koong --- 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 | 220 +++++++++++++----- scripts/bpf_doc.py | 1 + tools/include/uapi/linux/bpf.h | 8 +- .../selftests/bpf/prog_tests/user_ringbuf.c | 10 +- 8 files changed, 195 insertions(+), 81 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 798aec816970..dfe45f6caa4f 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -709,7 +709,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. */ @@ -2761,7 +2761,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 fb4c911d2a03..b7543efd6284 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -5290,7 +5290,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*. @@ -5300,7 +5300,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*. @@ -5310,7 +5310,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. * @@ -5411,7 +5411,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 d02ae2f4249b..ba3b50895f6b 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6568,14 +6568,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, NULL)) + if (process_dynptr_func(env, regno, ARG_PTR_TO_DYNPTR | MEM_RDONLY, NULL)) return -EINVAL; continue; } diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 283f55bbeb70..714f5c9d0c1f 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 41ef7e4b73e4..c484e632b0cd 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -565,7 +565,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) { @@ -699,6 +699,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) { @@ -720,9 +741,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 */ @@ -730,8 +750,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; @@ -753,25 +773,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; @@ -787,9 +805,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; @@ -808,15 +831,19 @@ 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; /* 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 @@ -1324,9 +1351,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) { @@ -1389,6 +1413,26 @@ 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, so simply + * set it unconditionally as it is ignored for STACK_DYNPTR anyway. + */ + __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) { @@ -5692,20 +5736,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, - struct bpf_call_arg_meta *meta) + enum bpf_arg_type arg_type, struct bpf_call_arg_meta *meta) { struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; int argno = regno - 1; - /* 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"); @@ -5722,6 +5808,12 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, meta->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", @@ -5729,6 +5821,8 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, return -EINVAL; } + /* Fold MEM_RDONLY, only inspect arg_type */ + arg_type &= ~MEM_RDONLY; if (!is_dynptr_type_expected(env, reg, arg_type)) { const char *err_extra = ""; @@ -5874,7 +5968,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, } }; @@ -6050,12 +6144,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, @@ -6119,11 +6216,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)) { @@ -7091,11 +7194,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 */ @@ -7474,7 +7576,13 @@ 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 directly. + */ if (meta.uninit_dynptr_regno) { + if (WARN_ON_ONCE(regs[meta.uninit_dynptr_regno].type == CONST_PTR_TO_DYNPTR)) + return -EFAULT; /* we write BPF_DW bits (8 bytes) at a time */ for (i = 0; i < BPF_DYNPTR_SIZE; i += 8) { err = check_mem_access(env, insn_idx, meta.uninit_dynptr_regno, @@ -7492,15 +7600,22 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn if (meta.release_regno) { err = -EINVAL; - if (arg_type_is_dynptr(fn->arg_type[meta.release_regno - BPF_REG_1])) + /* 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 directly. + */ + if (arg_type_is_dynptr(fn->arg_type[meta.release_regno - BPF_REG_1])) { + if (WARN_ON_ONCE(regs[meta.release_regno].type == CONST_PTR_TO_DYNPTR)) + return -EFAULT; err = unmark_stack_slots_dynptr(env, ®s[meta.release_regno]); - else if (meta.ref_obj_id) + } else if (meta.ref_obj_id) { err = release_reference(env, meta.ref_obj_id); - /* meta.ref_obj_id can only be 0 if register that is meant to be - * released is NULL, which must be > R0. - */ - else if (register_is_null(®s[meta.release_regno])) + } else if (register_is_null(®s[meta.release_regno])) { + /* meta.ref_obj_id can only be 0 if register that is meant to be + * released is NULL, which must be > R0. + */ err = 0; + } if (err) { verbose(env, "func %s#%d reference has not been acquired before\n", func_id_name(func_id), func_id); @@ -7574,11 +7689,8 @@ 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 fdb0aff8cb5a..e8d90829f23e 100755 --- a/scripts/bpf_doc.py +++ b/scripts/bpf_doc.py @@ -752,6 +752,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 fb4c911d2a03..b7543efd6284 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -5290,7 +5290,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*. @@ -5300,7 +5300,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*. @@ -5310,7 +5310,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. * @@ -5411,7 +5411,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 Nov 15 00:01:27 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: 13042988 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 0507DC433FE for ; Tue, 15 Nov 2022 00:02:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230336AbiKOAC4 (ORCPT ); Mon, 14 Nov 2022 19:02:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54680 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231959AbiKOABq (ORCPT ); Mon, 14 Nov 2022 19:01:46 -0500 Received: from mail-pl1-x642.google.com (mail-pl1-x642.google.com [IPv6:2607:f8b0:4864:20::642]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A791FB1ED for ; Mon, 14 Nov 2022 16:01:45 -0800 (PST) Received: by mail-pl1-x642.google.com with SMTP id w23so5175779ply.12 for ; Mon, 14 Nov 2022 16:01:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=qmkzNiMhMpzVxaMwoGjL9LJGwwdguwmfc96PIBOM3ZA=; b=Ava8re4a3pTJmdbRGzKolVMPiBHb6aa1cZxm30imlN3TdIhXpHNnljcwa/bUSf/uUh eXE5Eeg7lNKiQD1ik5i3gpAgtMprSr1kc7I5qwibPIhKe0M4NQLXVICnVnCXxyJBQWSh LvjxrlxN7j2JGv48s4sEnSfMS7kjNRbrbYR+Y5cuNSijDHmTll3TuRsIEVbB70h/5pxU iFfTKjfYgzOp/kF/TOwRbNc0RaOablZ1UMHMQFuBZCKq7qG2D4sNJNPpj7sLhmmYPM/Q O6hFS0TU6V24lH13vJGYfyMKJ10YMZmRoMyLH9iNND6DrDmd/m5rPakdUrwdzwEwzQt7 jbEA== 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=qmkzNiMhMpzVxaMwoGjL9LJGwwdguwmfc96PIBOM3ZA=; b=vzaOUgti3NWb7VLlYUMXZJ276B++OzguNHIwrmOsdKHsgAX8626SIIpQG5XGJcbbwI wNV4ibIqdeTshKw3TkXgF+zIH0oJZclVsp0nvVYRs4AzlkS8qrLCwIULV4CpZBdHl/DJ +jEGQ/5tztmpsZRC256LTw+2EEC+oosZBZdWwFmdYiwvm/oL2guUFI6uaqFXex2NIkQk jQpdreuw97ZWdmJ0ApINfEGWdwJ0u6Dqo6s4jyN7El8N52Xt6Z0cSuE9dFZcKdFiYTde v76Z6rte2qOUmn5ud9aI6vThxFKU8mRhBHF68qMjxTcXiZHzkEVqOwW30K1lpzRqQHI2 /pBA== X-Gm-Message-State: ANoB5pnzkUpCyCiCrW8oH01lrUEjCd15itgJk5HCRaI18GDcH18cBpjq 8uF+YlU6IvWRQRYz+cA/YYXXB7PSJA9lOQ== X-Google-Smtp-Source: AA0mqf6UcCKkl7Cl401fVOkX7kYhu/Ib5/zZcKiqZef9f9La4tWBJmLqkkqAyVm46BnPFoe2H2gecw== X-Received: by 2002:a17:90a:6f61:b0:215:db2e:bb17 with SMTP id d88-20020a17090a6f6100b00215db2ebb17mr15889830pjk.166.1668470505082; Mon, 14 Nov 2022 16:01:45 -0800 (PST) Received: from localhost ([59.152.80.69]) by smtp.gmail.com with ESMTPSA id z6-20020a17090a8b8600b0020ad86f4c54sm7150820pjn.16.2022.11.14.16.01.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Nov 2022 16:01:44 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 4/7] bpf: Rework check_func_arg_reg_off Date: Tue, 15 Nov 2022 05:31:27 +0530 Message-Id: <20221115000130.1967465-5-memxor@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221115000130.1967465-1-memxor@gmail.com> References: <20221115000130.1967465-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=6373; i=memxor@gmail.com; h=from:subject; bh=Qvz4MOK2PUkz7b8XDLMgB2UMc4dEh3SP1U0Ifo75YT8=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjctaC7kT5AnynkXcc0YTmJ/gSuG7EDRuyDNRE6WBc 9HmnR0yJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY3LWggAKCRBM4MiGSL8RytRRD/ 9WGzbQuJAcUL1vNB+XqrIYqE9/02466aIwF+N5yeiu3/AGMQdXIxCQcO7r+62pKEyqDFOMB+L8ASSw 3H0rOO6yl224WnsfWbFQnYi4rILOqxpOyu9EsSnFtjWxY3OLnF0yGbEPMs5Bt2EqzWCQPwBYc3A7N1 ZEaJWdmv/jiZpcFD86OxGF8p4BsJ8NU49FgYcCf6uecPJYWYY4h9sI4OOYHZKGVGxkFI/03azmCAXe L0ITYXbL4URxFr3UPqWDQWGjjv+lXsIk7ZeMACN4Aek258rz2cwEGGB4/BvVdeAtWEoZ9hs9oFbM8k kCi+w32tbxH/WuyEbAc9r/tFxhb5nWTHZBsUByJREaueCZ+EP7MCXbK3eIW9r46+RHql4UjMhG2nRx 5GPlGbJFZyXzKnEahQn5qlkvkXF3iXBS8Ts3M7oMsaK8Ns3rsIyP+YK4BYafQqQcMj21+vlySzqTRd ObquGzMaEy7gDbY5V4VtBp3uRe2d7hZyrsA5sorcfSedhOBoHj+W2t+BEkmoOk+48jzlHsNLB/W9IP 0CwNfwBuM5zeTXJlYfnYYVGQzD0F+Ulqv5TPnke3IgmYxiq3ZVMdGHS2Cc6Bf7BeXizwF0zfXqU9eV Mhu98O3De4fNFnW+Fe6LFljQXplxEv4bJs8a0237RyahmnHGkdaglyfHW2Bw== 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. The function will be restrictive by default, and cover all possible cases when OBJ_RELEASE is set, without having to update the function again (and missing to do that being a bug). 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). 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. This is not specific to ARG_PTR_TO_DYNPTR though, we will need to make this exception for any future object on the stack that needs to be released. In this sense, PTR_TO_STACK as a candidate for object on stack argument is a special case for release offset checks, and they need to be done by the helper releasing the object on stack. Since the check has been lifted above all register type checks, remove the duplicated check that is being done for PTR_TO_BTF_ID. Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: Joanne Koong Acked-by: David Vernet Acked-by: David Vernet --- kernel/bpf/verifier.c | 62 ++++++++++++------- .../testing/selftests/bpf/verifier/ringbuf.c | 2 +- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index c484e632b0cd..34e67d04579b 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6092,11 +6092,38 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, const struct bpf_reg_state *reg, int regno, enum bpf_arg_type arg_type) { - enum bpf_reg_type type = reg->type; - bool fixed_off_ok = false; + u32 type = reg->type; - switch ((u32)type) { - /* Pointer types where reg offset is explicitly allowed: */ + /* 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 with OBJ_RELEASE 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) { + /* Doing check_ptr_off_reg check for the offset 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; + } + return __check_ptr_off_reg(env, reg, regno, false); + } + /* Fallback to default handling */ + } + switch (type) { + /* Pointer types where both fixed and variable 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"); @@ -6113,35 +6140,22 @@ 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. */ case PTR_TO_BTF_ID: /* When referenced PTR_TO_BTF_ID is passed to release function, * it's fixed offset must be 0. In the other cases, fixed offset - * can be non-zero. + * can be non-zero. This was already checked above. So pass + * fixed_off_ok as 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. */ - if (arg_type_is_release(arg_type) && reg->off) { - verbose(env, "R%d must have zero offset when passed to release func\n", - regno); - return -EINVAL; - } - /* 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. - */ - fixed_off_ok = true; - break; + return __check_ptr_off_reg(env, reg, regno, true); default: - break; + return __check_ptr_off_reg(env, reg, regno, false); } - return __check_ptr_off_reg(env, reg, regno, fixed_off_ok); } static u32 dynptr_ref_obj_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg) 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 Nov 15 00:01:28 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: 13042989 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 BBDD5C4332F for ; Tue, 15 Nov 2022 00:02:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231787AbiKOAC5 (ORCPT ); Mon, 14 Nov 2022 19:02:57 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54688 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237577AbiKOABt (ORCPT ); Mon, 14 Nov 2022 19:01:49 -0500 Received: from mail-pl1-x643.google.com (mail-pl1-x643.google.com [IPv6:2607:f8b0:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F06E6F3F for ; Mon, 14 Nov 2022 16:01:48 -0800 (PST) Received: by mail-pl1-x643.google.com with SMTP id io19so11599506plb.8 for ; Mon, 14 Nov 2022 16:01:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=FSaNQ/dcQ4x6jvSeP/uwsY/xIHDGGm96/RFbVHCGjvs=; b=qXb9bWVVXlymBEFkIrbQN0phea6tC7Ub1cfTMPNNoTXOeTX2QZXSviLcH5FVVEpQ4v JJaJukZkWswkREH7D7U6veGYeqKNfETMQLHlcW/p/b2y3bW07/Gr39uoQ5Gamhuebxxd 7YSF8Spc4CxFbwCH9trDQim+/O8V2CXB1JV0zynkI218tyHGoc8kylF+xm7oFk9/97uU cYpHfW7JnN/qa3QTtyxUetnKISsSDQRK9L2cFiHDWzIaUalslw8Q7mIgZmVC7YDmC3om MJB6kyCHI3y076ipWOkUMKmwoNNTSoZKVMapXo+Ith5RyJC6aKiu3IPHcjZ00SltQjQZ qNYQ== 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=FSaNQ/dcQ4x6jvSeP/uwsY/xIHDGGm96/RFbVHCGjvs=; b=VUfGYyV7laMtwjmTQ3S7X4bkOW1EehiXNrKUnzWlDAz5684A/MK0ss9NvK/OetQHQq tUs3g4ez9DVxY5kgGkFklaEa/r7WIKkRohwfDOw9zI+SyQ5FHb3WPkPubCmoP/Abqyzi eb7qik+tqkRCte7bKihxrJPEhYTkqaK23YXJdTcSBDFNmHiBsdejPlD0AXPZ7a8ystEa T5oFFDxolKhyFLAlgthft5tf1aMpG8yXS3q0hr6USg98nIASeQ8IarCwusGYPcPp9whz fgVlDJ8pxZVloSaPi7RmxVKMLpx9qxl67H6xxhJF1osensPyinEFNyUY5uw/2f+mC0Gs zbRg== X-Gm-Message-State: ANoB5pn60Qh0HOetcqOocXVZaq5MWaXE6PppRWTBvavflHOfGLN/x8U2 hBXhTXDP53gur8zxem4KSCUOBUFsipSTUw== X-Google-Smtp-Source: AA0mqf42biwAiNCBQUU1zQ5pcY8AaeE80r3HCcPiGbm5Hy/9yCbYkwThYnOY0gpwMUDdI99UW4K8xw== X-Received: by 2002:a17:90a:1b23:b0:218:725:c820 with SMTP id q32-20020a17090a1b2300b002180725c820mr15906688pjq.170.1668470507977; Mon, 14 Nov 2022 16:01:47 -0800 (PST) Received: from localhost ([59.152.80.69]) by smtp.gmail.com with ESMTPSA id gn19-20020a17090ac79300b0020de216d0f7sm7165284pjb.18.2022.11.14.16.01.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Nov 2022 16:01:47 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong , David Vernet Subject: [PATCH bpf-next v1 5/7] bpf: Move PTR_TO_STACK alignment check to process_dynptr_func Date: Tue, 15 Nov 2022 05:31:28 +0530 Message-Id: <20221115000130.1967465-6-memxor@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221115000130.1967465-1-memxor@gmail.com> References: <20221115000130.1967465-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2166; i=memxor@gmail.com; h=from:subject; bh=wwpedgumiY2D1BsQiE1DVI3mrdHRTA/QF170z6s3Esw=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjctaCULBcQmHu62Xw8Stpoe7K7PEQNoT51ilLjHBn qqr7+OOJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY3LWggAKCRBM4MiGSL8RymS5D/ 9lqzTqKSYgno295pk1xNBxNHoaSRhaDKJo+hUR9gpHXSwCTMo/k160fRSJ0sK/cW1wGAOCQSlJdX1f Qg71UrA9mdZZJw3NY2LYBwzIkEZ/yIGzG5PK24dFvNB5E7Vj62/7QgRfC16ZmgfMHlm/LpZYOvkqCc z9TwWY2xHg9pqtkzQ905IawwcJTSyS/hUmK7M0/5dnHtxtF6QyVHRsk/G87DvM5ycpXci2zHzWajhX c1tOKCrw/Lww/5gASE5gb/AMUF+9QLFfCmekCwBs971Q6oBGjZB5sBQgnlkh0x0pbh7siYaJ8YCuI7 YmHj2QMS3Cn2FeTa+sirHiObldOIXypec10QQtJpmPniFBl/3ZjAy/n+xobUveApQiwni/PMKV/SkT 3zzNFsSekunkfx5bgrQoLn1KwV0gMwAbjI0y+AHD96noX6UB0ARy0LyMUTo7c6/YaN60SyG8nojCq9 SYH4pOZhEmQIezKh9xidLUqqkFo71QrqTOVGAiNzP7r7JCjhSsxvSDpkHQl7/6vLAnVESA5lwlX4g0 oWZb3lGT0L2MVs502IZHIFImPtoqf56kb7bTcSK7FSz3KLkMKYUv0HY6YZM3LVS3nuZm3o4lj7mWh6 +WkROD5nBZLBaUbH2fiHBgJvEwnranrxyXaKg+X7dzI2LNGrGPAlO3jYz3Ug== 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 After previous commit, we are minimizing helper specific assumptions from check_func_arg_reg_off, making it generic, and offloading checks for a specific argument type to their respective functions called after check_func_arg_reg_off has been called. This allows relying on a consistent set of guarantees after that call and then relying on them in code that deals with registers for each argument type later. This is in line with how process_spin_lock, process_timer_func, process_kptr_func check reg->var_off to be constant. The same reasoning is used here to move the alignment check into process_dynptr_func. Note that it also needs to check for constant var_off, and accumulate the constant var_off when computing the spi in get_spi, but that fix will come in later changes. Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: Joanne Koong --- kernel/bpf/verifier.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 34e67d04579b..fd292f762d53 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5774,6 +5774,14 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno, return -EFAULT; } + /* CONST_PTR_TO_DYNPTR already has fixed and var_off as 0 due to + * check_func_arg_reg_off's logic. We only need to check offset + * alignment for PTR_TO_STACK. + */ + if (reg->type == PTR_TO_STACK && (reg->off % BPF_REG_SIZE)) { + verbose(env, "cannot pass in dynptr at an offset=%d\n", reg->off); + return -EINVAL; + } /* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to a * ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*): * @@ -6125,11 +6133,6 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, switch (type) { /* Pointer types where both fixed and variable 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"); - return -EINVAL; - } - fallthrough; case PTR_TO_PACKET: case PTR_TO_PACKET_META: case PTR_TO_MAP_KEY: From patchwork Tue Nov 15 00:01:29 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: 13042990 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 43974C43219 for ; Tue, 15 Nov 2022 00:02:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231894AbiKOAC6 (ORCPT ); Mon, 14 Nov 2022 19:02:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54698 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231984AbiKOABw (ORCPT ); Mon, 14 Nov 2022 19:01:52 -0500 Received: from mail-pl1-x644.google.com (mail-pl1-x644.google.com [IPv6:2607:f8b0:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 802B9117B for ; Mon, 14 Nov 2022 16:01:51 -0800 (PST) Received: by mail-pl1-x644.google.com with SMTP id d20so11589125plr.10 for ; Mon, 14 Nov 2022 16:01:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=Shrual+sQARxwS3Eq7Ztga+SseY41SgICBVkPEePOJw=; b=UuDscHsaHn//91RFnebGFO9YuZnZppJMmX6OWuXsHIEl7vG6g6FU6vYr/wL1GsJ0fK /k1BSGVBU9LAjS++MmMP0rTVIt+ythOuvC+ZujMeiy1NGw5J66/xB0Y9ivng4BVWtzkT raqj3SzuLb3sC6uT897TOoDHa+eD2bFLlruoqg1siiuGBbRe8F6APlcrmLq7ksRo+7zf gV9NleA2smEuvtrxq7iWrTvVa1fYGFZK+4/chBffW8hJOfbqwbp+eWXejZvBzVA9QGRY 5KVOCG8oxJaCUTqMz7A/HihNvT04Rwdzj3ljOcUBEHo2d2c+szGN+X8a17hrEFzXvj8u 7lQw== 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=Shrual+sQARxwS3Eq7Ztga+SseY41SgICBVkPEePOJw=; b=6s02xDwB8fO4SjRF8gwOpxoGcs0u5sg/Eb8ocfzjNi8DEMmrBbnwxIiyY8G1asvztW RqJek7xwgA8Faa+roJWYeA6EPkdnsWrEf0tOrWIDdRcfpAZcb5Fmo6n019TqSq/+cJFZ C1W83fr7qqD1JlTgXEuHa1zqcNgkBx7hI+2KeW3j/zRp6X748iSnNaJeDcXWRT+ZvqOy csamfSiDJu7XG0o34fcTjY2g8WfWcfUHc8pEZ8DdfimQfSyLCyAa8WUiByjmXJqg3pnO stZ71RMw4IFt0fvlbFuKuYVZVn5RH2Rsy48Ak1/ghOATRvL2y1+/qupZX0rLaRjGH7D/ b4ag== X-Gm-Message-State: ANoB5pmrWyE1vu7Zayvcyoig7aNO0p7p7vqWzGY03jZSBeSyDZI9jXBU go6uwIdA/nhpnCkRU58B/QsNfH4BVjt4yg== X-Google-Smtp-Source: AA0mqf7NmhvyU7fTrJJZG/mDle+GM35ltFxQoW24sQwrSfJBDgDKbdOxVSDh8fKfGdM4Oiwgd+y28Q== X-Received: by 2002:a17:902:9046:b0:186:6d69:6e01 with SMTP id w6-20020a170902904600b001866d696e01mr1399423plz.160.1668470510862; Mon, 14 Nov 2022 16:01:50 -0800 (PST) Received: from localhost ([59.152.80.69]) by smtp.gmail.com with ESMTPSA id c4-20020a170902d90400b00187022627d7sm8138104plz.36.2022.11.14.16.01.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Nov 2022 16:01:50 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: Joanne Koong , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , David Vernet Subject: [PATCH bpf-next v1 6/7] bpf: Use memmove for bpf_dynptr_{read,write} Date: Tue, 15 Nov 2022 05:31:29 +0530 Message-Id: <20221115000130.1967465-7-memxor@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221115000130.1967465-1-memxor@gmail.com> References: <20221115000130.1967465-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1264; i=memxor@gmail.com; h=from:subject; bh=N/5OHsSmqeJt/icLLaIXxrEti8S0MTFqAak+Q5xdCLw=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjctaCHlIzGsrCHiP/KWFyiEayLnP4gu1RPG73YYTF R3bxX+GJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY3LWggAKCRBM4MiGSL8RykOYD/ 0XEhkk4fp69NkS+BPkrw22TPHKgQNuVsL6IMqg3R361EgA+L6EwmM9679oghwSexV9T1kxo3yCnZsT XYEHp2L1yUtOGPmOj0IEMSNgntcQypTDChWC8m3mat41PsO3ni/gynETvw6V4CtzSaUGszQ5nAjlrC aKvp0aPWjFcA8qtAvuwR+gCB9YzSO6NvTCPaIa3ANBFmG2xc94GOVVmnx/J4EycIF28I8p2Lb14htw 4bwF54Af85RydIL1NgbBcuosB5b/hnVYSooA/D3gN2bq+xI9E77ExWOiEes1NnsTBK3LTVhRBmEIJK gp7SkfuJdNPH66MYUuNTWwqTEVC60Gsq1HLNGN9GesqTBuOsYJ8Y99RtWxge/kXx4Fc8R48JDk8/iW 1atqnZzFDllQ5YhSXfkcUUwm5DqMv2VK6yTdI2dQkt807tymYBgWmRfukEbcre9j3mbmeRKz2SQJoN Xwy2ku7pSCqjRVlHyN639gZB21Zfw/KxYtQnjRUz6UWkLSzN8RNNqCHY5tvNyWGRsRSn6CfFiUy6Bv LBzcLn90CtbRHVFL1g3HuSZpnl9dkfapVJY4VKWiBmGC50XZGqXfJ0ojgWMOPn4TpuxnwlAxCl4TJ2 8oRTI/2l5Oio+zvintk/rk6sxTPex3t43NOH+WwUXg4jGVaPDj7ZZex/f3Og== 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. Acked-by: Joanne Koong Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: David Vernet --- 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 714f5c9d0c1f..1099ed1e7712 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 Nov 15 00:01:30 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: 13042986 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 7392EC43219 for ; Tue, 15 Nov 2022 00:02:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230415AbiKOACB (ORCPT ); Mon, 14 Nov 2022 19:02:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54710 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237649AbiKOABz (ORCPT ); Mon, 14 Nov 2022 19:01:55 -0500 Received: from mail-pf1-x442.google.com (mail-pf1-x442.google.com [IPv6:2607:f8b0:4864:20::442]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5A17EF3F for ; Mon, 14 Nov 2022 16:01:54 -0800 (PST) Received: by mail-pf1-x442.google.com with SMTP id m6so12610020pfb.0 for ; Mon, 14 Nov 2022 16:01:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=6qiFDbqjXUxto4VZlgYzZmUfXOGly2z0K0YGSO0F0m8=; b=GDF4RsjJyLpg+kMFiMSR2t1PJY1t+uB37VdqwwkWCSJjOcc7YIbUa6/Tng+33FrqGr b2tHLO2BOYmphwWMw5AfQkAgrFqlsHj7/zvtqK5Ttc2CHfL+tBEdQ0vCOLyTYkHu1/R8 xQTSY3T6E4PtIPVSE32O+ULKzqvd5NgjyK4bMDd4oi65sVulDp3jQLnZV/GPNwr0ROMo EM9RRmgHUvoroJ/N06L0ornc7eQSK7GzB6URWOp5aXBWZ+/hwrcCyNEkPGLwCgWw0ah9 R+QZff+wqRI49164exImOHGN1tsO/w6wqNcKdQMgvk2WxKNYbDccTWsl8JZNY+QT8W3y 40bA== 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=6qiFDbqjXUxto4VZlgYzZmUfXOGly2z0K0YGSO0F0m8=; b=8HJwtGgY6t57NnPXznDvgxs6WB/d6Eta5eMXn25AfcyXeHxfvxTPQtksnwHviygLrB cbfnd7WzSv1e7AMnJhATq8uAySybwl88eJinzS7sGiAO0k25ntLgDHqwyjNKn70M61N5 eY+VtgOQTQMO4T+dMa/BS7TuJWQcX9nkHiIeup0rce+7rrVMRGHXaguKLlkJKfwXvL1n Xjh9UnuGY4dFfhpUlwirfRCOPY+FvSJDwdZuTerVXcU9e45Ii7EjyGD7Aq+on5C5J46S 95dFfUmzigwwztcrqHlK5MujWZut2Zrv742ABdi56GZNPKd5toU2jNQpgrpfKQEktDLP XR/g== X-Gm-Message-State: ANoB5pkJB3z6VW6VH/6rQXkexRv4zBKzIFvqdWylgA2+BoxXgqFs/mc3 1FD3yXfbOSmEi7oiWl3qKBDiON8wNktMRw== X-Google-Smtp-Source: AA0mqf713gNI7CTI0lgC1Cjvu/j4qEblzw5kPeDjohTT4UI66o3oPPav5hRFdAIdQIz4hW8i8EPhTQ== X-Received: by 2002:a63:e242:0:b0:473:e502:71d3 with SMTP id y2-20020a63e242000000b00473e50271d3mr13742939pgj.227.1668470513707; Mon, 14 Nov 2022 16:01:53 -0800 (PST) Received: from localhost ([59.152.80.69]) by smtp.gmail.com with ESMTPSA id ne5-20020a17090b374500b0020b2082e0acsm7112233pjb.0.2022.11.14.16.01.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Nov 2022 16:01:53 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: David Vernet , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Joanne Koong Subject: [PATCH bpf-next v1 7/7] selftests/bpf: Add test for dynptr reinit in user_ringbuf callback Date: Tue, 15 Nov 2022 05:31:30 +0530 Message-Id: <20221115000130.1967465-8-memxor@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221115000130.1967465-1-memxor@gmail.com> References: <20221115000130.1967465-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3072; i=memxor@gmail.com; h=from:subject; bh=KGMe3EiZxZpqYFfmV01eKi4iWDkcxxO+yLBDjrSQ6BI=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBjctaCB6l5QzhsTIZ2pAWx0VJpr8Fn4ORBOChmz815 qyXDiXaJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCY3LWggAKCRBM4MiGSL8RypFNEA CellLPu+8tenKkc0gDDOhuSDOp8xh6cgKl8lGpMeGw+H9C+6sDaYOe79psNl1BqRTkH74EbTR5BPq7 cDU/kZKMIekG+9/S1xXcVUIDgxj8vfCQm9GaVMgmj4ZNC4nngcXjdslk0cU695pqEtAo2SVr47Z+hr FoBWTJiqYl6zgaV2NC969SD06nuHdncekkt/t30FsH2luj7BMHbPbbprq1R3rPSAFfDdoeYu6ZBviq FFLRNRmHlzD7cD8ArGU6yYDrXT7JNKNb7LdRCiqAzNyZT/HKdTomm7rxABjIVfoiLtZGiZnVMcV+lI qGFm+S7pgv9g9PZiTHEfdLuPsa11XpLwn8eJ5Mb0sDT/jI0wEXVxb61pBfXbMfJJyUdKUrs7grUnk/ cJKcxUDt5b4S0miC4PTK5a2lmpI+HStSAw5YffXqy/xORHf1H7x9eg3g3nMgsmnBMG0ID2Vm0ZgEJr 63a7KsaRrh5vH95+21zZGDgXtZyKnc5zJkU7lcccy9J21MZYW0B71R2H3Qlyf/NamI2QM+BI4B9fa2 1/gOtoAvsE5Ynwz5EbQn/GVKQ8rO3E+47YGdnwHeIEYNZhY4lv5C36H6lOo3gxnf0TfbXE5JTu7FAK Z6HK8QPXuVIeaHjIjKab+ZqfkUAJW3aGQ46mJtPjPtco6AjCUB0TiFJqjhCQ== 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. Acked-by: David Vernet Signed-off-by: Kumar Kartikeya Dwivedi Acked-by: Joanne Koong --- .../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; +}