linux-kernel - [PATCH V2] NFS: Fix shift-out-of-bounds UBSAN warning with negative retrans

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>] [day] [month] [year] [list]

Message-ID: <20250408031504.2549173-1-wangzhaolong1@huawei.com>
Date: Tue, 8 Apr 2025 11:15:04 +0800
From: Wang Zhaolong <wangzhaolong1@...wei.com>
To: <trondmy@...nel.org>, <anna@...nel.org>, <rdunlap@...radead.org>
CC: <linux-nfs@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<wangzhaolong1@...wei.com>, <yi.zhang@...wei.com>, <yangerkun@...wei.com>
Subject: [PATCH V2] NFS: Fix shift-out-of-bounds UBSAN warning with negative retrans

The previous commit c09f11ef3595 ("NFS: fs_context: validate UDP retrans
to prevent shift out-of-bounds") added a check to prevent shift values
that are too large, but it fails to account for negative retrans values.
When data->retrans is negative, the condition `data->retrans >= 64` is
skipped, allowing negative values to be copied to context->retrans,
which is unsigned. This results in a large positive number that can
trigger the original UBSAN issue[1].

This issue has actually been present since very early kernels (dating
back to 2.6.x series), as the code has historically not validated
retrans values from userspace. When these large or negative values are
later used as unsigned in bit shift operations, they cause the UBSAN
warning.

This patch modifies the check to explicitly handle both negative values
and values that are too large, completing the fix that was partially
implemented by c09f11ef3595.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=219988
Fixes: c09f11ef3595 ("NFS: fs_context: validate UDP retrans to prevent shift out-of-bounds")
Signed-off-by: Wang Zhaolong <wangzhaolong1@...wei.com>
---
 fs/nfs/fs_context.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 13f71ca8c974..0703ac0349cb 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -1161,11 +1161,12 @@ static int nfs23_parse_monolithic(struct fs_context *fc,
 		 * for proto == XPRT_TRANSPORT_UDP, which is what uses
 		 * to_exponential, implying shift: limit the shift value
 		 * to BITS_PER_LONG (majortimeo is unsigned long)
 		 */
 		if (!(data->flags & NFS_MOUNT_TCP)) /* this will be UDP */
-			if (data->retrans >= 64) /* shift value is too large */
+			/* Reject invalid retrans values (negative or too large) */
+			if (data->retrans < 0 || data->retrans >= 64)
 				goto out_invalid_data;

 		/*
 		 * Translate to nfs_fs_context, which nfs_fill_super
 		 * can deal with.
-- 
2.34.3