Both the REG and LREG inode readers take the fragment offset verbatim
off disk:

	frag = le32_to_cpu(sqsh_ino->fragment);
	if (frag != SQUASHFS_INVALID_FRAG) {
		/*
		 * the file cannot have a fragment (tailend) and have a
		 * file size a multiple of the block size
		 */
		if ((inode->i_size & (msblk->block_size - 1)) == 0) { ... }
		frag_offset = le32_to_cpu(sqsh_ino->offset);
		...
	}
	...
	squashfs_i(inode)->fragment_offset = frag_offset;

The check added by commit 9ee94bfbe930 ("Squashfs: add additional inode
sanity checking") is about i_size, not about offset, and nothing else
looks at offset at all.  It reaches squashfs_copy_data() unchanged, for
instance from squashfs_readahead_fragment():

	bytes = squashfs_copy_data(addr, buffer, offset +
			squashfs_i(inode)->fragment_offset, avail);

where the parameter is a signed int:

	while (offset < entry->length) {
		void *buff = entry->data[offset / PAGE_SIZE]
				+ (offset % PAGE_SIZE);

An on-disk offset of 0x80000000 arrives as -2147483648, the loop guard
passes, and entry->data[-524288] is loaded from about 4 MiB below the
cache entry's page pointer array.

The tailend of the file has to lie inside the uncompressed fragment
block, and that block is at most msblk->block_size bytes, so check that
when the inode is read -- the offset is on-disk metadata and belongs
validated where it is parsed, not where it eventually faults.

Mounting a crafted image requires CAP_SYS_ADMIN in the initial user
namespace -- SquashFS is not FS_USERNS_MOUNT, so an unprivileged user in
a user namespace cannot mount one either.  The realistic threat model is
untrusted images: automounted removable media and udisks2, .snap and
AppImage style container images, mount -o loop from a setuid helper, and
any service that mounts a squashfs it did not build.  Note the trigger
is unprivileged and a read-only mount is enough: a single read() of the
crafted file by any user who can open it.

  # mksquashfs -noI -noD -noF -noX -b 4096 with one 1-byte file, then
  # rewrite that inode's fragment offset to 0x80000000
  mount -t squashfs -o ro /dev/loop0 /mnt
  cat /mnt/file

  BUG: KASAN: wild-memory-access in squashfs_copy_data+0xb8/0x158
  Read of size 8 at addr 007f0000c7b1a0a0 by task cat/132
   __asan_load8+0x84/0xd0
   squashfs_copy_data+0xb8/0x158
   squashfs_readahead+0x9b8/0xe60
   read_pages+0x134/0x550
   page_cache_ra_unbounded+0x264/0x540
   filemap_get_pages+0x1d0/0xaa0
   filemap_splice_read+0x248/0x548
   do_sendfile+0x540/0x618
   __arm64_sys_sendfile64+0x1f8/0x220
   el0t_64_sync+0x198/0x1a0
  Unable to handle kernel paging request at virtual address 007f0000c7b1a0a0
  ...
  Kernel panic - not syncing: Oops: Fatal exception

The KASAN report is immediately followed by a fatal page fault at the
same address, so this is not a sanitizer-only artefact: the same load
oopses on a kernel built without KASAN.  (The trace goes through
sendfile(2) because busybox cat uses it; the plain read(2) path reaches
the same squashfs_copy_data() through squashfs_readpage_fragment().)

mksquashfs packs tailends inside a single fragment block, so every image
it produces satisfies the new check.

Fixes: 6545b246a2c8 ("Squashfs: inode operations")
Cc: stable@vger.kernel.org
Signed-off-by: Yuejie Shi <syjcnss@gmail.com>
---
 fs/squashfs/inode.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/squashfs/inode.c b/fs/squashfs/inode.c
index 82b687414e65..4872b28f1b77 100644
--- a/fs/squashfs/inode.c
+++ b/fs/squashfs/inode.c
@@ -156,6 +156,16 @@ int squashfs_read_inode(struct inode *inode, long long
 				goto failed_read;
 			}
 			frag_offset = le32_to_cpu(sqsh_ino->offset);
+			/*
+			 * the tailend has to lie within the uncompressed
+			 * fragment block, which is at most block_size bytes
+			 */
+			if (frag_offset +
+			    (inode->i_size & (msblk->block_size - 1)) >
+					msblk->block_size) {
+				err = -EINVAL;
+				goto failed_read;
+			}
 			frag_size = squashfs_frag_lookup(sb, frag, &frag_blk);
 			if (frag_size < 0) {
 				err = frag_size;
@@ -212,6 +222,16 @@ int squashfs_read_inode(struct inode *inode, long long
 				goto failed_read;
 			}
 			frag_offset = le32_to_cpu(sqsh_ino->offset);
+			/*
+			 * the tailend has to lie within the uncompressed
+			 * fragment block, which is at most block_size bytes
+			 */
+			if (frag_offset +
+			    (inode->i_size & (msblk->block_size - 1)) >
+					msblk->block_size) {
+				err = -EINVAL;
+				goto failed_read;
+			}
 			frag_size = squashfs_frag_lookup(sb, frag, &frag_blk);
 			if (frag_size < 0) {
 				err = frag_size;
--
2.51.0

On 03/08/2026 04:27, Yuejie Shi wrote:
> Both the REG and LREG inode readers take the fragment offset verbatim
> off disk:
> 
> 	frag = le32_to_cpu(sqsh_ino->fragment);
> 	if (frag != SQUASHFS_INVALID_FRAG) {
> 		/*
> 		 * the file cannot have a fragment (tailend) and have a
> 		 * file size a multiple of the block size
> 		 */
> 		if ((inode->i_size & (msblk->block_size - 1)) == 0) { ... }
> 		frag_offset = le32_to_cpu(sqsh_ino->offset);
> 		...
> 	}
> 	...
> 	squashfs_i(inode)->fragment_offset = frag_offset;
> 
> The check added by commit 9ee94bfbe930 ("Squashfs: add additional inode
> sanity checking") is about i_size, not about offset, and nothing else
> looks at offset at all.  It reaches squashfs_copy_data() unchanged, for
> instance from squashfs_readahead_fragment():
> 
> 	bytes = squashfs_copy_data(addr, buffer, offset +
> 			squashfs_i(inode)->fragment_offset, avail);
> 
> where the parameter is a signed int:
> 
> 	while (offset < entry->length) {
> 		void *buff = entry->data[offset / PAGE_SIZE]
> 				+ (offset % PAGE_SIZE);
> 

NACK.

You have found a bug, but your fix is completely wrong.

Also curiously this got raised as an issue with Squashfs-tools recently
https://github.com/plougher/squashfs-tools/issues/398

and I fixed it in a commit last week
https://github.com/plougher/squashfs-tools/commit/88c47b58df408e8106814079e6979a454269018c

Whether AI is involved or not, this is part of the growing trend to
have multiple reports of the same or related issues by different people,
within days or weeks of each other.  Which is making it more and more
difficult and time consuming to deal with them fairly and quickly.

Comments below

> ---
>   fs/squashfs/inode.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/fs/squashfs/inode.c b/fs/squashfs/inode.c
> index 82b687414e65..4872b28f1b77 100644
> --- a/fs/squashfs/inode.c
> +++ b/fs/squashfs/inode.c
> @@ -156,6 +156,16 @@ int squashfs_read_inode(struct inode *inode, long long
>   				goto failed_read;
>   			}
>   			frag_offset = le32_to_cpu(sqsh_ino->offset);
> +			/*
> +			 * the tailend has to lie within the uncompressed
> +			 * fragment block, which is at most block_size bytes
> +			 */
> +			if (frag_offset +
> +			    (inode->i_size & (msblk->block_size - 1)) >
> +					msblk->block_size) {
> +				err = -EINVAL;
> +				goto failed_read;
> +			}

This is remarkably like the fix I put into Squashfs-tools last week.  Except I
put it in the file write path, when the fragment is about to be written out to
disk.

Why? Because the fragment is only accessed when you read data, it isn't necessary
to create the inode.  Putting the check in the inode instantiation means you
won't be able to use Unsquashfs to do a file listing (unsquashfs -l example.sqsh).

Exactly the same here.  You are prematurely rejecting something before you get to
where the problem is.  Doing it here means you can't stat the file, and if the
tailend is say the last 100 bytes of a 3 Megabyte file, you now can't read any of
it, rather than just the last 100 bytes.

It is also using a hammer to crush a nut, complete over-kill. Cases where
frag_offset (or the combination of frag_offset and the tailend size) go beyond
the end of the buffer are rejected when the tailend is copied.  The only thing which
isn't at that point is a negative offset, and that can be checked then.

Plus this "fix" doesn't even do what it pretends to do either, which is to
prevent accesses beyond the end of the fragment block.  A fragment block may
be a maximum of a block size, but, it is almost always smaller, and can be anything
from 1 byte in size to the block size.

So if the fragment block decompresses to 5 bytes, the fact the frag_offset has
been bounded  to block size is completely useless.  It is the underlying code
which prevents access beyond the actual size of the fragment block when the
tailend is copied.

So NACK.

Phillip
   >   			frag_size = squashfs_frag_lookup(sb, frag, &frag_blk);
>   			if (frag_size < 0) {
>   				err = frag_size;
> @@ -212,6 +222,16 @@ int squashfs_read_inode(struct inode *inode, long long
>   				goto failed_read;
>   			}
>   			frag_offset = le32_to_cpu(sqsh_ino->offset);
> +			/*
> +			 * the tailend has to lie within the uncompressed
> +			 * fragment block, which is at most block_size bytes
> +			 */
> +			if (frag_offset +
> +			    (inode->i_size & (msblk->block_size - 1)) >
> +					msblk->block_size) {
> +				err = -EINVAL;
> +				goto failed_read;
> +			}
>   			frag_size = squashfs_frag_lookup(sb, frag, &frag_blk);
>   			if (frag_size < 0) {
>   				err = frag_size;
> --
> 2.51.0