2021-03-03 00:37:27

by Phillip Lougher

Subject: [PATCH] Squashfs: fix xattr id and id lookup sanity checks

The checks for maximum metadata block size is
missing SQUASHFS_BLOCK_OFFSET (the two byte length
count).

Cc: [email protected]
Signed-off-by: Phillip Lougher <[email protected]>
---
fs/squashfs/id.c | 6 ++++--
fs/squashfs/xattr_id.c | 6 ++++--
2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/squashfs/id.c b/fs/squashfs/id.c
index 11581bf31af4..ea5387679723 100644
--- a/fs/squashfs/id.c
+++ b/fs/squashfs/id.c
@@ -97,14 +97,16 @@ __le64 *squashfs_read_id_index_table(struct super_block *sb,
start = le64_to_cpu(table[n]);
end = le64_to_cpu(table[n + 1]);

- if (start >= end || (end - start) > SQUASHFS_METADATA_SIZE) {
+ if (start >= end || (end - start) >
+ (SQUASHFS_METADATA_SIZE + SQUASHFS_BLOCK_OFFSET)) {
kfree(table);
return ERR_PTR(-EINVAL);
}
}

start = le64_to_cpu(table[indexes - 1]);
- if (start >= id_table_start || (id_table_start - start) > SQUASHFS_METADATA_SIZE) {
+ if (start >= id_table_start || (id_table_start - start) >
+ (SQUASHFS_METADATA_SIZE + SQUASHFS_BLOCK_OFFSET)) {
kfree(table);
return ERR_PTR(-EINVAL);
}
diff --git a/fs/squashfs/xattr_id.c b/fs/squashfs/xattr_id.c
index ead66670b41a..087cab8c78f4 100644
--- a/fs/squashfs/xattr_id.c
+++ b/fs/squashfs/xattr_id.c
@@ -109,14 +109,16 @@ __le64 *squashfs_read_xattr_id_table(struct super_block *sb, u64 table_start,
start = le64_to_cpu(table[n]);
end = le64_to_cpu(table[n + 1]);

- if (start >= end || (end - start) > SQUASHFS_METADATA_SIZE) {
+ if (start >= end || (end - start) >
+ (SQUASHFS_METADATA_SIZE + SQUASHFS_BLOCK_OFFSET)) {
kfree(table);
return ERR_PTR(-EINVAL);
}
}

start = le64_to_cpu(table[indexes - 1]);
- if (start >= table_start || (table_start - start) > SQUASHFS_METADATA_SIZE) {
+ if (start >= table_start || (table_start - start) >
+ (SQUASHFS_METADATA_SIZE + SQUASHFS_BLOCK_OFFSET)) {
kfree(table);
return ERR_PTR(-EINVAL);
}
--
2.29.2

2021-03-04 07:52:14

by Andrew Morton

Subject: Re: [PATCH] Squashfs: fix xattr id and id lookup sanity checks

On Mon, 1 Mar 2021 07:27:57 +0000 (GMT) Phillip Lougher <[email protected]> wrote:

> The checks for maximum metadata block size is
> missing SQUASHFS_BLOCK_OFFSET (the two byte length
> count).

What are the user visible consequences of this bug?

> Cc: [email protected]
> Signed-off-by: Phillip Lougher <[email protected]>

Fixes: f37aa4c7366e23f ("squashfs: add more sanity checks in id lookup")

yes?

2021-03-04 08:09:05

by Phillip Lougher

Subject: Re: [PATCH] Squashfs: fix xattr id and id lookup sanity checks

> On 03/03/2021 00:34 Andrew Morton <[email protected]> wrote:
>
>
> On Mon, 1 Mar 2021 07:27:57 +0000 (GMT) Phillip Lougher <[email protected]> wrote:
>
> > The checks for maximum metadata block size is
> > missing SQUASHFS_BLOCK_OFFSET (the two byte length
> > count).
>
> What are the user visible consequences of this bug?

The user will be unable to mount the filesystem, because it will
fail the sanity check.


>
> > Cc: [email protected]
> > Signed-off-by: Phillip Lougher <[email protected]>
>
> Fixes: f37aa4c7366e23f ("squashfs: add more sanity checks in id lookup")
>
> yes?

Yes.

Phillip

2021-03-09 05:39:02

by Andrew Morton

Subject: Re: [PATCH] Squashfs: fix xattr id and id lookup sanity checks

On Mon, 1 Mar 2021 07:27:57 +0000 (GMT) Phillip Lougher <[email protected]> wrote:

> The checks for maximum metadata block size is
> missing SQUASHFS_BLOCK_OFFSET (the two byte length
> count).

There is no definition of SQUASHFS_BLOCK_OFFSET. Makes compiler unhappy.

2021-03-09 14:47:35

by Sean Nyekjaer

Subject: Re: [PATCH] Squashfs: fix xattr id and id lookup sanity checks

Hi Andrew

This patch depends on my patch ([PATCH] squashfs: fix inode lookup sanity checks)
which add's the SQUASHFS_BLOCK_OFFSET :)

/Sean