Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   Shung-Hsi Yu <shung-hsi.yu@suse.com>
To:     netdev@vger.kernel.org, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org
CC:     Shung-Hsi Yu <shung-hsi.yu@suse.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>
Subject: [PATCH bpf-next 3/4] bpf: verifier: remove redundant opcode checks
Date:   Fri, 20 May 2022 19:37:27 +0800
Message-ID: <20220520113728.12708-4-shung-hsi.yu@suse.com>
In-Reply-To: <20220520113728.12708-1-shung-hsi.yu@suse.com>
References: <20220520113728.12708-1-shung-hsi.yu@suse.com>
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?x4mJRXkZ6vlV9EWYhl/py8/IwTpfRWWa4thujcTORrvm71/iqrAoNipsWDBG?=
 =?us-ascii?Q?deHvVAtiZSFCCxn6gyC2m0zpttj1Yopfbkdy1HX1uLPLmpSkI958OQGOj+vX?=
 =?us-ascii?Q?hJkfvUuUc/i8KGeVjd6Jsnnj6X7WC6UFSYgIEJ8ocLHeliPQRPbN99FiA2Uf?=
 =?us-ascii?Q?DInqkD0S6meO+gkOgYZ1wMI/0MqVTsvYL0KYLLJKUYgEqX+/sQERgFGYmgjY?=
 =?us-ascii?Q?VATFMYu2QG0xGLYhCxNVMZaTt1Q3jzqAh8qePIznGm+9eBNkTNB144OmX6ME?=
 =?us-ascii?Q?bRVmA0IWHg8LcoJYnbpMcdqCn5yD6iQ9VJoVUXDbcv/Q4DPLMQxWaAD8wF1s?=
 =?us-ascii?Q?1Fy/4Q6pmKwlRe/LcUV36guUZv0ncy6KyV3oQRBeQvihFpgpj98IaVa7nOZC?=
 =?us-ascii?Q?LifL19fN0tmZkwqroyf7ee24eQsX0X2jAdIMpO/WdBaDjFaCx5732LnmVtG/?=
 =?us-ascii?Q?7PLnQ0QTROKTAC5xjJPpRGkZaIisSb2ADTJT8wHW00a3R0fkCO05UCXYc/w6?=
 =?us-ascii?Q?Fx5dFWBDY/vzMn8Q8Wp/yczHNlU71/z7iiY8g9u6z8CrDsdVYskVDhlJ7OEs?=
 =?us-ascii?Q?yNcnjQEYwolGorDWA3ceNyWYcOzFLQ/+MXEtX+0RoJZjTATKDbQFrxI/E+HA?=
 =?us-ascii?Q?P9sgyS6eltrsK72Istnky8bi4hShQwBBnNuU08gFVJV97WQiIE9NrMO0lLx+?=
 =?us-ascii?Q?lcOjul0VA7ZGVopK127jSBNCro/ixE65Nno8ODyjHVFf2gQ7KhZ8w0QBXnCH?=
 =?us-ascii?Q?d/LXIbMX7nvbak4OVjezD5wYx8Xun9URXiqgwrEfupfLufnoQSBT7pyQUX0C?=
 =?us-ascii?Q?ElxeZooZ2ovRyTQHyqfyGM8CVOayoD8T3g/NkH7B6EhPuPwX6/QIrqXuW5pu?=
 =?us-ascii?Q?6qT8yP8HVsaKS/3kv+wspaS0Vh84lfOVwWAMk10X+53O6MB0g3YozEsqde+k?=
 =?us-ascii?Q?HC0XTEC3c/vOxsPOk/fACLUIBVBqohIuxkMu4zbyIRlPv9bD4oVzt2t2tO9n?=
 =?us-ascii?Q?WsRhD3H5P5K03ZEAmXgSXsOkjHCd1TWT6bhluct9TBOptL5CzSEFxG1hFm+T?=
 =?us-ascii?Q?JxBl1KSxc67wkHN1k/eMf3IptN3Pk6Myu1TnHTgBJXXL/Sem48MWjjoqkh4P?=
 =?us-ascii?Q?lR4OzBMMLCPp06s6jd58FcDuMSz69nfVGg5sEqq29yCA3IW/AclFXWMWXqdV?=
 =?us-ascii?Q?33zq6EOOVGm30wUdwLssL/VwX0VtM4WA30xqnuRm90YFarjJFGiIMtfjFHi8?=
 =?us-ascii?Q?lm2DJ/MIosKJ4xK1BLaq8RsaAx6Iunz2nk2SZdOwgVB7+fWDY/Fel093P7Bb?=
 =?us-ascii?Q?hoZA6T7dnsVihAxTvkglKCr0KuGDXWTy98/sMrFJNHNLg9HMi9jPHEZHL6je?=
 =?us-ascii?Q?KrBaNSIQesDhGWW0fpls0SqLwDeSvi8NsujsoYimKwfc9RXn5hZ+4fHdcaIz?=
 =?us-ascii?Q?MKcQzxIdiyqtZCeClFFrbVo4Cdfwr9PV3MVIAkDliKrqJk/iFeE/iFGoVvfx?=
 =?us-ascii?Q?VsYv/n1Dkqsxajr535jTh4/1+eGBWy/IUhvPs6TaRF5gzBrpc/rdyoa0DuIG?=
 =?us-ascii?Q?vihGnRhc97Wv3oK4QLxjSZw7don9rZxDRJG9lDj8VG5eIUoW7pzXlGKzR3gG?=
 =?us-ascii?Q?AYufl/tSxJtUY/pkkLuM51wA8xuqxv68eWzojVw3CWaZqO++6pxQ4mht1Yue?=
 =?us-ascii?Q?9RrwD+aoH5U35YyFb/jjJM/AGluj+Yr8awg+jvKi5Ozu4u2Ux5RSFGlPixh2?=
 =?us-ascii?Q?Wl3V39OUDQ=3D=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b9124e83-b4d8-4745-ec1f-08da3a55317a
