syzbot found WARNING in squashfs_read_table [1] when length of xattr_ids
exceeds KMALLOC_MAX_SIZE in squashfs_read_table() for kmalloc().
For other squashfs tables, currently such as boundary is checked with
another table's boundaries. Xattr table is the last one, so there is
no defined limit. But to avoid order >= MAX_ORDER warning condition,
we should restrict SQUASHFS_XATTR_BLOCK_BYTES(*xattr_ids) to
KMALLOC_MAX_SIZE, and it gives 1024 pages in squashfs_read_table via
(length + PAGE_SIZE - 1) >> PAGE_SHIFT.
[1]
Call Trace:
alloc_pages_current+0x18c/0x2a0 mm/mempolicy.c:2267
alloc_pages include/linux/gfp.h:547 [inline]
kmalloc_order+0x2e/0xb0 mm/slab_common.c:916
kmalloc_order_trace+0x14/0x120 mm/slab_common.c:932
kmalloc include/linux/slab.h:559 [inline]
squashfs_read_table+0x43/0x1e0 fs/squashfs/cache.c:413
squashfs_read_xattr_id_table+0x191/0x220 fs/squashfs/xattr_id.c:81
Reported-by: [email protected]
Signed-off-by: Sabyrzhan Tasbolatov <[email protected]>
---
fs/squashfs/xattr_id.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/squashfs/xattr_id.c b/fs/squashfs/xattr_id.c
index d99e08464554..6bb51cd3d5c1 100644
--- a/fs/squashfs/xattr_id.c
+++ b/fs/squashfs/xattr_id.c
@@ -78,5 +78,8 @@ __le64 *squashfs_read_xattr_id_table(struct super_block *sb, u64 start,
TRACE("In read_xattr_index_table, length %d\n", len);
+ if (len > KMALLOC_MAX_SIZE)
+ return -ENOMEM;
+
return squashfs_read_table(sb, start + sizeof(*id_table), len);
}
--
2.25.1
Hi Sabyrzhan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linux/master]
[also build test WARNING on linus/master v5.11-rc6 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Sabyrzhan-Tasbolatov/fs-squashfs-restrict-length-of-xattr_ids-in-read_xattr_id_table/20210203-184131
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2ab38c17aac10bf55ab3efde4c4db3893d8691d2
config: s390-randconfig-r031-20210202 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 275c6af7d7f1ed63a03d05b4484413e447133269)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/7e2a858aa7b4de58aa7b037aad474710f07807e8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sabyrzhan-Tasbolatov/fs-squashfs-restrict-length-of-xattr_ids-in-read_xattr_id_table/20210203-184131
git checkout 7e2a858aa7b4de58aa7b037aad474710f07807e8
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> fs/squashfs/xattr_id.c:82:10: warning: incompatible integer to pointer conversion returning 'int' from a function with result type '__le64 *' (aka 'unsigned long long *') [-Wint-conversion]
return -ENOMEM;
^~~~~~~
1 warning generated.
vim +82 fs/squashfs/xattr_id.c
48
49
50 /*
51 * Read uncompressed xattr id lookup table indexes from disk into memory
52 */
53 __le64 *squashfs_read_xattr_id_table(struct super_block *sb, u64 start,
54 u64 *xattr_table_start, int *xattr_ids)
55 {
56 unsigned int len;
57 struct squashfs_xattr_id_table *id_table;
58
59 id_table = squashfs_read_table(sb, start, sizeof(*id_table));
60 if (IS_ERR(id_table))
61 return (__le64 *) id_table;
62
63 *xattr_table_start = le64_to_cpu(id_table->xattr_table_start);
64 *xattr_ids = le32_to_cpu(id_table->xattr_ids);
65 kfree(id_table);
66
67 /* Sanity check values */
68
69 /* there is always at least one xattr id */
70 if (*xattr_ids == 0)
71 return ERR_PTR(-EINVAL);
72
73 /* xattr_table should be less than start */
74 if (*xattr_table_start >= start)
75 return ERR_PTR(-EINVAL);
76
77 len = SQUASHFS_XATTR_BLOCK_BYTES(*xattr_ids);
78
79 TRACE("In read_xattr_index_table, length %d\n", len);
80
81 if (len > KMALLOC_MAX_SIZE)
> 82 return -ENOMEM;
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
In PATCH v2 fixed return -ENOMEM as error pointer.
>syzbot found WARNING in squashfs_read_table [1] when length of xattr_ids
>exceeds KMALLOC_MAX_SIZE in squashfs_read_table() for kmalloc().
>
>For other squashfs tables, currently such as boundary is checked with
>another table's boundaries. Xattr table is the last one, so there is
>no defined limit. But to avoid order >= MAX_ORDER warning condition,
>we should restrict SQUASHFS_XATTR_BLOCK_BYTES(*xattr_ids) to
>KMALLOC_MAX_SIZE, and it gives 1024 pages in squashfs_read_table via
>(length + PAGE_SIZE - 1) >> PAGE_SHIFT.
>
>[1]
>Call Trace:
> alloc_pages_current+0x18c/0x2a0 mm/mempolicy.c:2267
> alloc_pages include/linux/gfp.h:547 [inline]
> kmalloc_order+0x2e/0xb0 mm/slab_common.c:916
> kmalloc_order_trace+0x14/0x120 mm/slab_common.c:932
> kmalloc include/linux/slab.h:559 [inline]
> squashfs_read_table+0x43/0x1e0 fs/squashfs/cache.c:413
> squashfs_read_xattr_id_table+0x191/0x220 fs/squashfs/xattr_id.c:81
Reported-by: [email protected]
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Sabyrzhan Tasbolatov <[email protected]>
---
fs/squashfs/xattr_id.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/squashfs/xattr_id.c b/fs/squashfs/xattr_id.c
index d99e08464554..2462876c66c4 100644
--- a/fs/squashfs/xattr_id.c
+++ b/fs/squashfs/xattr_id.c
@@ -78,5 +78,8 @@ __le64 *squashfs_read_xattr_id_table(struct super_block *sb, u64 start,
TRACE("In read_xattr_index_table, length %d\n", len);
+ if (len > KMALLOC_MAX_SIZE)
+ return ERR_PTR(-ENOMEM);
+
return squashfs_read_table(sb, start + sizeof(*id_table), len);
}
--
2.25.1
Hi Sabyrzhan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linux/master]
[also build test WARNING on linus/master v5.11-rc6 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Sabyrzhan-Tasbolatov/fs-squashfs-restrict-length-of-xattr_ids-in-read_xattr_id_table/20210203-184131
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2ab38c17aac10bf55ab3efde4c4db3893d8691d2
config: i386-randconfig-s001-20210202 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-215-g0fb77bb6-dirty
# https://github.com/0day-ci/linux/commit/7e2a858aa7b4de58aa7b037aad474710f07807e8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sabyrzhan-Tasbolatov/fs-squashfs-restrict-length-of-xattr_ids-in-read_xattr_id_table/20210203-184131
git checkout 7e2a858aa7b4de58aa7b037aad474710f07807e8
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
fs/squashfs/xattr_id.c: In function 'squashfs_read_xattr_id_table':
>> fs/squashfs/xattr_id.c:82:10: warning: returning 'int' from a function with return type '__le64 *' {aka 'long long unsigned int *'} makes pointer from integer without a cast [-Wint-conversion]
82 | return -ENOMEM;
| ^
"sparse warnings: (new ones prefixed by >>)"
>> fs/squashfs/xattr_id.c:82:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __le64 [usertype] * @@ got int @@
fs/squashfs/xattr_id.c:82:24: sparse: expected restricted __le64 [usertype] *
fs/squashfs/xattr_id.c:82:24: sparse: got int
vim +82 fs/squashfs/xattr_id.c
48
49
50 /*
51 * Read uncompressed xattr id lookup table indexes from disk into memory
52 */
53 __le64 *squashfs_read_xattr_id_table(struct super_block *sb, u64 start,
54 u64 *xattr_table_start, int *xattr_ids)
55 {
56 unsigned int len;
57 struct squashfs_xattr_id_table *id_table;
58
59 id_table = squashfs_read_table(sb, start, sizeof(*id_table));
60 if (IS_ERR(id_table))
61 return (__le64 *) id_table;
62
63 *xattr_table_start = le64_to_cpu(id_table->xattr_table_start);
64 *xattr_ids = le32_to_cpu(id_table->xattr_ids);
65 kfree(id_table);
66
67 /* Sanity check values */
68
69 /* there is always at least one xattr id */
70 if (*xattr_ids == 0)
71 return ERR_PTR(-EINVAL);
72
73 /* xattr_table should be less than start */
74 if (*xattr_table_start >= start)
75 return ERR_PTR(-EINVAL);
76
77 len = SQUASHFS_XATTR_BLOCK_BYTES(*xattr_ids);
78
79 TRACE("In read_xattr_index_table, length %d\n", len);
80
81 if (len > KMALLOC_MAX_SIZE)
> 82 return -ENOMEM;
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]