From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B36B730AD0F for ; Fri, 21 Nov 2025 22:21:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763763717; cv=none; b=sbW9vRj0u7A9j8LqraZwM1BiZ2/JSn8pBKTHuAY8/THSS0U0a7JzZYIMIok6ONqcO3rH/DCZhZAXJozcUE6tGaku7UNPYdfZIYze1CZonEUswi8MAOthvnOWSelw9avhyuPNyciPijbLkXuMkvu8kf8CewoFOQ1LqDv/5elOd88= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763763717; c=relaxed/simple; bh=3mbP4oJdEUwqMn1arQ0PSlVAjMxz6nq0gDpKZ/8yC18=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=JKmcdGz5t5PHtOOd42FRU/LSmfJ4q5tqvxacGm+HoEcZuOavAm4+zrdUjNWse5ksERy0mbG8JjMMXYeQScOr917aZe/S7AZLMQu2AElKo5UWpnzvEsNlY47oeTYJzBeeSwO8dexUJc11RqIlCHsXl8wYYaH6PPzD/f7+C3I/5Jw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Yg7o7pLr; arc=none smtp.client-ip=209.85.221.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Yg7o7pLr" Received: by mail-wr1-f52.google.com with SMTP id ffacd0b85a97d-42b2a0c18caso1625546f8f.1 for ; Fri, 21 Nov 2025 14:21:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763763714; x=1764368514; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=m+9ZuFI1JtdjyJq6hwdeVp2Tnba9rAmSqaxmJ5MGXJg=; b=Yg7o7pLreDP/mhuLoA9qPdAK80O3iThcBF/gi1PrVfIQM6p4TjlQhvRW2zgQm3MDuG Qk5L/KDzdPs93hpMdjsQue7HsZLDa4NbMIA7wOK5o0UVhC78kUp1hu+thyeczyshTHBj 982KS5FzcySetUchH+aEE0X37tXrZuDoqOYFEbsclut5dcg3MCwQnL10Wjjaz7tmTj8G ntaC/MxpXGQ7edvFeUFW+7Tl2W5/DGDJ+mBIWv9ggjDtsN+1S8tPohod2zyrFDfsQXdd JmKIFDkGPxwtVqEUasx2jTrdbJbdZph0b+My7OIMi2qxrO9jQ3n4HtV+iSgKe5d9pXrI Nxpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763763714; x=1764368514; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=m+9ZuFI1JtdjyJq6hwdeVp2Tnba9rAmSqaxmJ5MGXJg=; b=u8KkjPa+BrciXChiWPVCMqsao84H1joe7UOb0+LjFBRJB1nP4I03QKvLjXIx4VcGcu x15/r8RpY7GescF7h5RGJr2BE04woTnrPkkbYCEW2+sXalJwNcbvU9cjsD73u4DubUAT 4AZR6tuf7ru0FYhoOqbi8sLr2L4N9B0ddvTUXf0zVGIZ6FrTyF6GhDjTwhiIbkHNxblj rvz1MB0CD33f7UdhneMXqOb58VhPcJo4nFXbk6P6m2bdvfMkLuQLsGRuhaNEa/1jGjjI TtC5jMbm34GoepeKsawC1WNcPwAn7LScUgBnQQHSTQGllcNj634LeQH072kSI+sIrfLh sXfw== X-Gm-Message-State: AOJu0YxjihxJybf4txGoZYkePxxzxtX+mkZTnC5/r7g1Z55JqXNLzBr8 cLRlIluvlriNl/MkbNqx8/hb1CqSTbIYKO0x5rinWtaZhFbtjm0g0zu8 X-Gm-Gg: ASbGnctA22KyRgy87/xxTVltAFScguAz9s+9ZuLbhkJt8Sv+LMAB8LIqrj+ZksaZKOW dNDRj2J9PfN4gEiP0B3hIxdrQGx7gD5IbW2bblRHII3nJe3QlxLXoiZCusMxScJwNI3zmme1jfi sSjKb85NXETIdjBbbYP4riLF7Hsoi7C1rhft+935loa4f0gyd9hW0s3YSnDcvdQ4JFAUP6d3MDj ByLZPb3Kys1GKCzJwNwUDbMnxL+ChCgqC/sxcgrV8aTDbOrb0i2Jm9VfbLEtYDRflXWs0sCcZ2U JmPAnRTAjm0UjqPNIyLVz8Fh8YOWEZm6kR8ecLhz4fR/4tEajI3HCjRNpTrbuxYit9OL/mIshsS 056hksPfnN/a7xX6r/0zkm+OgY7QffXjmXfhrv5B6DC6/bUASRaJXKOdzkfFPQqYHRuHzaSLTF3 oExRl2igyQIjR/1BTVNoR4kA2mGpycinQNGNZGDUOvluWEJXsYnTCB X-Google-Smtp-Source: AGHT+IGd6rMg3zzv9HBTrZB1Yxi0Hawm3D5oRHAYi+F3tGVuQ0Hom334ujQBjZSLME0pvZtm8O722w== X-Received: by 2002:a05:6000:184d:b0:429:d66b:508f with SMTP id ffacd0b85a97d-42cc1d0cf29mr3643189f8f.30.1763763713643; Fri, 21 Nov 2025 14:21:53 -0800 (PST) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-42cb7f34ff3sm13948627f8f.16.2025.11.21.14.21.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Nov 2025 14:21:53 -0800 (PST) Date: Fri, 21 Nov 2025 22:21:51 +0000 From: David Laight To: Alexei Starovoitov Cc: LKML , bpf , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann Subject: Re: [PATCH 06/44] bpf: Verifier, remove some unusual uses of min_t() and max_t() Message-ID: <20251121222151.7056c4fa@pumpkin> In-Reply-To: References: <20251119224140.8616-1-david.laight.linux@gmail.com> <20251119224140.8616-7-david.laight.linux@gmail.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Fri, 21 Nov 2025 13:40:36 -0800 Alexei Starovoitov wrote: > On Wed, Nov 19, 2025 at 2:42=E2=80=AFPM wr= ote: > > > > From: David Laight > > > > min_t() and max_t() are normally used to change the signedness > > of a positive value to avoid a signed-v-unsigned compare warning. > > > > However they are used here to convert an unsigned 64bit pattern > > to a signed to a 32/64bit signed number. > > To avoid any confusion use plain min()/max() and explicitely cast > > the u64 expression to the correct signed value. > > > > Use a simple max() for the max_pkt_offset calulation and delete the > > comment about why the cast to u32 is safe. > > > > Signed-off-by: David Laight > > --- > > kernel/bpf/verifier.c | 29 +++++++++++------------------ > > 1 file changed, 11 insertions(+), 18 deletions(-) > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > index ff40e5e65c43..22fa9769fbdb 100644 > > --- a/kernel/bpf/verifier.c > > +++ b/kernel/bpf/verifier.c > > @@ -2319,12 +2319,12 @@ static void __update_reg32_bounds(struct bpf_re= g_state *reg) > > struct tnum var32_off =3D tnum_subreg(reg->var_off); > > > > /* min signed is max(sign bit) | min(other bits) */ > > - reg->s32_min_value =3D max_t(s32, reg->s32_min_value, > > - var32_off.value | (var32_off.mask & S32_MIN)); > > + reg->s32_min_value =3D max(reg->s32_min_value, > > + (s32)(var32_off.value | (var32_off.mask & S32_M= IN))); > > /* max signed is min(sign bit) | max(other bits) */ > > - reg->s32_max_value =3D min_t(s32, reg->s32_max_value, > > - var32_off.value | (var32_off.mask & S32_MAX)); > > - reg->u32_min_value =3D max_t(u32, reg->u32_min_value, (u32)var3= 2_off.value); > > + reg->s32_max_value =3D min(reg->s32_max_value, > > + (s32)(var32_off.value | (var32_off.mask & S32_M= AX))); =20 >=20 > Nack. > This is plain ugly for no good reason. > Leave the code as-is. It is really horrid before. =46rom what i remember var32_off.value (and .mask) are both u64. The pattern actually patches that used a few lines down the file. I've been trying to build allmodconfig with the size test added to min_t() and max_t(). The number of real (or potentially real) bugs I've found is stunning. The only fix is to nuke min_t() and max_t() to they can't be used. The basic problem is the people have used the type of the target not that of the largest parameter. The might be ok for ulong v uint (on 64bit), but there are plenty of places where u16 and u8 are used - a lot are pretty much buggy. Perhaps the worst ones I've found are with clamp_t(), this is from 2/44: - (raw_inode)->xtime =3D cpu_to_le32(clamp_t(int32_t, (ts).tv_sec, S32_MIN= , S32_MAX)); \ + (raw_inode)->xtime =3D cpu_to_le32(clamp((ts).tv_sec, S32_MIN, S32_MAX))= ; \ If also found clamp_t(u8, xxx, 0, 255). There are just so many broken examples. David >=20 > pw-bot: cr