X-MS-Exchange-CrossTenant-AuthSource: DB9PR04MB8107.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 May 2022 11:38:00.4523
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: lHSqXRmBBPy7JQoYiyus/x4blOO5ezM1G0HVO1LxBoFOtL+Q2NyOigE/UdzcLWEUpm+5GdsbLUez330wPVCi8w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR04MB3253
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

The introduction of opcode validation with bpf_opcode_in_insntable() in
commit 5e581dad4fec ("bpf: make unknown opcode handling more robust")
has made opcode checks done in do_check_common() and its callees
redundant, so either remove them entirely, or turn them into comments in
places where the redundancy may not be clear.

Opcode code check is not removed for BPF_LD_{ABS,IND} in check_ld_abs()
and BPF_JMP_{JA,CALL,EXIT} in do_check() because they cover opcode
validation not done in bpf_opcode_in_insntable().

Signed-off-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
---
 kernel/bpf/verifier.c | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 133929751f80..d528848083b9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4797,11 +4797,6 @@ static int check_atomic(struct bpf_verifier_env *env=
, int insn_idx, struct bpf_i
 		return -EINVAL;
 	}
=20
-	if (BPF_SIZE(insn->code) !=3D BPF_W && BPF_SIZE(insn->code) !=3D BPF_DW) =
{
-		verbose(env, "invalid atomic operand size\n");
-		return -EINVAL;
-	}
-
 	/* check src1 operand */
 	err =3D check_reg_arg(env, insn->src_reg, SRC_OP);
 	if (err)
@@ -8793,8 +8788,7 @@ static int check_alu_op(struct bpf_verifier_env *env,=
 struct bpf_insn *insn)
 			}
 		} else {
 			if (insn->src_reg !=3D BPF_REG_0 || insn->off !=3D 0 ||
-			    (insn->imm !=3D 16 && insn->imm !=3D 32 && insn->imm !=3D 64) ||
-			    BPF_CLASS(insn->code) =3D=3D BPF_ALU64) {
+			    (insn->imm !=3D 16 && insn->imm !=3D 32 && insn->imm !=3D 64)) {
 				verbose(env, "BPF_END uses reserved fields\n");
 				return -EINVAL;
 			}
@@ -11874,9 +11868,8 @@ static int do_check(struct bpf_verifier_env *env)
 					return err;
 				env->insn_idx++;
 				continue;
-			}
-
-			if (BPF_MODE(insn->code) !=3D BPF_MEM || insn->imm !=3D 0) {
+			} else if (insn->imm !=3D 0) {
+				/* check for mode is already done, so mode can only be BPF_MEM */
 				verbose(env, "BPF_STX uses reserved fields\n");
 				return -EINVAL;
 			}
@@ -11909,8 +11902,7 @@ static int do_check(struct bpf_verifier_env *env)
 			}
=20
 		} else if (class =3D=3D BPF_ST) {
-			if (BPF_MODE(insn->code) !=3D BPF_MEM ||
-			    insn->src_reg !=3D BPF_REG_0) {
+			if (insn->src_reg !=3D BPF_REG_0) {
 				verbose(env, "BPF_ST uses reserved fields\n");
 				return -EINVAL;
 			}
@@ -11944,8 +11936,7 @@ static int do_check(struct bpf_verifier_env *env)
 				    (insn->src_reg !=3D BPF_REG_0 &&
 				     insn->src_reg !=3D BPF_PSEUDO_CALL &&
 				     insn->src_reg !=3D BPF_PSEUDO_KFUNC_CALL) ||
-				    insn->dst_reg !=3D BPF_REG_0 ||
-				    class =3D=3D BPF_JMP32) {
+				    insn->dst_reg !=3D BPF_REG_0) {
 					verbose(env, "BPF_CALL uses reserved fields\n");
 					return -EINVAL;
 				}
@@ -11968,8 +11959,7 @@ static int do_check(struct bpf_verifier_env *env)
 				if (BPF_SRC(insn->code) !=3D BPF_K ||
 				    insn->imm !=3D 0 ||
 				    insn->src_reg !=3D BPF_REG_0 ||
-				    insn->dst_reg !=3D BPF_REG_0 ||
-				    class =3D=3D BPF_JMP32) {
+				    insn->dst_reg !=3D BPF_REG_0) {
 					verbose(env, "BPF_JA uses reserved fields\n");
 					return -EINVAL;
 				}
@@ -11981,8 +11971,7 @@ static int do_check(struct bpf_verifier_env *env)
 				if (BPF_SRC(insn->code) !=3D BPF_K ||
 				    insn->imm !=3D 0 ||
 				    insn->src_reg !=3D BPF_REG_0 ||
-				    insn->dst_reg !=3D BPF_REG_0 ||
-				    class =3D=3D BPF_JMP32) {
+				    insn->dst_reg !=3D BPF_REG_0) {
 					verbose(env, "BPF_EXIT uses reserved fields\n");
 					return -EINVAL;
 				}
@@ -14751,6 +14740,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_att=
r *attr, bpfptr_t uattr)
 	if (ret)
 		goto skip_full_check;
=20
+	/* checks for validity of opcodes */
 	ret =3D resolve_pseudo_ldimm64(env);
 	if (ret < 0)
 		goto skip_full_check;
--=20
2.36.1