linux-kernel - Re: [PATCH v2 bpf-next] bpf: Fix latent unsoundness in and/or/xor value tracking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <82892691-a48f-ad06-7005-5bbd7d0757eb@iogearbox.net>
Date: Wed, 3 Apr 2024 10:39:20 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Harishankar Vishwanathan <harishankar.vishwanathan@...il.com>,
 ast@...nel.org
Cc: harishankar.vishwanathan@...gers.edu, paul@...valent.com,
 Matan Shachnai <m.shachnai@...gers.edu>,
 Srinivas Narayana <srinivas.narayana@...gers.edu>,
 Santosh Nagarakatte <santosh.nagarakatte@...gers.edu>,
 John Fastabend <john.fastabend@...il.com>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
 Edward Cree <ecree@...arflare.com>, "David S. Miller" <davem@...emloft.net>,
 bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 bpf-next] bpf: Fix latent unsoundness in and/or/xor
 value tracking

On 4/2/24 11:20 PM, Harishankar Vishwanathan wrote:
[...]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index fcb62300f407..a7404a7d690f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13326,23 +13326,21 @@ static void scalar32_min_max_and(struct bpf_reg_state *dst_reg,
>   		return;
>   	}
>   
> -	/* We get our minimum from the var_off, since that's inherently
> +	/* We get our minimum from the var32_off, since that's inherently
>   	 * bitwise.  Our maximum is the minimum of the operands' maxima.
>   	 */
>   	dst_reg->u32_min_value = var32_off.value;
>   	dst_reg->u32_max_value = min(dst_reg->u32_max_value, umax_val);
> -	if (dst_reg->s32_min_value < 0 || smin_val < 0) {

The smin_val is now unused, triggering the following warnings :

./kernel/bpf/verifier.c:13321:6: warning: unused variable 'smin_val' [-Wunused-variable]
  13321 |         s32 smin_val = src_reg->s32_min_value;
        |             ^~~~~~~~
./kernel/bpf/verifier.c:13352:6: warning: unused variable 'smin_val' [-Wunused-variable]
  13352 |         s64 smin_val = src_reg->smin_value;
        |             ^~~~~~~~
./kernel/bpf/verifier.c:13386:6: warning: unused variable 'smin_val' [-Wunused-variable]
  13386 |         s32 smin_val = src_reg->s32_min_value;
        |             ^~~~~~~~
./kernel/bpf/verifier.c:13417:6: warning: unused variable 'smin_val' [-Wunused-variable]
  13417 |         s64 smin_val = src_reg->smin_value;
        |             ^~~~~~~~
./kernel/bpf/verifier.c:13451:6: warning: unused variable 'smin_val' [-Wunused-variable]
  13451 |         s32 smin_val = src_reg->s32_min_value;
        |             ^~~~~~~~
./kernel/bpf/verifier.c:13479:6: warning: unused variable 'smin_val' [-Wunused-variable]
  13479 |         s64 smin_val = src_reg->smin_value;
        |             ^~~~~~~~

Removing these builds fine then, please follow-up with a v3.

Thanks,
Daniel