[PATCH] nilfs2: validate file block counts during recovery

[PATCH] nilfs2: validate file block counts during recovery

[Aldo Ariel Panzardo]

nilfs_scan_dsync_log() trusts the block counts in each on-disk file
information entry. If fi_ndatablk is greater than fi_nblocks, the data
block loop can consume excessive summary entries and the later subtraction
used to derive the number of node blocks underflows.

Reject inconsistent file information entries before consuming their block
information.

Fixes: 0f3e1c7f23f8 ("nilfs2: recovery functions")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 fs/nilfs2/recovery.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
index 4d5a6aa521..6e4e0cf4cc 100644
--- a/fs/nilfs2/recovery.c
+++ b/fs/nilfs2/recovery.c
@@ -322,6 +322,7 @@ static void nilfs_skip_summary_info(struct the_nilfs *nilfs,
  *
  * Return: 0 on success, or one of the following negative error codes on
  * failure:
+ * * %-EINVAL	- Invalid block counts in a file information entry.
  * * %-EIO	- I/O error.
  * * %-ENOMEM	- Insufficient memory available.
  */
@@ -359,6 +360,10 @@ static int nilfs_scan_dsync_log(struct the_nilfs *nilfs, sector_t start_blocknr,
 		ino = le64_to_cpu(finfo->fi_ino);
 		nblocks = le32_to_cpu(finfo->fi_nblocks);
 		ndatablk = le32_to_cpu(finfo->fi_ndatablk);
+		if (ndatablk > nblocks) {
+			err = -EINVAL;
+			goto out;
+		}
 		nnodeblk = nblocks - ndatablk;
 
 		while (ndatablk-- > 0) {
-- 
2.43.0

Re: [PATCH] nilfs2: validate file block counts during recovery

[Viacheslav Dubeyko]

On Tue, 2026-09-15 at 08:13 -0300, Aldo Ariel Panzardo wrote:
> nilfs_scan_dsync_log() trusts the block counts in each on-disk file
> information entry. If fi_ndatablk is greater than fi_nblocks, the
> data
> block loop can consume excessive summary entries and the later
> subtraction
> used to derive the number of node blocks underflows.
> 
> Reject inconsistent file information entries before consuming their
> block
> information.
> 
> Fixes: 0f3e1c7f23f8 ("nilfs2: recovery functions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
>  fs/nilfs2/recovery.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
> index 4d5a6aa521..6e4e0cf4cc 100644
> --- a/fs/nilfs2/recovery.c
> +++ b/fs/nilfs2/recovery.c
> @@ -322,6 +322,7 @@ static void nilfs_skip_summary_info(struct
> the_nilfs *nilfs,
>   *
>   * Return: 0 on success, or one of the following negative error
> codes on
>   * failure:
> + * * %-EINVAL	- Invalid block counts in a file information entry.
>   * * %-EIO	- I/O error.
>   * * %-ENOMEM	- Insufficient memory available.
>   */
> @@ -359,6 +360,10 @@ static int nilfs_scan_dsync_log(struct the_nilfs
> *nilfs, sector_t start_blocknr,
>  		ino = le64_to_cpu(finfo->fi_ino);
>  		nblocks = le32_to_cpu(finfo->fi_nblocks);
>  		ndatablk = le32_to_cpu(finfo->fi_ndatablk);
> +		if (ndatablk > nblocks) {
> +			err = -EINVAL;

It sounds like -EIO because we have corrupted state of on-disk
metadata.

Thanks,
Slava.

> +			goto out;
> +		}
>  		nnodeblk = nblocks - ndatablk;
>  
>  		while (ndatablk-- > 0) {

Re: [PATCH] nilfs2: validate file block counts during recovery

[Aldo Ariel Panzardo]

Hi Slava,

Right, -EIO is more appropriate for corrupt on-disk metadata. Will
fix in v2.

Aldo

[PATCH v2] nilfs2: validate file block counts during recovery

[Aldo Ariel Panzardo]

nilfs_scan_dsync_log() trusts the block counts in each on-disk file
information entry. If fi_ndatablk is greater than fi_nblocks, the data
block loop can consume excessive summary entries and the later subtraction
used to derive the number of node blocks underflows.

Reject inconsistent file information entries before consuming their block
information.

Fixes: 0f3e1c7f23f8 ("nilfs2: recovery functions")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 fs/nilfs2/recovery.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
index 4d5a6aa521..28200cef4d 100644
--- a/fs/nilfs2/recovery.c
+++ b/fs/nilfs2/recovery.c
@@ -359,6 +359,10 @@ static int nilfs_scan_dsync_log(struct the_nilfs *nilfs, sector_t start_blocknr,
 		ino = le64_to_cpu(finfo->fi_ino);
 		nblocks = le32_to_cpu(finfo->fi_nblocks);
 		ndatablk = le32_to_cpu(finfo->fi_ndatablk);
+		if (ndatablk > nblocks) {
+			err = -EIO;
+			goto out;
+		}
 		nnodeblk = nblocks - ndatablk;
 
 		while (ndatablk-- > 0) {
-- 
2.43.0

Re: [PATCH v2] nilfs2: validate file block counts during recovery

[Viacheslav Dubeyko]

On Tue, 2026-09-15 at 16:57 -0300, Aldo Ariel Panzardo wrote:
> nilfs_scan_dsync_log() trusts the block counts in each on-disk file
> information entry. If fi_ndatablk is greater than fi_nblocks, the
> data
> block loop can consume excessive summary entries and the later
> subtraction
> used to derive the number of node blocks underflows.
> 
> Reject inconsistent file information entries before consuming their
> block
> information.
> 
> Fixes: 0f3e1c7f23f8 ("nilfs2: recovery functions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
>  fs/nilfs2/recovery.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
> index 4d5a6aa521..28200cef4d 100644
> --- a/fs/nilfs2/recovery.c
> +++ b/fs/nilfs2/recovery.c
> @@ -359,6 +359,10 @@ static int nilfs_scan_dsync_log(struct the_nilfs
> *nilfs, sector_t start_blocknr,
>  		ino = le64_to_cpu(finfo->fi_ino);
>  		nblocks = le32_to_cpu(finfo->fi_nblocks);
>  		ndatablk = le32_to_cpu(finfo->fi_ndatablk);
> +		if (ndatablk > nblocks) {
> +			err = -EIO;
> +			goto out;
> +		}
>  		nnodeblk = nblocks - ndatablk;
>  
>  		while (ndatablk-- > 0) {

Looks good.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.

Re: [PATCH] nilfs2: validate file block counts during recovery

[Ryusuke Konishi]

On Tue, Sep 15, 2026 at 8:14 PM Aldo Ariel Panzardo wrote:
>
> nilfs_scan_dsync_log() trusts the block counts in each on-disk file
> information entry. If fi_ndatablk is greater than fi_nblocks, the data
> block loop can consume excessive summary entries and the later subtraction
> used to derive the number of node blocks underflows.
>
> Reject inconsistent file information entries before consuming their block
> information.
>
> Fixes: 0f3e1c7f23f8 ("nilfs2: recovery functions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
>  fs/nilfs2/recovery.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
> index 4d5a6aa521..6e4e0cf4cc 100644
> --- a/fs/nilfs2/recovery.c
> +++ b/fs/nilfs2/recovery.c
> @@ -322,6 +322,7 @@ static void nilfs_skip_summary_info(struct the_nilfs *nilfs,
>   *
>   * Return: 0 on success, or one of the following negative error codes on
>   * failure:
> + * * %-EINVAL  - Invalid block counts in a file information entry.
>   * * %-EIO     - I/O error.
>   * * %-ENOMEM  - Insufficient memory available.
>   */
> @@ -359,6 +360,10 @@ static int nilfs_scan_dsync_log(struct the_nilfs *nilfs, sector_t start_blocknr,
>                 ino = le64_to_cpu(finfo->fi_ino);
>                 nblocks = le32_to_cpu(finfo->fi_nblocks);
>                 ndatablk = le32_to_cpu(finfo->fi_ndatablk);
> +               if (ndatablk > nblocks) {
> +                       err = -EINVAL;
> +                       goto out;
> +               }
>                 nnodeblk = nblocks - ndatablk;
>
>                 while (ndatablk-- > 0) {
> --
> 2.43.0

Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>

Hi Viacheslav,

Please apply this v1 patch instead of the v2 patch.

This function is called as part of mount operations.  When format
errors are detected during superblock reading or log scanning at mount
time, returning -EINVAL rather than -EIO is in line with the mount
system call behavior, so the v1 implementation is the correct one.

Also, please replace the 'Cc: stable' tag with the following tag, as
with the previous patch:

Cc: stable+noautosel@kernel.org # Non-fatal bug fix; defer backport
until a real issue is reported

I actually ran a test where a pseudo underflow of the nnodeblk
variable occurred, but no issues that compromise system stability
happened.

Even if nnodeblk becomes a large value, long-duration block device
scanning does not occur; it simply skips the position and abandons
roll-forward recovery midway.  Since the mount normally succeeds, if
anything, this silent failure is the issue.

However, this is based on the assumption that the file system image
was intentionally tampered with (anything goes) with root privileges,
including log checksums.  Therefore, as long as it does not break the
system, I do not believe it meets the stable kernel rules for
backporting.

Thanks,
Ryusuke Konishi