[PATCH 6.1] Squashfs: check the inode number is not the invalid value of zero

[PATCH 6.1] Squashfs: check the inode number is not the invalid value of zero

[Xiangyu Chen]

From: Phillip Lougher <phillip@squashfs.org.uk>

[ upstream commit 9253c54e01b6505d348afbc02abaa4d9f8a01395 ]

Syskiller has produced an out of bounds access in fill_meta_index().

That out of bounds access is ultimately caused because the inode
has an inode number with the invalid value of zero, which was not checked.

The reason this causes the out of bounds access is due to following
sequence of events:

1. Fill_meta_index() is called to allocate (via empty_meta_index())
   and fill a metadata index.  It however suffers a data read error
   and aborts, invalidating the newly returned empty metadata index.
   It does this by setting the inode number of the index to zero,
   which means unused (zero is not a valid inode number).

2. When fill_meta_index() is subsequently called again on another
   read operation, locate_meta_index() returns the previous index
   because it matches the inode number of 0.  Because this index
   has been returned it is expected to have been filled, and because
   it hasn't been, an out of bounds access is performed.

This patch adds a sanity check which checks that the inode number
is not zero when the inode is created and returns -EINVAL if it is.

[phillip@squashfs.org.uk: whitespace fix]
  Link: https://lkml.kernel.org/r/20240409204723.446925-1-phillip@squashfs.org.uk
Link: https://lkml.kernel.org/r/20240408220206.435788-1-phillip@squashfs.org.uk
Signed-off-by: Phillip Lougher <phillip@squashfs.org.uk>
Reported-by: "Ubisectech Sirius" <bugreport@ubisectech.com>
Closes: https://lore.kernel.org/lkml/87f5c007-b8a5-41ae-8b57-431e924c5915.bugreport@ubisectech.com/
Cc: Christian Brauner <brauner@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
Verified on qemux86-64. 
The test code from https://lore.kernel.org/lkml/87f5c007-b8a5-41ae-8b57-431e924c5915.bugreport@ubisectech.com/
Test code would trigger a kernel crash (crash point at read_blocklist) and the crash won't happen anymore after
applying this commit.
---
 fs/squashfs/inode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/squashfs/inode.c b/fs/squashfs/inode.c
index f31649080a88..95a9ff9e2399 100644
--- a/fs/squashfs/inode.c
+++ b/fs/squashfs/inode.c
@@ -48,6 +48,10 @@ static int squashfs_new_inode(struct super_block *sb, struct inode *inode,
 	gid_t i_gid;
 	int err;
 
+	inode->i_ino = le32_to_cpu(sqsh_ino->inode_number);
+	if (inode->i_ino == 0)
+		return -EINVAL;
+
 	err = squashfs_get_id(sb, le16_to_cpu(sqsh_ino->uid), &i_uid);
 	if (err)
 		return err;
@@ -58,7 +62,6 @@ static int squashfs_new_inode(struct super_block *sb, struct inode *inode,
 
 	i_uid_write(inode, i_uid);
 	i_gid_write(inode, i_gid);
-	inode->i_ino = le32_to_cpu(sqsh_ino->inode_number);
 	inode->i_mtime.tv_sec = le32_to_cpu(sqsh_ino->mtime);
 	inode->i_atime.tv_sec = inode->i_mtime.tv_sec;
 	inode->i_ctime.tv_sec = inode->i_mtime.tv_sec;
-- 
2.25.1

Re: [PATCH 6.1] Squashfs: check the inode number is not the invalid value of zero

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

✅ All tests passed successfully. No issues detected.
No action required from the submitter.

The upstream commit SHA1 provided is correct: [ upstream commit 9253c54e01b6505d348afbc02abaa4d9f8a01395 ]

WARNING: Author mismatch between patch and upstream commit:
Backport author: Xiangyu Chen<xiangyu.chen@eng.windriver.com>
Commit author: 

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.1.y        |  Success    |  Success   |

Re: [PATCH 6.1] Squashfs: check the inode number is not the invalid value of zero

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

✅ All tests passed successfully. No issues detected.
No action required from the submitter.

The upstream commit SHA1 provided is correct: 9253c54e01b6505d348afbc02abaa4d9f8a01395

WARNING: Author mismatch between patch and upstream commit:
Backport author: Xiangyu Chen<xiangyu.chen@eng.windriver.com>
Commit author: Phillip Lougher<phillip@squashfs.org.uk>

Status in newer kernel trees:
6.13.y | Present (exact SHA1)
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: be383effaee3)

Note: The patch differs from the upstream commit:
---
1:  9253c54e01b65 ! 1:  fee0f4eb6fa1f Squashfs: check the inode number is not the invalid value of zero
    @@ Metadata
      ## Commit message ##
         Squashfs: check the inode number is not the invalid value of zero
     
    +    [ upstream commit 9253c54e01b6505d348afbc02abaa4d9f8a01395 ]
    +
         Syskiller has produced an out of bounds access in fill_meta_index().
     
         That out of bounds access is ultimately caused because the inode
    @@ Commit message
         Cc: Christian Brauner <brauner@kernel.org>
         Cc: <stable@vger.kernel.org>
         Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    +    Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
    +    Signed-off-by: He Zhe <zhe.he@windriver.com>
     
      ## fs/squashfs/inode.c ##
     @@ fs/squashfs/inode.c: static int squashfs_new_inode(struct super_block *sb, struct inode *inode,
    @@ fs/squashfs/inode.c: static int squashfs_new_inode(struct super_block *sb, struc
      	i_uid_write(inode, i_uid);
      	i_gid_write(inode, i_gid);
     -	inode->i_ino = le32_to_cpu(sqsh_ino->inode_number);
    - 	inode_set_mtime(inode, le32_to_cpu(sqsh_ino->mtime), 0);
    - 	inode_set_atime(inode, inode_get_mtime_sec(inode), 0);
    - 	inode_set_ctime(inode, inode_get_mtime_sec(inode), 0);
    + 	inode->i_mtime.tv_sec = le32_to_cpu(sqsh_ino->mtime);
    + 	inode->i_atime.tv_sec = inode->i_mtime.tv_sec;
    + 	inode->i_ctime.tv_sec = inode->i_mtime.tv_sec;
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.1.y        |  Success    |  Success   |