On Mon 11-01-21 12:35:41, Zhengyuan Liu wrote:
> From: Zhengyuan Liu <liuzhengyuan@xxxxxxxxxx>
>
> When doing fuzzing test to quota, an error occurred due to the
> mismatch of data type:
>
> Quota error (device loop0): qtree_write_dquot: Error -1244987383 occurred while creating quota
> Unable to handle kernel paging request at virtual address ffffffffb5cb0071
> Mem abort info:
> ESR = 0x96000006
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x00000006
> CM = 0, WnR = 0
> swapper pgtable: 64k pages, 48-bit VAs, pgdp=0000000023980000
> [ffffffffb5cb0071] pgd=00000000243f0003, p4d=00000000243f0003, pud=00000000243f0003, pmd=0000000000000000
> Internal error: Oops: 96000006 [#1] SMP
> Modules linked in: nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nftn
> CPU: 1 PID: 1256 Comm: a.out Not tainted 5.10.0 #31
> Hardware name: XXXX XXXX/Kunpeng Desktop Board D920L11K, BIOS 0.23 07/22/2020
> pstate: 00400009 (nzcv daif +PAN -UAO -TCO BTYPE=--)
> pc : dquot_add_space+0x30/0x258
> lr : __dquot_alloc_space+0x22c/0x358
> sp : ffff80001c10f660
> x29: ffff80001c10f660 x28: 0000000000000001
> x27: 0000000000000000 x26: 0000000000000000
> x25: ffff800011add9c8 x24: ffff0020a2110470
> x23: 0000000000000400 x22: 0000000000000400
> x21: 0000000000000000 x20: 0000000000000400
> x19: ffffffffb5cb0009 x18: 0000000000000000
> x17: 0000000000000020 x16: 0000000000000000
> x15: 0000000000000010 x14: 65727471203a2930
> x13: 706f6f6c20656369 x12: 0000000000000020
> x11: 000000000000000a x10: 0000000000000400
> x9 : ffff8000103afb5c x8 : 0000000800000000
> x7 : 0000000000002000 x6 : 000000000000000f
> x5 : ffffffffb5cb0009 x4 : ffff80001c10f728
> x3 : 0000000000000001 x2 : 0000000000000000
> x1 : 0000000000000400 x0 : ffffffffb5cb0009
> Call trace:
> dquot_add_space+0x30/0x258
> __dquot_alloc_space+0x22c/0x358
> ext4_mb_new_blocks+0x100/0xe88
> ext4_new_meta_blocks+0xb4/0x110
> ext4_xattr_block_set+0x4ec/0xce8
> ext4_xattr_set_handle+0x400/0x528
> ext4_xattr_set+0xc4/0x170
> ext4_xattr_security_set+0x30/0x40
> __vfs_setxattr+0x7c/0xa0
> __vfs_setxattr_noperm+0x88/0x218
> __vfs_setxattr_locked+0xf8/0x120
> vfs_setxattr+0x6c/0x100
> setxattr+0x148/0x240
> path_setxattr+0xc4/0xd8
> __arm64_sys_setxattr+0x2c/0x40
> el0_svc_common.constprop.4+0x94/0x178
> do_el0_svc+0x78/0x98
> el0_svc+0x20/0x30
> el0_sync_handler+0x90/0xb8
> el0_sync+0x158/0x180
>
> In this test case, the return value from get_free_dqblk() could be
> info->dqi_free_blk, which is defined as unsigned int, but we use
> type int in do_insert_tree to check the return value, and therefor we
> may get a negative duo to the transformation. This negative(as aboved
> said -1244987383) then can transmit to dquots in __dquot_initialize(),
> and once we access there can trigger above panic.
>
> __dquot_initialize():
> dquot = dqget(sb, qid);
> if (IS_ERR(dquot)) {
> /* We raced with somebody turning quotas off... */
> if (PTR_ERR(dquot) != -ESRCH) {
> ret = PTR_ERR(dquot);
> goto out_put;
> }
> dquot = NULL;
> }
> got[cnt] = dquot;
>
> Try to fix this problem by making the data type consistent.
>
> Signed-off-by: Zhengyuan Liu <liuzhengyuan@xxxxxxxxxx>
Hum, I think this problem has already been fixed by commits:
10f04d40a9f "quota: Don't overflow quota file offsets"
and
11c514a99bb "quota: Sanity-check quota file headers on load"
I'll try your reproducer...
Honza
> ---
> fs/quota/quota_tree.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c
> index c5562c871c8b..f898a550a3ee 100644
> --- a/fs/quota/quota_tree.c
> +++ b/fs/quota/quota_tree.c
> @@ -81,11 +81,11 @@ static ssize_t write_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf)
> }
>
> /* Remove empty block from list and return it */
> -static int get_free_dqblk(struct qtree_mem_dqinfo *info)
> +static ssize_t get_free_dqblk(struct qtree_mem_dqinfo *info)
> {
> char *buf = getdqbuf(info->dqi_usable_bs);
> struct qt_disk_dqdbheader *dh = (struct qt_disk_dqdbheader *)buf;
> - int ret, blk;
> + ssize_t ret, blk;
>
> if (!buf)
> return -ENOMEM;
> @@ -295,11 +295,12 @@ static uint find_free_dqentry(struct qtree_mem_dqinfo *info,
> }
>
> /* Insert reference to structure into the trie */
> -static int do_insert_tree(struct qtree_mem_dqinfo *info, struct dquot *dquot,
> +static ssize_t do_insert_tree(struct qtree_mem_dqinfo *info, struct dquot *dquot,
> uint *treeblk, int depth)
> {
> char *buf = getdqbuf(info->dqi_usable_bs);
> - int ret = 0, newson = 0, newact = 0;
> + int newson = 0, newact = 0;
> + ssize_t ret = 0;
> __le32 *ref;
> uint newblk;
>
> @@ -335,7 +336,7 @@ static int do_insert_tree(struct qtree_mem_dqinfo *info, struct dquot *dquot,
> goto out_buf;
> }
> #endif
> - newblk = find_free_dqentry(info, dquot, &ret);
> + newblk = find_free_dqentry(info, dquot, (int*)&ret);
> } else {
> ret = do_insert_tree(info, dquot, &newblk, depth+1);
> }
> @@ -352,7 +353,7 @@ static int do_insert_tree(struct qtree_mem_dqinfo *info, struct dquot *dquot,
> }
>
> /* Wrapper for inserting quota structure into tree */
> -static inline int dq_insert_tree(struct qtree_mem_dqinfo *info,
> +static inline ssize_t dq_insert_tree(struct qtree_mem_dqinfo *info,
> struct dquot *dquot)
> {
> int tmp = QT_TREEOFF;
> --
> 2.20.1
>
>
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
