| From: Phillip Lougher <phillip@squashfs.org.uk> |
| Subject: Squashfs: check the inode number is not the invalid value of zero |
| Date: Mon, 8 Apr 2024 23:02:06 +0100 |
| |
| 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> |
| --- |
| |
| fs/squashfs/inode.c | 5 ++++- |
| 1 file changed, 4 insertions(+), 1 deletion(-) |
| |
| --- a/fs/squashfs/inode.c~squashfs-check-the-inode-number-is-not-the-invalid-value-of-zero |
| +++ a/fs/squashfs/inode.c |
| @@ -48,6 +48,10 @@ static int squashfs_new_inode(struct sup |
| 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 sup |
| |
| 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); |
| _ |