[PATCH] fs/squashfs: restrict length of xattr_ids in read_xattr_id_table

[PATCH] fs/squashfs: restrict length of xattr_ids in read_xattr_id_table

[Sabyrzhan Tasbolatov]

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: syzbot+2ccea6339d368360800d@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@gmail.com>
---
 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

Re: [PATCH] fs/squashfs: restrict length of xattr_ids in read_xattr_id_table

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3306 bytes --]

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 <lkp@intel.com>

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/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27135 bytes --]

[PATCH v2] fs/squashfs: restrict length of xattr_ids in read_xattr_id_table

[Sabyrzhan Tasbolatov]

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: syzbot+2ccea6339d368360800d@syzkaller.appspotmail.com
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@gmail.com>
---
 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

Re: [PATCH] fs/squashfs: restrict length of xattr_ids in read_xattr_id_table

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3435 bytes --]

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 <lkp@intel.com>

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/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35527 bytes --]