From patchwork Thu Dec 7 04:11:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrei Matei X-Patchwork-Id: 13482663 X-Patchwork-Delegate: bpf@iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Ku5hG8Mk" Received: from mail-qv1-xf29.google.com (mail-qv1-xf29.google.com [IPv6:2607:f8b0:4864:20::f29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0190AD5C for ; Wed, 6 Dec 2023 20:12:15 -0800 (PST) Received: by mail-qv1-xf29.google.com with SMTP id 6a1803df08f44-67acdcb3ccdso3606196d6.1 for ; Wed, 06 Dec 2023 20:12:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701922334; x=1702527134; darn=vger.kernel.org; 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=plujwPOWwzBNVBTx6fQ8sSzojYinlDykJ3bgicn6ZrA=; b=Ku5hG8MkpFPEpzrZRpuayyzxBBFPoksK8sVGqlwpetmJklKWoKkAh+jcgK8ufEUw5z JDcSxpyZtM+WEdl9zAdzNKRDxvvSKhstPWm9Tm+fv1ZxTPlsIj+YjuRqQMyPAJ6TZEL7 79CbZev+vgauQgFLYQPSMOKkVhb7lJYmsaF5Ovy7BNinPOwpLZms+0WyYsl2FXQK/FEB PBK6ZpKfBN+jdLGSnmv0a3RQ6tchxy1OWe4wuGxdJkmtbddSo9D+0vq0wt+Gd0yCl69F Dw+wi7yJmJHkTpeUJMHp7VHvNIrZb+I98/pyYTboic5Sz9Zp65c+qB9m/JOOkRSyK8dX jSYQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701922334; x=1702527134; 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=plujwPOWwzBNVBTx6fQ8sSzojYinlDykJ3bgicn6ZrA=; b=TVenxlTh7k8fXpcmTVxOrL/D1U4BHQyzmk7mR6FeXdYI+AnNaTjzODwb55vNYOg94M TmfktIIRhPk9SibZdnOKZsfhsV7ewklxnwnJJ9pe7tMytLZQmzGtgIQuV/PXzlnIVAwT c3dGSisFXa9NmR0RO/GERoxg3pP9ygov/HZ/Jbn02iNr1NWL/vK1mASY6wCGcG2G1pDr pXJGziziI1i6KFIy60GQ6s9CFl4FbwHMYy8Oa/Wz9Acnvx0E6jaF6SqBMLykYKmXdBDf h4AS/dVDRw2lPyzIOQKy2tmKNhW05SmW1u77ZcT/e7MS6vizA0rtkM4OJMdJczWN0wmG E4Yg== X-Gm-Message-State: AOJu0YyJMxaxdqlzPhaIwhRnT4iQ2baPHA+lvnllry0g7PpR8tg0gkAc oSjIOIDd8+W4P3YVwtejmOZ4WguETaNTFQ== X-Google-Smtp-Source: AGHT+IH1/Vb9Yrt9/G1wWmDQwnFML6U2Hj55bXe8tQ6gAVWnifeld/AfPrZjAEVEGne8Q4Vv0Sv+eg== X-Received: by 2002:a05:6214:511:b0:67a:97ec:7426 with SMTP id px17-20020a056214051100b0067a97ec7426mr2239112qvb.42.1701922333679; Wed, 06 Dec 2023 20:12:13 -0800 (PST) Received: from andrei-framework.verizon.net ([2600:4041:599b:1100:225d:9ebb:8c9b:7326]) by smtp.gmail.com with ESMTPSA id o6-20020a056214108600b0066cf4fa7b47sm172808qvr.4.2023.12.06.20.12.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Dec 2023 20:12:13 -0800 (PST) From: Andrei Matei To: bpf@vger.kernel.org Cc: sunhao.th@gmail.com, andrii.nakryiko@gmail.com, eddyz87@gmail.com, Andrei Matei , Andrii Nakryiko Subject: [PATCH bpf-next v5 1/3] bpf: fix verification of indirect var-off stack access Date: Wed, 6 Dec 2023 23:11:48 -0500 Message-Id: <20231207041150.229139-2-andreimatei1@gmail.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231207041150.229139-1-andreimatei1@gmail.com> References: <20231207041150.229139-1-andreimatei1@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net This patch fixes a bug around the verification of possibly-zero-sized stack accesses. When the access was done through a var-offset stack pointer, check_stack_access_within_bounds was incorrectly computing the maximum-offset of a zero-sized read to be the same as the register's min offset. Instead, we have to take in account the register's maximum possible value. The patch also simplifies how the max offset is checked; the check is now simpler than for min offset. The bug was allowing accesses to erroneously pass the check_stack_access_within_bounds() checks, only to later crash in check_stack_range_initialized() when all the possibly-affected stack slots are iterated (this time with a correct max offset). check_stack_range_initialized() is relying on check_stack_access_within_bounds() for its accesses to the stack-tracking vector to be within bounds; in the case of zero-sized accesses, we were essentially only verifying that the lowest possible slot was within bounds. We would crash when the max-offset of the stack pointer was >= 0 (which shouldn't pass verification, and hopefully is not something anyone's code attempts to do in practice). Thanks Hao for reporting! Reported-by: Hao Sun Fixes: 01f810ace9ed3 ("bpf: Allow variable-offset stack access") Closes: https://lore.kernel.org/bpf/CACkBjsZGEUaRCHsmaX=h-efVogsRfK1FPxmkgb0Os_frnHiNdw@mail.gmail.com/ Signed-off-by: Andrei Matei Acked-by: Eduard Zingerman Acked-by: Andrii Nakryiko --- kernel/bpf/verifier.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index e5ce530641ba..137240681fa9 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6620,10 +6620,7 @@ static int check_stack_access_within_bounds( if (tnum_is_const(reg->var_off)) { min_off = reg->var_off.value + off; - if (access_size > 0) - max_off = min_off + access_size - 1; - else - max_off = min_off; + max_off = min_off + access_size; } else { if (reg->smax_value >= BPF_MAX_VAR_OFF || reg->smin_value <= -BPF_MAX_VAR_OFF) { @@ -6632,15 +6629,12 @@ static int check_stack_access_within_bounds( return -EACCES; } min_off = reg->smin_value + off; - if (access_size > 0) - max_off = reg->smax_value + off + access_size - 1; - else - max_off = min_off; + max_off = reg->smax_value + off + access_size; } err = check_stack_slot_within_bounds(min_off, state, type); - if (!err) - err = check_stack_slot_within_bounds(max_off, state, type); + if (!err && max_off > 0) + err = -EINVAL; /* out of stack access into non-negative offsets */ if (err) { if (tnum_is_const(reg->var_off)) { From patchwork Thu Dec 7 04:11:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrei Matei X-Patchwork-Id: 13482664 X-Patchwork-Delegate: bpf@iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Pnypx7Hr" Received: from mail-yw1-x1129.google.com (mail-yw1-x1129.google.com [IPv6:2607:f8b0:4864:20::1129]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 328A9D73 for ; Wed, 6 Dec 2023 20:12:17 -0800 (PST) Received: by mail-yw1-x1129.google.com with SMTP id 00721157ae682-5d3644ca426so2462467b3.1 for ; Wed, 06 Dec 2023 20:12:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701922335; x=1702527135; darn=vger.kernel.org; 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=7ZVIPLfCvSCDiu0dICZQDlpADC4uzsTyBFadpnSqTwA=; b=Pnypx7Hri02a4JysVga44JLmb0+CNoTqM9ShhFdS8hNy3SbtOibQTogIGgMzaFRm2I iWa/8xQftdLZ5bGPvKsbiEozb+0/JmQ/hx2s72VXdfmGwSTwsfgaHGw/FRx4J3s6TkNn 0LgZBYcWt7aWIB2q1vH6UXIplWc/Vgkdtogm3jrCBBOvoVZeTYBrjic2LgHSu3SwstqR +rZasw1IzaYblMQsxGPRPUeEoV/ghoY8rB9jBu6oCc1R4gIU79oPUTWbniEJuA+TqLTb W1IbKjRyNwGey6um5dhI1059nQQAfSQK0ejDsGMU95VLrRZczyXR/6l8W5S4ML6xqItU rmIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701922335; x=1702527135; 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=7ZVIPLfCvSCDiu0dICZQDlpADC4uzsTyBFadpnSqTwA=; b=UTL6v4nGoQJTjEv7+DGGVJBJ0Wbnckq9RaRf06tH43NuUAGYA7J+l4t/gXoPpvFRp7 z+z0Q8JTMALOfQ24z1Yl+mkJ6iMsQqbUj4rm9B2dBZjsM5MQqXv1nEOZbp0BN/37O2yP JPKN1jW2epdtWjW9/HQ0r1sY2b7WE6wUhcybOI45YhAJ2Muf0Our1LX54t5ueK/dwF/M 38gDPL5KtuY34jWaP8iYvZLAcobN4Um1ZgLJqPvuQ03/DM90kHnIRdZXD/yo+5J8GGxx J2mL0HqnlVACLhBaGFjvqcqKvkdzWDJ441tqSTBsVLKFNT+GUSr3zWuPzp2/xRyvwdm2 dGCA== X-Gm-Message-State: AOJu0YxIQUGi1YSFPZvFvm0vlWFe8iFOP9RHL98FntDZ3Ct5DDfPwGF1 +hnMu0cPAEejQl0Jw7AxjNAh3A92nVRbLQ== X-Google-Smtp-Source: AGHT+IHXtdXdFYbMFeEKI8PVSssNtX2wDSrXHajQQ6upbi2X0ByU8vqyiOPkvW/kP/qfvEV+pHbC0A== X-Received: by 2002:a81:b71f:0:b0:5d8:17bf:c50f with SMTP id v31-20020a81b71f000000b005d817bfc50fmr1826619ywh.5.1701922335273; Wed, 06 Dec 2023 20:12:15 -0800 (PST) Received: from andrei-framework.verizon.net ([2600:4041:599b:1100:225d:9ebb:8c9b:7326]) by smtp.gmail.com with ESMTPSA id o6-20020a056214108600b0066cf4fa7b47sm172808qvr.4.2023.12.06.20.12.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Dec 2023 20:12:14 -0800 (PST) From: Andrei Matei To: bpf@vger.kernel.org Cc: sunhao.th@gmail.com, andrii.nakryiko@gmail.com, eddyz87@gmail.com, Andrei Matei Subject: [PATCH bpf-next v5 2/3] bpf: add verifier regression test for previous patch Date: Wed, 6 Dec 2023 23:11:49 -0500 Message-Id: <20231207041150.229139-3-andreimatei1@gmail.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231207041150.229139-1-andreimatei1@gmail.com> References: <20231207041150.229139-1-andreimatei1@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net Add a regression test for var-off zero-sized reads. Acked-by: Eduard Zingerman --- .../selftests/bpf/progs/verifier_var_off.c | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/verifier_var_off.c b/tools/testing/selftests/bpf/progs/verifier_var_off.c index 83a90afba785..b7bdd7db3a35 100644 --- a/tools/testing/selftests/bpf/progs/verifier_var_off.c +++ b/tools/testing/selftests/bpf/progs/verifier_var_off.c @@ -224,6 +224,35 @@ __naked void access_max_out_of_bound(void) : __clobber_all); } +/* Similar to the test above, but this time check the special case of a + * zero-sized stack access. We used to have a bug causing crashes for zero-sized + * out-of-bounds accesses. + */ +SEC("socket") +__description("indirect variable-offset stack access, zero-sized, max out of bound") +__failure __msg("invalid variable-offset indirect access to stack R1") +__naked void zero_sized_access_max_out_of_bound(void) +{ + asm volatile (" \ + r0 = 0; \ + /* Fill some stack */ \ + *(u64*)(r10 - 16) = r0; \ + *(u64*)(r10 - 8) = r0; \ + /* Get an unknown value */ \ + r1 = *(u32*)(r1 + 0); \ + r1 &= 63; \ + r1 += -16; \ + /* r1 is now anywhere in [-16,48) */ \ + r1 += r10; \ + r2 = 0; \ + r3 = 0; \ + call %[bpf_probe_read_kernel]; \ + exit; \ +" : + : __imm(bpf_probe_read_kernel) + : __clobber_all); +} + SEC("lwt_in") __description("indirect variable-offset stack access, min out of bound") __failure __msg("invalid variable-offset indirect access to stack R2") From patchwork Thu Dec 7 04:11:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrei Matei X-Patchwork-Id: 13482665 X-Patchwork-Delegate: bpf@iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Eu70WAmz" Received: from mail-qk1-x72a.google.com (mail-qk1-x72a.google.com [IPv6:2607:f8b0:4864:20::72a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 72271D5C for ; Wed, 6 Dec 2023 20:12:18 -0800 (PST) Received: by mail-qk1-x72a.google.com with SMTP id af79cd13be357-77ecedad216so13038885a.3 for ; Wed, 06 Dec 2023 20:12:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701922337; x=1702527137; darn=vger.kernel.org; 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=JJNOZXW/19vdyac9iHcxbBhS2H5cmLHne945AbcBJZg=; b=Eu70WAmzwT2TMGY0rU/OkBChP3/qM5dfFatpieBMjRl63i6si7pnqjc50YxzZdm/LB GgAdvy83GJRUkaNtUfejXMDnbVxGeh9aPd0JkTW+MXZ2HxNkt52XW7KdnecO4Ly0tKZd V+Y9aCCcDPIpwFgiMeZSM43mcWj0XHWi7EhECBKZ3el9RoCxQeZ0ebTINSnrvBJNo39D 8rzuf1jyy5HSWcEEMyw2MSOf+txKRge4+dwYIfeNQ9IKm+3yVUnB5/alx2b/5m4J2W7l un6Y+eoo2zwGw6M0JCBwjvcmSnw4UFNBwB64fZ02HlfShoQM8IOHlt9ScQaXFl8QkR2t eyww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701922337; x=1702527137; 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=JJNOZXW/19vdyac9iHcxbBhS2H5cmLHne945AbcBJZg=; b=QnbeVXf28bsVoGhSQjjoRjBdsNWMSf03IzLmmAhOlemdeidHb+rXoPco5gsiTB11uP oz9GdvltI3D/efATabp2F2XTUZ2eE8c/Z2DSx/iLM/eca49DQVsPLD0vE+JAZIxRrOxB uc3VKKfO1pPiS6bk0/C6YHuFvYHhPY8d0JFcYTg/xp0FJMvBdm+iG3ZDClo5UGyu4oLK 5zQVUoGSqqj1kl6EjkIKPh6TpM9v/k7vRMwzVAVcme7xDy3xMxswYtKqjsYLFr9JM5gM sKI86/3rK24sY+EmY5ToaqGrLa1r13MCxfv7WLB6MAjOd/vmQlQb3g7jFTxJQKFSEyZV joLA== X-Gm-Message-State: AOJu0Yxo4pbP20KmfEPhqCr7qfVaEVIT0GzJETXXCU4FYzhaFi+BbZ5k WRQTipCUzZdKTNkA24J//UCrtqPX1KQHzw== X-Google-Smtp-Source: AGHT+IHisPmqs1x1oPJg1GBYRGZ69mGfuA81o+/VccNwrbkIY+LmTPOEWZbxSikb/voJl2tG+KAX1Q== X-Received: by 2002:a05:6214:805:b0:67a:b459:b594 with SMTP id df5-20020a056214080500b0067ab459b594mr1983322qvb.119.1701922336610; Wed, 06 Dec 2023 20:12:16 -0800 (PST) Received: from andrei-framework.verizon.net ([2600:4041:599b:1100:225d:9ebb:8c9b:7326]) by smtp.gmail.com with ESMTPSA id o6-20020a056214108600b0066cf4fa7b47sm172808qvr.4.2023.12.06.20.12.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Dec 2023 20:12:16 -0800 (PST) From: Andrei Matei To: bpf@vger.kernel.org Cc: sunhao.th@gmail.com, andrii.nakryiko@gmail.com, eddyz87@gmail.com, Andrei Matei , Andrii Nakryiko Subject: [PATCH bpf-next v5 3/3] bpf: guard stack limits against 32bit overflow Date: Wed, 6 Dec 2023 23:11:50 -0500 Message-Id: <20231207041150.229139-4-andreimatei1@gmail.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231207041150.229139-1-andreimatei1@gmail.com> References: <20231207041150.229139-1-andreimatei1@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net This patch promotes the arithmetic around checking stack bounds to be done in the 64-bit domain, instead of the current 32bit. The arithmetic implies adding together a 64-bit register with a int offset. The register was checked to be below 1<<29 when it was variable, but not when it was fixed. The offset either comes from an instruction (in which case it is 16 bit), from another register (in which case the caller checked it to be below 1<<29 [1]), or from the size of an argument to a kfunc (in which case it can be a u32 [2]). Between the register being inconsistently checked to be below 1<<29, and the offset being up to an u32, it appears that we were open to overflowing the `int`s which were currently used for arithmetic. [1] https://github.com/torvalds/linux/blob/815fb87b753055df2d9e50f6cd80eb10235fe3e9/kernel/bpf/verifier.c#L7494-L7498 [2] https://github.com/torvalds/linux/blob/815fb87b753055df2d9e50f6cd80eb10235fe3e9/kernel/bpf/verifier.c#L11904 Signed-off-by: Andrei Matei Reported-by: Andrii Nakryiko Acked-by: Andrii Nakryiko --- kernel/bpf/verifier.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 137240681fa9..6832ed743765 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6577,7 +6577,7 @@ static int check_ptr_to_map_access(struct bpf_verifier_env *env, * The minimum valid offset is -MAX_BPF_STACK for writes, and * -state->allocated_stack for reads. */ -static int check_stack_slot_within_bounds(int off, +static int check_stack_slot_within_bounds(s64 off, struct bpf_func_state *state, enum bpf_access_type t) { @@ -6606,7 +6606,7 @@ static int check_stack_access_within_bounds( struct bpf_reg_state *regs = cur_regs(env); struct bpf_reg_state *reg = regs + regno; struct bpf_func_state *state = func(env, reg); - int min_off, max_off; + s64 min_off, max_off; int err; char *err_extra; @@ -6619,7 +6619,7 @@ static int check_stack_access_within_bounds( err_extra = " write to"; if (tnum_is_const(reg->var_off)) { - min_off = reg->var_off.value + off; + min_off = (s64)reg->var_off.value + off; max_off = min_off + access_size; } else { if (reg->smax_value >= BPF_MAX_VAR_OFF ||