From patchwork Thu Jul 11 08:07:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zac Ecob X-Patchwork-Id: 13730148 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-4322.protonmail.ch (mail-4322.protonmail.ch [185.70.43.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4823F51004 for ; Thu, 11 Jul 2024 08:07:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.70.43.22 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720685279; cv=none; b=ObYW2uBE1vn08bco9CUHMc/A95ZImBzOMU7F16CGnVY7J0h5tLPNT2BclP37FXxUnOrUzQVhfeCnEbfPkHXIO/Xzpgem2hy6ixOkX7g/7nJ/Jd6z5x5cMPsCGxyqety8i+x+FUHEjcBLOhP34w2i1+kgIL25cvym04y3QiUIjYM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720685279; c=relaxed/simple; bh=OxQXMI4hwADNPnDDsNvITF62UjyRKuL78WHsK880+J0=; h=Date:To:From:Subject:Message-ID:MIME-Version:Content-Type; b=uYsJzDlA6XvW5P0W11CrloMZnq1CaeNKSUqUkZTiYPA0IRNJ09ac3BToQptUziMyAJ7lctv7L6JEmIxnFs0v9iPrYKrjow+C5LIgxZlJ+kzpbJ13qP9u3KBWnmBZccilMwG4T+iu3oQLuOLOC8m197XVw/4NFY78xb4eAEuw3mc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=protonmail.com; spf=pass smtp.mailfrom=protonmail.com; dkim=pass (2048-bit key) header.d=protonmail.com header.i=@protonmail.com header.b=zXo89cYO; arc=none smtp.client-ip=185.70.43.22 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=protonmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=protonmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=protonmail.com header.i=@protonmail.com header.b="zXo89cYO" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1720685275; x=1720944475; bh=/z57iun9sRmEOEOAOX7K0UhzGB5n8q1MbGzXlOVaqgA=; h=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date: Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector; b=zXo89cYObDsg3RaqPoXGRUcn7EWlkvTJkQoabmih7YAvRe8mBY7K7BZjW3UWXsqi8 /3ky24iCr7sIP7N593EhEJHKkH53z2vJSzrw3p+sl8vb2VESY38hsiFwdVIZNdonE6 +J9miaTb1Y9z/TleqOLWsShTRIBJ60+mqM3suLNbw5UdoXxGE7F9CnyfOFYuW2vOno DkfiinX+5b6WWEpB3iuoPZlpjgrIeIEMGIXDLxEr1ItAcJAITLLvp0kR5pJ9M4RUZR xPlFdk2mNJKuFXFTaSbtZCWT4OCbIzeE6DlcIE+e9APxPdZ5SRvw22HWkVhB/u3jnh SmmMFqMQw5/tw== Date: Thu, 11 Jul 2024 08:07:49 +0000 To: "bpf@vger.kernel.org" From: Zac Ecob Subject: Fixing coerce_subreg_to_size_sx invalidly setting reg->umax_value Message-ID: Feedback-ID: 29112261:user:proton X-Pm-Message-ID: b6b1780a07af3b1d166710a38df49b736e0d6143 Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Hi, My fuzzer recently found another bug, in which `reg->umax_value` is being invalidly set in regards to sign extensions. The lines below contain the bug: ``` reg->umin_value = reg->u32_min_value = s64_min; reg->umax_value = reg->u32_max_value = s64_max; ``` If `s64_min` / `s64_max` are negative values here, they correctly cast when assigning to the u32 values. However, when assigned to `umin_value` / `umax_value`, it seems there is an implicit (u32) cast applied, causing the top 32 bits to not be set. I've attached the files to reproduce, as well as the patch file, based off of 6.10-rc4 - albeit this is my first patch so I'd appreciate someone checking it's formatted fine. Thanks. From da5ef523f7cd018f3f0991454a18bc961ea1abba Mon Sep 17 00:00:00 2001 From: Zac Ecob Date: Thu, 11 Jul 2024 17:41:55 +1000 Subject: [PATCH] Fixed sign-extension issue in coerce_subreg_to_size_sx --- kernel/bpf/verifier.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 010a6eb864dc..eccf3ac8996a 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6213,8 +6213,14 @@ static void coerce_reg_to_size_sx(struct bpf_reg_state *reg, int size) if ((s64_max >= 0) == (s64_min >= 0)) { reg->smin_value = reg->s32_min_value = s64_min; reg->smax_value = reg->s32_max_value = s64_max; - reg->umin_value = reg->u32_min_value = s64_min; - reg->umax_value = reg->u32_max_value = s64_max; + + // Cannot chain assignments, like reg->umax_val = reg->u32_max_val = (signed input) + // Because of the implicit cast leading to reg->umax_val not being properly set for negative numbers + reg->u32_min_value = s64_min; + reg->u32_max_value = s64_max; + reg->umin_value = s64_min; + reg->umax_value = s64_max; + reg->var_off = tnum_range(s64_min, s64_max); return; } -- 2.30.2