From patchwork Tue Dec 5 19:32: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: 13480679 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="afMQgWT/" Received: from mail-qt1-x836.google.com (mail-qt1-x836.google.com [IPv6:2607:f8b0:4864:20::836]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 35B4DAB for ; Tue, 5 Dec 2023 11:33:30 -0800 (PST) Received: by mail-qt1-x836.google.com with SMTP id d75a77b69052e-423e95c2d54so59431cf.3 for ; Tue, 05 Dec 2023 11:33:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701804809; x=1702409609; 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=GbRqrgH8VyqKtobRYe59P+dsXRiu9wPR5YICgxB87x8=; b=afMQgWT/5u0OXvLcLLE3YTH4ksPNL5bV+SCiaQsICDN9V+5f542AgC181cQI42nFUo LSZxQOjPtt/QNX/9mYPyYNmQh0/OJNTCgwJXhTe5an+LnIMb6KTRCTOinsuXMhe7mdAi gtDUSDJ8EwZEx2lXsyCIw22a/LE4k1Qux4T3QAugAbcKHVtMJKMn0q1MSWTwRSL0BtBP 5YPODA9d0jnjMiN/kwTgRtrixE/ch2+0L3pyzW4mNlU1f8neHl8N4gxwpytcd9iM/42a 6eMr0KnA2hGmMs5jGCiv8Hd4jNNU5vmvFVwAnz4E/qelWbtuJ3SJ5FDwLnVipTZ51da9 aX8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701804809; x=1702409609; 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=GbRqrgH8VyqKtobRYe59P+dsXRiu9wPR5YICgxB87x8=; b=I67XIGFADYboh6kx0e8qj43Rln1ix5/1kf1aCYta/ssX3XLXCeViko5rpdKYW5EhXS Z515ZfHto2N20yWseE2H/IPGYmgWXzezGYmofK2YF4wHKeqB7JBPaBINuu9Ox8j9556m KxK0mNVLkrB6yxQ7Xvp6u2j/u9q5qtkDITvzioiEt6uXDwyDAYmRlpRXiUdUqBEGdVie DD7eZiFO909RgLHtVLfVCDCLhHrbjB89HVtPvgGw2JFJnGsDE6gM96rwehQJ1t9cW5QH iZ9zbv1Q499PiIjpvzmCV+ZE+eODBcI4W9EqR9UbgJK7PUJMD2CXglRpltoy07K40uPH gHYQ== X-Gm-Message-State: AOJu0YxdOsQn2tDTT9x+0d4M3ITcD5LVGQezqS57Br8Q1d9iXvIDdMGV C8/S+ydixPLS3uLHM37OUtSUpkYQfpc= X-Google-Smtp-Source: AGHT+IHRPmfFAs3NJDaaGK6APb50kCtInZ7nQQE8A/u0SyKsondA/W9oSJvV6wltVpreHuRFrfZnqQ== X-Received: by 2002:a05:622a:609:b0:425:4043:8d44 with SMTP id z9-20020a05622a060900b0042540438d44mr1617460qta.95.1701804808830; Tue, 05 Dec 2023 11:33:28 -0800 (PST) Received: from andrei-desktop.taildd130.ts.net ([71.125.252.241]) by smtp.gmail.com with ESMTPSA id kg18-20020a05622a761200b00421c272bcbasm5334588qtb.11.2023.12.05.11.33.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Dec 2023 11:33:28 -0800 (PST) From: Andrei Matei To: bpf@vger.kernel.org, andrii.nakryiko@gmail.com, sunhao.th@gmail.com Cc: Andrei Matei Subject: [PATCH bpf v3 1/2] bpf: fix verification of indirect var-off stack access Date: Tue, 5 Dec 2023 14:32:49 -0500 Message-Id: <20231205193250.260862-2-andreimatei1@gmail.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231205193250.260862-1-andreimatei1@gmail.com> References: <20231205193250.260862-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 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 --- 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 af2819d5c8ee..29d39ebac196 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6804,10 +6804,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) { @@ -6816,15 +6813,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 Tue Dec 5 19:32: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: 13480680 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="dII4yxi2" Received: from mail-qt1-x836.google.com (mail-qt1-x836.google.com [IPv6:2607:f8b0:4864:20::836]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 16671135 for ; Tue, 5 Dec 2023 11:33:38 -0800 (PST) Received: by mail-qt1-x836.google.com with SMTP id d75a77b69052e-423e7e0a619so83771cf.1 for ; Tue, 05 Dec 2023 11:33:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701804817; x=1702409617; 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=UKN5S5p5BrRsBONU1msN/AsNOFlD/8MoNWa9P8Foodc=; b=dII4yxi2/7hpJUHW0FM/w3qHHHdUaxEZmuTNu3t6hzs53Vlra+AMqYepW3ngMdUnsn eKour/0X3/L5vvcQbddHIgJY0vY3KeZPZPMq0KAW5eTlB+kSl7f6F+C0u9OnmuRnYN7Z WTq8Cz48X5Gh8NHgp+Yit0qR3+N5jfm5siWxuqt0TdeopE7IXe1bxsNZNAKl0MB+pma+ mj+/U09ms0x+Y3GaOxMBRojrenIIsxxVLuEgTjiY03mvWCJLFsUF6ywKwvuBHzMQ8KZU ytE/dx9lUuZFSeLGanVJgUiHRL1iR2wvsgf7UFfCg5vJa30lcDw0gsU0lmFuz5vWG808 ZHHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701804817; x=1702409617; 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=UKN5S5p5BrRsBONU1msN/AsNOFlD/8MoNWa9P8Foodc=; b=sJJ3C4zO5TBbazchqzH0xJNaHfPX3hYcDTxoDWdX74BzxVCJsuWLKGU4jjj3hW8wgC 3vfHFUqAfjsYR1L+XHspXvJBWmixGITuFadYlX15gwe+jhF1+C1ExZHYyovR7aEaToAL CeJpVF3tenAhDdwCQ11z+dibaSptmxgEmFCantwLWzb+EitAeZtbqL4PVnRf4wvH13Rw OeNuy5CUo6gbrIv9smCBm+EQJ+nRuqCXAMkwFtFAnmec/7X5qqTRsZ6DB1c2E09XwyCU kSbXpTG2J6fWmfA6HbNouT+iMALivEqPLheTYjZ4nineiENEnGOPZfAeucNNTUCeBlUk Q0ww== X-Gm-Message-State: AOJu0Yy1lM5gJKK5oun+tZm74RPVpe0rOpN3jLcfWyyZ2KM8MEQwUDk4 l5ZtuquyE0mnj+9okgfdV7NdAV10cuA= X-Google-Smtp-Source: AGHT+IGk0aL/2xKIjk4zJV2g19poWKkeCNycP1D5gAIIDfAhpvEw3uq2dIqzg0oyR5ui5/9lWkjhsw== X-Received: by 2002:a05:622a:181c:b0:418:11c9:ddb5 with SMTP id t28-20020a05622a181c00b0041811c9ddb5mr2419802qtc.25.1701804816783; Tue, 05 Dec 2023 11:33:36 -0800 (PST) Received: from andrei-desktop.taildd130.ts.net ([71.125.252.241]) by smtp.gmail.com with ESMTPSA id kg18-20020a05622a761200b00421c272bcbasm5334588qtb.11.2023.12.05.11.33.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Dec 2023 11:33:36 -0800 (PST) From: Andrei Matei To: bpf@vger.kernel.org, andrii.nakryiko@gmail.com, sunhao.th@gmail.com Cc: Andrei Matei Subject: [PATCH bpf v3 2/2] bpf: guard stack limits against 32bit overflow Date: Tue, 5 Dec 2023 14:32:50 -0500 Message-Id: <20231205193250.260862-3-andreimatei1@gmail.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231205193250.260862-1-andreimatei1@gmail.com> References: <20231205193250.260862-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 --- 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 29d39ebac196..ebebbb8feb17 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6761,7 +6761,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) { @@ -6790,7 +6790,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; @@ -6803,7 +6803,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 ||