From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-f179.google.com (mail-oi1-f179.google.com [209.85.167.179]) (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 44A0F2ED87C for ; Mon, 19 Jan 2026 17:12:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768842739; cv=none; b=OdVwsS6bHdW3YM7PPiCqog7mvQSuWkgA3rqsPqxyba63bf/mM4k11VAtXNmAyfgnUQQVbIJl3xruUbO0fnnbFSfETs1L+fs+FJn/vAprODHHIqNIwtLvU3Ogl411EIM+PNPR4UbNc5X7+5/m1s33DQxaHzyz5BURLdd2IgNhyVE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768842739; c=relaxed/simple; bh=Es8fWJIXZFvBKNnhHl4h+J2YcPiwozZIBFfu63J/rAc=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=k8BkwUU6GByATHgp2Fbet8GFtgeVf9RvDJzBNiJKOGj7iItVp3zSunFQZGLcOoWdDEoWksweJa/3Oitc028WBGWF3QTDBlwX5J4WNEKsbpVSjPVZSLwb6H+YQjVjxObcOqvlRqlk2QG3sVJT852Mok0FgLLQi0v0j/evIYqWBGI= 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=bI3gltkf; arc=none smtp.client-ip=209.85.167.179 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="bI3gltkf" Received: by mail-oi1-f179.google.com with SMTP id 5614622812f47-45c8e85deffso1584145b6e.1 for ; Mon, 19 Jan 2026 09:12:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1768842737; x=1769447537; 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=VF5ppv2Sxgm2Nlm7d7GJMxqNR8PWC8FPp4y5+/xcFc0=; b=bI3gltkfPjhGmUoBuxzYKKYt35oy6ke51+Z2MVw36RX7Xcb6fl5zqvd2Q/LIIrMmao Ch7zYukEKxjjmJ6dBmcTAo0ifjx79jo+KhYL/H3R+U1+8PkM08kzkdHmaBVrS3dTB+7d LsbJ2VtF4Gyi82zJyJt7Jsev6fbdV49rNfLXZ9v1iG71hywtnSANSH/emilyICnHaPOk xXnjFfZn8rfVDtfMeRTidbXCTaOz8yy0CSbqfyUcu/Qh8GeRnpgvdY5B9xz8M4ZGPQyt +4dTiKC9ZG7vLpryMTYaZedEq+koqFZIKPmPewVPD1nMBCM9Uc6zr6eOVSowG+POGEeZ cpBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768842737; x=1769447537; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=VF5ppv2Sxgm2Nlm7d7GJMxqNR8PWC8FPp4y5+/xcFc0=; b=Pa6I04kTwkM8DpX7dWvZw12cGqAOmYNh8TAOchofnzLwcdVAjgQ5lLHWeLl+hU3aR3 9/RB2tL1JQ8HYV0a8TvdMI60ia8F6zKjGcS7iSwyz91r+DjmqyqkwN69vR0pSsJiUyQi laO1TQT5Ynhzk6bqgREvAX2grI3AdH7KwJIfPiVhP8Ahn9ltYdI5wiNfCvWOkVfLCssM yKqCssgeD00sjHgFmxiUwoO48XBYFORFndHzpzQUVNxM8sWKfjzKD/muaaUAGwf9qrsz j3XRKqLAKy6IESQGCLzBrK94uYUxwP4By76014ehqu0FWuccWLq8/abe+Wv+h5nAoF39 Tjjg== X-Forwarded-Encrypted: i=1; AJvYcCWuEjJd4GfrOkOgYJTZJi8sUAV9cNbfyvtD7XQYg4rtxOoAVtdQhxD3OJqc1Iv5sKA2m00PchUjC8BrEbc=@vger.kernel.org X-Gm-Message-State: AOJu0Yz7oifDlUBKW2weu28FJyn248WJWr/bphCLcY4Lj9wAFzR8Tvf0 CnvIIgwFHd38Y9M6OVS+t6ZKY4h/iKrvA3kfzWTF8RVAYQmOtHpO7ddsWT3+3g== X-Gm-Gg: AY/fxX7r9UeVbnFKApI7ecqzOYu3H6Q5JgC27JW4o788HaJYWCvoZIk89k6W/zWS6mP 4wCSgc2iEc2SkH44vdb1nVwXBj+n4bNz0juX9F0rseBdevPl+1i7mhbYigdySGhH2FZAI7WvoKM XrUWJAQcK63Lc9ei6RZDNFl0/1Zv8vPplPrl9/SPkF6Jp49yKFqZPm6NB+iV1CiQU7IkR5QxGpH NZaTe3O1s3FKxyU8dnf+kycvTd3cJW/gVy9uylTBkzkkGedhIRUB+GxD0V6Vn5f5yOE0WgmSS8O ubE32njWrCwonTs3kxFxnbVCDBA1sGm1v3bprqxhDYXKIa2zBXNqFX23J/VFoaQWBiWiTlmj33c Mae10/Ey1xkDjnLeVsdlrowDHZuU0jHBu7rEXlds84TXrh41PgVFsF+tyxovki34mSeDU/6gaZq WGASC3/JTr/BDmcvF9VbmmHhxF6ODiHZCr X-Received: by 2002:a05:6808:159b:b0:456:9f44:da9f with SMTP id 5614622812f47-45c9d4ee409mr5551384b6e.0.1768842737066; Mon, 19 Jan 2026 09:12:17 -0800 (PST) Received: from newman.cs.purdue.edu ([128.10.127.250]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-7cfdf2a5f6dsm7061688a34.20.2026.01.19.09.12.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Jan 2026 09:12:16 -0800 (PST) From: Jiasheng Jiang To: heming.zhao@suse.com Cc: jiashengjiangcool@gmail.com, jlbec@evilplan.org, joseph.qi@linux.alibaba.com, linux-kernel@vger.kernel.org, mark@fasheh.com, ocfs2-devel@lists.linux.dev Subject: Re: [PATCH v2] ocfs2: Fix NULL pointer dereference in ocfs2_get_refcount_rec() Date: Mon, 19 Jan 2026 17:12:15 +0000 Message-Id: <20260119171215.1076-1-jiashengjiangcool@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <5bflve5fog7jreyjzn4po47t2wzd6vomsmlkd6km44joogcybl@6vd2h5orgwge> References: <5bflve5fog7jreyjzn4po47t2wzd6vomsmlkd6km44joogcybl@6vd2h5orgwge> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit On Mon, 19 Jan 2026 22:43:26 +0800, Heming Zhao wrote: > On Sun, Jan 18, 2026 at 07:05:23PM +0000, Jiasheng Jiang wrote: >> In ocfs2_get_refcount_rec(), the 'rec' pointer is initialized to NULL. >> If the extent list is empty (el->l_next_free_rec == 0), the loop skips >> assignment, leaving 'rec' as NULL and 'found' as 0. >> >> Currently, the code skips the 'if (found)' block but proceeds directly to >> dereference 'rec' at line 767 (le64_to_cpu(rec->e_blkno)), causing a >> NULL pointer dereference panic. > > Do you have reproduction steps to support your analysis? > Thanks for your review. We found this issue through static analysis and code review. We do not have a specific reproduction script or a corrupted filesystem image to trigger this at the moment. > there are two types of 'rb': leaf or tree. > the check 'if (!(le32_to_cpu(rb->rf_flags) & OCFS2_REFCOUNT_TREE_FL))' > handles leaf case and returns to the caller. > the subsequent code handles the 'tree' type, therefore, in theory, > el->l_next_free_rec should be ">= 1", and the execution should enter the > for-loop to assign the 'rec'. > > Thanks, > Heming > You are absolutely correct. Theoretically, if the OCFS2_REFCOUNT_TREE_FL flag is set, the extent list should not be empty, and el->l_next_free_rec should be >= 1. However, if the filesystem metadata is corrupted (e.g., due to bit rot or software bugs), it is possible that l_next_free_rec reads as 0 even for a tree root. In the current implementation, such inconsistency leads to a NULL pointer dereference and panics the system. This patch is intended as a hardening measure. Instead of crashing the kernel when encountering such invalid on-disk data, it is safer to report a filesystem error via ocfs2_error and abort the operation gracefully. This approach aligns with recent fixes in OCFS2. For example, Commit 44acc46d182f ("ocfs2: avoid NULL pointer dereference in dx_dir_lookup_rec()") addressed an identical issue where an empty extent list left the 'rec' pointer NULL. I will update the commit message in the next version to explicitly state that this is a hardening measure against on-disk corruption, ensuring the rationale is accurate. >> >> This patch adds an 'else' branch to the 'if (found)' check. If no valid >> record is found, it reports a filesystem error and exits, preventing >> the invalid memory access. >> >> Fixes: e73a819db9c2 ("ocfs2: Add support for incrementing refcount in the tree.") >> Signed-off-by: Jiasheng Jiang >> --- >> Changelog: >> >> v1 -> v2: >> >> 1. Add a Fixes tag. >> --- >> fs/ocfs2/refcounttree.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c >> index c92e0ea85bca..464bdd6e0a8e 100644 >> --- a/fs/ocfs2/refcounttree.c >> +++ b/fs/ocfs2/refcounttree.c >> @@ -1122,6 +1122,11 @@ static int ocfs2_get_refcount_rec(struct ocfs2_caching_info *ci, >> >> if (cpos_end < low_cpos + len) >> len = cpos_end - low_cpos; >> + } else { >> + ret = ocfs2_error(sb, "Refcount tree %llu has no extent record covering cpos %u\n", >> + (unsigned long long)ocfs2_metadata_cache_owner(ci), >> + low_cpos); >> + goto out; >> } >> >> ret = ocfs2_read_refcount_block(ci, le64_to_cpu(rec->e_blkno), >> -- >> 2.25.1 >> >>