KASAN reported the following issue:
[ 36.825817][ T5923] BUG: KASAN: wild-memory-access in v9fs_get_acl+0x1a4/0x390
[ 36.827479][ T5923] Write of size 4 at addr 9fffeb37f97f1c00 by task syz-executor798/5923
[ 36.829303][ T5923]
[ 36.829846][ T5923] CPU: 0 PID: 5923 Comm: syz-executor798 Not tainted 6.2.0-syzkaller-18302-g596b6b709632 #0
[ 36.832110][ T5923] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/21/2023
[ 36.834464][ T5923] Call trace:
[ 36.835196][ T5923] dump_backtrace+0x1c8/0x1f4
[ 36.836229][ T5923] show_stack+0x2c/0x3c
[ 36.837100][ T5923] dump_stack_lvl+0xd0/0x124
[ 36.838103][ T5923] print_report+0xe4/0x4c0
[ 36.839068][ T5923] kasan_report+0xd4/0x130
[ 36.840052][ T5923] kasan_check_range+0x264/0x2a4
[ 36.841199][ T5923] __kasan_check_write+0x2c/0x3c
[ 36.842216][ T5923] v9fs_get_acl+0x1a4/0x390
[ 36.843232][ T5923] v9fs_mount+0x77c/0xa5c
[ 36.844163][ T5923] legacy_get_tree+0xd4/0x16c
[ 36.845173][ T5923] vfs_get_tree+0x90/0x274
[ 36.846137][ T5923] do_new_mount+0x25c/0x8c8
[ 36.847066][ T5923] path_mount+0x590/0xe58
[ 36.848147][ T5923] __arm64_sys_mount+0x45c/0x594
[ 36.849273][ T5923] invoke_syscall+0x98/0x2c0
[ 36.850421][ T5923] el0_svc_common+0x138/0x258
[ 36.851397][ T5923] do_el0_svc+0x64/0x198
[ 36.852398][ T5923] el0_svc+0x58/0x168
[ 36.853224][ T5923] el0t_64_sync_handler+0x84/0xf0
[ 36.854293][ T5923] el0t_64_sync+0x190/0x194
Calling '__v9fs_get_acl' method in 'v9fs_get_acl' creates the
following chain of function calls:
__v9fs_get_acl
v9fs_fid_get_acl
v9fs_fid_xattr_get
p9_client_xattrwalk
Function p9_client_xattrwalk accepts a pointer to u64-typed
variable attr_size and puts some u64 value into it. However,
after the executing the p9_client_xattrwalk, in some circumstances
we assign the value of u64-typed variable 'attr_size' to the
variable 'retval', which we will return. However, the type of
'retval' is ssize_t, and if the value of attr_size is larger
than SSIZE_MAX, we will face the signed type overflow. If the
overflow occurs, the result of v9fs_fid_xattr_get may be
negative, but not classified as an error. When we try to allocate
an acl with 'broken' size we receive an error, but don't process
it. When we try to free this acl, we face the 'wild-memory-access'
error (because it wasn't allocated).
This patch will add new condition to the 'v9fs_fid_xattr_get'
function, so it will return an EOVERFLOW error if the 'attr_size'
is larger than SSIZE_MAX.
In this version of the patch I removed explicit type conversion and
added separate condition to check the possible overflow and return
an error (in v1 version I've just modified the existing condition).
Reported-by: [email protected]
Link: https://syzkaller.appspot.com/bug?id=fbbef66d9e4d096242f3617de5d14d12705b4659
Signed-off-by: Ivan Orlov <[email protected]>
---
fs/9p/xattr.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
index 50f7f3f6b55e..6affd6b3f5e6 100644
--- a/fs/9p/xattr.c
+++ b/fs/9p/xattr.c
@@ -35,10 +35,14 @@ ssize_t v9fs_fid_xattr_get(struct p9_fid *fid, const char *name,
return retval;
}
if (attr_size > buffer_size) {
- if (!buffer_size) /* request to get the attr_size */
- retval = attr_size;
- else
+ if (!buffer_size) {/* request to get the attr_size */
+ if (attr_size > SSIZE_MAX)
+ retval = -EOVERFLOW;
+ else
+ retval = attr_size;
+ } else {
retval = -ERANGE;
+ }
} else {
iov_iter_truncate(&to, attr_size);
retval = p9_client_read(attr_fid, 0, &to, &err);
--
2.34.1
On 3/11/23 10:34, Ivan Orlov wrote:
> KASAN reported the following issue:
> [ 36.825817][ T5923] BUG: KASAN: wild-memory-access in v9fs_get_acl+0x1a4/0x390
> [ 36.827479][ T5923] Write of size 4 at addr 9fffeb37f97f1c00 by task syz-executor798/5923
> [ 36.829303][ T5923]
> [ 36.829846][ T5923] CPU: 0 PID: 5923 Comm: syz-executor798 Not tainted 6.2.0-syzkaller-18302-g596b6b709632 #0
> [ 36.832110][ T5923] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/21/2023
> [ 36.834464][ T5923] Call trace:
> [ 36.835196][ T5923] dump_backtrace+0x1c8/0x1f4
> [ 36.836229][ T5923] show_stack+0x2c/0x3c
> [ 36.837100][ T5923] dump_stack_lvl+0xd0/0x124
> [ 36.838103][ T5923] print_report+0xe4/0x4c0
> [ 36.839068][ T5923] kasan_report+0xd4/0x130
> [ 36.840052][ T5923] kasan_check_range+0x264/0x2a4
> [ 36.841199][ T5923] __kasan_check_write+0x2c/0x3c
> [ 36.842216][ T5923] v9fs_get_acl+0x1a4/0x390
> [ 36.843232][ T5923] v9fs_mount+0x77c/0xa5c
> [ 36.844163][ T5923] legacy_get_tree+0xd4/0x16c
> [ 36.845173][ T5923] vfs_get_tree+0x90/0x274
> [ 36.846137][ T5923] do_new_mount+0x25c/0x8c8
> [ 36.847066][ T5923] path_mount+0x590/0xe58
> [ 36.848147][ T5923] __arm64_sys_mount+0x45c/0x594
> [ 36.849273][ T5923] invoke_syscall+0x98/0x2c0
> [ 36.850421][ T5923] el0_svc_common+0x138/0x258
> [ 36.851397][ T5923] do_el0_svc+0x64/0x198
> [ 36.852398][ T5923] el0_svc+0x58/0x168
> [ 36.853224][ T5923] el0t_64_sync_handler+0x84/0xf0
> [ 36.854293][ T5923] el0t_64_sync+0x190/0x194
>
> Calling '__v9fs_get_acl' method in 'v9fs_get_acl' creates the
> following chain of function calls:
>
> __v9fs_get_acl
> v9fs_fid_get_acl
> v9fs_fid_xattr_get
> p9_client_xattrwalk
>
> Function p9_client_xattrwalk accepts a pointer to u64-typed
> variable attr_size and puts some u64 value into it. However,
> after the executing the p9_client_xattrwalk, in some circumstances
> we assign the value of u64-typed variable 'attr_size' to the
> variable 'retval', which we will return. However, the type of
> 'retval' is ssize_t, and if the value of attr_size is larger
> than SSIZE_MAX, we will face the signed type overflow. If the
> overflow occurs, the result of v9fs_fid_xattr_get may be
> negative, but not classified as an error. When we try to allocate
> an acl with 'broken' size we receive an error, but don't process
> it. When we try to free this acl, we face the 'wild-memory-access'
> error (because it wasn't allocated).
>
> This patch will add new condition to the 'v9fs_fid_xattr_get'
> function, so it will return an EOVERFLOW error if the 'attr_size'
> is larger than SSIZE_MAX.
>
> In this version of the patch I removed explicit type conversion and
> added separate condition to check the possible overflow and return
> an error (in v1 version I've just modified the existing condition).
>
> Reported-by: [email protected]
> Link: https://syzkaller.appspot.com/bug?id=fbbef66d9e4d096242f3617de5d14d12705b4659
> Signed-off-by: Ivan Orlov <[email protected]>
> ---
> fs/9p/xattr.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
> index 50f7f3f6b55e..6affd6b3f5e6 100644
> --- a/fs/9p/xattr.c
> +++ b/fs/9p/xattr.c
> @@ -35,10 +35,14 @@ ssize_t v9fs_fid_xattr_get(struct p9_fid *fid, const char *name,
> return retval;
> }
> if (attr_size > buffer_size) {
> - if (!buffer_size) /* request to get the attr_size */
> - retval = attr_size;
> - else
> + if (!buffer_size) {/* request to get the attr_size */
> + if (attr_size > SSIZE_MAX)
> + retval = -EOVERFLOW;
> + else
> + retval = attr_size;
> + } else {
> retval = -ERANGE;
> + }
> } else {
> iov_iter_truncate(&to, attr_size);
> retval = p9_client_read(attr_fid, 0, &to, &err);
#syz test
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
KASAN: wild-memory-access Write in v9fs_get_acl
loop0: detected capacity change from 0 to 256
MINIX-fs: mounting unchecked file system, running fsck is recommended
==================================================================
BUG: KASAN: wild-memory-access in instrument_atomic_read_write include/linux/instrumented.h:102 [inline]
BUG: KASAN: wild-memory-access in atomic_fetch_sub_release include/linux/atomic/atomic-instrumented.h:176 [inline]
BUG: KASAN: wild-memory-access in __refcount_sub_and_test include/linux/refcount.h:272 [inline]
BUG: KASAN: wild-memory-access in __refcount_dec_and_test include/linux/refcount.h:315 [inline]
BUG: KASAN: wild-memory-access in refcount_dec_and_test include/linux/refcount.h:333 [inline]
BUG: KASAN: wild-memory-access in posix_acl_release include/linux/posix_acl.h:57 [inline]
BUG: KASAN: wild-memory-access in v9fs_get_acl+0x1a4/0x390 fs/9p/acl.c:102
Write of size 4 at addr 9fffeb37f97f1c00 by task syz-executor.0/6490
CPU: 0 PID: 6490 Comm: syz-executor.0 Not tainted 6.3.0-rc1-syzkaller-00230-gef5f68cc1f82 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/02/2023
Call trace:
dump_backtrace+0x1c8/0x1f4 arch/arm64/kernel/stacktrace.c:158
show_stack+0x2c/0x3c arch/arm64/kernel/stacktrace.c:165
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xd0/0x124 lib/dump_stack.c:106
print_report+0xe4/0x514 mm/kasan/report.c:433
kasan_report+0xd4/0x130 mm/kasan/report.c:536
kasan_check_range+0x264/0x2a4 mm/kasan/generic.c:187
__kasan_check_write+0x2c/0x3c mm/kasan/shadow.c:37
instrument_atomic_read_write include/linux/instrumented.h:102 [inline]
atomic_fetch_sub_release include/linux/atomic/atomic-instrumented.h:176 [inline]
__refcount_sub_and_test include/linux/refcount.h:272 [inline]
__refcount_dec_and_test include/linux/refcount.h:315 [inline]
refcount_dec_and_test include/linux/refcount.h:333 [inline]
posix_acl_release include/linux/posix_acl.h:57 [inline]
v9fs_get_acl+0x1a4/0x390 fs/9p/acl.c:102
v9fs_mount+0x77c/0xa5c fs/9p/vfs_super.c:183
legacy_get_tree+0xd4/0x16c fs/fs_context.c:610
vfs_get_tree+0x90/0x274 fs/super.c:1501
do_new_mount+0x25c/0x8c8 fs/namespace.c:3042
path_mount+0x590/0xe20 fs/namespace.c:3372
do_mount fs/namespace.c:3385 [inline]
__do_sys_mount fs/namespace.c:3594 [inline]
__se_sys_mount fs/namespace.c:3571 [inline]
__arm64_sys_mount+0x45c/0x594 fs/namespace.c:3571
__invoke_syscall arch/arm64/kernel/syscall.c:38 [inline]
invoke_syscall+0x98/0x2c0 arch/arm64/kernel/syscall.c:52
el0_svc_common+0x138/0x258 arch/arm64/kernel/syscall.c:142
do_el0_svc+0x64/0x198 arch/arm64/kernel/syscall.c:193
el0_svc+0x58/0x168 arch/arm64/kernel/entry-common.c:637
el0t_64_sync_handler+0x84/0xf0 arch/arm64/kernel/entry-common.c:655
el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591
==================================================================
Unable to handle kernel paging request at virtual address 9fffeb37f97f1c00
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004
CM = 0, WnR = 0
[9fffeb37f97f1c00] address between user and kernel address ranges
Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 6490 Comm: syz-executor.0 Tainted: G B 6.3.0-rc1-syzkaller-00230-gef5f68cc1f82 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/02/2023
pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __lse_atomic_fetch_add_release arch/arm64/include/asm/atomic_lse.h:62 [inline]
pc : __lse_atomic_fetch_sub_release arch/arm64/include/asm/atomic_lse.h:76 [inline]
pc : arch_atomic_fetch_sub_release arch/arm64/include/asm/atomic.h:51 [inline]
pc : atomic_fetch_sub_release include/linux/atomic/atomic-instrumented.h:177 [inline]
pc : __refcount_sub_and_test include/linux/refcount.h:272 [inline]
pc : __refcount_dec_and_test include/linux/refcount.h:315 [inline]
pc : refcount_dec_and_test include/linux/refcount.h:333 [inline]
pc : posix_acl_release include/linux/posix_acl.h:57 [inline]
pc : v9fs_get_acl+0x1b0/0x390 fs/9p/acl.c:102
lr : arch_atomic_fetch_sub_release arch/arm64/include/asm/atomic.h:51 [inline]
lr : atomic_fetch_sub_release include/linux/atomic/atomic-instrumented.h:177 [inline]
lr : __refcount_sub_and_test include/linux/refcount.h:272 [inline]
lr : __refcount_dec_and_test include/linux/refcount.h:315 [inline]
lr : refcount_dec_and_test include/linux/refcount.h:333 [inline]
lr : posix_acl_release include/linux/posix_acl.h:57 [inline]
lr : v9fs_get_acl+0x1ac/0x390 fs/9p/acl.c:102
sp : ffff80001e837970
x29: ffff80001e837970 x28: dfff800000000000 x27: 1ffff00003d06f3c
x26: 1ffff00003d06f38 x25: ffff0000dd9041e0 x24: ffff0000dd904178
x23: ffff0000e5068000 x22: dfff800000000000 x21: 9fffeb37f97f1c00
x20: 00000000fffffffb x19: fffffffffffffffb x18: 1fffe000368951b6
x17: ffff800015cdd000 x16: ffff80001245e54c x15: 0000000000000000
x14: 0000000040000000 x13: 0000000000000002 x12: 0000000000000001
x11: ff80800009d819b8 x10: 0000000000000000 x9 : ffff800009d819b8
x8 : 00000000ffffffff x7 : 1fffe000368951b7 x6 : ffff80000828dc14
x5 : 0000000000000000 x4 : 0000000000000001 x3 : ffff8000081bc3c4
x2 : 0000000000000001 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
arch_atomic_fetch_sub_release arch/arm64/include/asm/atomic.h:51 [inline]
atomic_fetch_sub_release include/linux/atomic/atomic-instrumented.h:177 [inline]
__refcount_sub_and_test include/linux/refcount.h:272 [inline]
__refcount_dec_and_test include/linux/refcount.h:315 [inline]
refcount_dec_and_test include/linux/refcount.h:333 [inline]
posix_acl_release include/linux/posix_acl.h:57 [inline]
v9fs_get_acl+0x1b0/0x390 fs/9p/acl.c:102
v9fs_mount+0x77c/0xa5c fs/9p/vfs_super.c:183
legacy_get_tree+0xd4/0x16c fs/fs_context.c:610
vfs_get_tree+0x90/0x274 fs/super.c:1501
do_new_mount+0x25c/0x8c8 fs/namespace.c:3042
path_mount+0x590/0xe20 fs/namespace.c:3372
do_mount fs/namespace.c:3385 [inline]
__do_sys_mount fs/namespace.c:3594 [inline]
__se_sys_mount fs/namespace.c:3571 [inline]
__arm64_sys_mount+0x45c/0x594 fs/namespace.c:3571
__invoke_syscall arch/arm64/kernel/syscall.c:38 [inline]
invoke_syscall+0x98/0x2c0 arch/arm64/kernel/syscall.c:52
el0_svc_common+0x138/0x258 arch/arm64/kernel/syscall.c:142
do_el0_svc+0x64/0x198 arch/arm64/kernel/syscall.c:193
el0_svc+0x58/0x168 arch/arm64/kernel/entry-common.c:637
el0t_64_sync_handler+0x84/0xf0 arch/arm64/kernel/entry-common.c:655
el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591
Code: 97b021c6 d503201f 979e3dbf 12800008 (b86802b6)
---[ end trace 0000000000000000 ]---
----------------
Code disassembly (best guess):
0: 97b021c6 bl 0xfffffffffec08718
4: d503201f nop
8: 979e3dbf bl 0xfffffffffe78f704
c: 12800008 mov w8, #0xffffffff // #-1
* 10: b86802b6 ldaddl w8, w22, [x21] <-- trapping instruction
Tested on:
commit: ef5f68cc Merge tag 'scsi-fixes' of git://git.kernel.or..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
console output: https://syzkaller.appspot.com/x/log.txt?x=115a390ac80000
kernel config: https://syzkaller.appspot.com/x/.config?x=fd3c6e109ff25818
dashboard link: https://syzkaller.appspot.com/bug?extid=cb1d16facb3cc90de5fb
compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
userspace arch: arm64
Note: no patches were applied.
On 3/11/23 10:34, Ivan Orlov wrote:
> KASAN reported the following issue:
> [ 36.825817][ T5923] BUG: KASAN: wild-memory-access in v9fs_get_acl+0x1a4/0x390
> [ 36.827479][ T5923] Write of size 4 at addr 9fffeb37f97f1c00 by task syz-executor798/5923
> [ 36.829303][ T5923]
> [ 36.829846][ T5923] CPU: 0 PID: 5923 Comm: syz-executor798 Not tainted 6.2.0-syzkaller-18302-g596b6b709632 #0
> [ 36.832110][ T5923] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/21/2023
> [ 36.834464][ T5923] Call trace:
> [ 36.835196][ T5923] dump_backtrace+0x1c8/0x1f4
> [ 36.836229][ T5923] show_stack+0x2c/0x3c
> [ 36.837100][ T5923] dump_stack_lvl+0xd0/0x124
> [ 36.838103][ T5923] print_report+0xe4/0x4c0
> [ 36.839068][ T5923] kasan_report+0xd4/0x130
> [ 36.840052][ T5923] kasan_check_range+0x264/0x2a4
> [ 36.841199][ T5923] __kasan_check_write+0x2c/0x3c
> [ 36.842216][ T5923] v9fs_get_acl+0x1a4/0x390
> [ 36.843232][ T5923] v9fs_mount+0x77c/0xa5c
> [ 36.844163][ T5923] legacy_get_tree+0xd4/0x16c
> [ 36.845173][ T5923] vfs_get_tree+0x90/0x274
> [ 36.846137][ T5923] do_new_mount+0x25c/0x8c8
> [ 36.847066][ T5923] path_mount+0x590/0xe58
> [ 36.848147][ T5923] __arm64_sys_mount+0x45c/0x594
> [ 36.849273][ T5923] invoke_syscall+0x98/0x2c0
> [ 36.850421][ T5923] el0_svc_common+0x138/0x258
> [ 36.851397][ T5923] do_el0_svc+0x64/0x198
> [ 36.852398][ T5923] el0_svc+0x58/0x168
> [ 36.853224][ T5923] el0t_64_sync_handler+0x84/0xf0
> [ 36.854293][ T5923] el0t_64_sync+0x190/0x194
>
> Calling '__v9fs_get_acl' method in 'v9fs_get_acl' creates the
> following chain of function calls:
>
> __v9fs_get_acl
> v9fs_fid_get_acl
> v9fs_fid_xattr_get
> p9_client_xattrwalk
>
> Function p9_client_xattrwalk accepts a pointer to u64-typed
> variable attr_size and puts some u64 value into it. However,
> after the executing the p9_client_xattrwalk, in some circumstances
> we assign the value of u64-typed variable 'attr_size' to the
> variable 'retval', which we will return. However, the type of
> 'retval' is ssize_t, and if the value of attr_size is larger
> than SSIZE_MAX, we will face the signed type overflow. If the
> overflow occurs, the result of v9fs_fid_xattr_get may be
> negative, but not classified as an error. When we try to allocate
> an acl with 'broken' size we receive an error, but don't process
> it. When we try to free this acl, we face the 'wild-memory-access'
> error (because it wasn't allocated).
>
> This patch will add new condition to the 'v9fs_fid_xattr_get'
> function, so it will return an EOVERFLOW error if the 'attr_size'
> is larger than SSIZE_MAX.
>
> In this version of the patch I removed explicit type conversion and
> added separate condition to check the possible overflow and return
> an error (in v1 version I've just modified the existing condition).
>
> Reported-by: [email protected]
> Link: https://syzkaller.appspot.com/bug?id=fbbef66d9e4d096242f3617de5d14d12705b4659
> Signed-off-by: Ivan Orlov <[email protected]>
> ---
> fs/9p/xattr.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
> index 50f7f3f6b55e..6affd6b3f5e6 100644
> --- a/fs/9p/xattr.c
> +++ b/fs/9p/xattr.c
> @@ -35,10 +35,14 @@ ssize_t v9fs_fid_xattr_get(struct p9_fid *fid, const char *name,
> return retval;
> }
> if (attr_size > buffer_size) {
> - if (!buffer_size) /* request to get the attr_size */
> - retval = attr_size;
> - else
> + if (!buffer_size) {/* request to get the attr_size */
> + if (attr_size > SSIZE_MAX)
> + retval = -EOVERFLOW;
> + else
> + retval = attr_size;
> + } else {
> retval = -ERANGE;
> + }
> } else {
> iov_iter_truncate(&to, attr_size);
> retval = p9_client_read(attr_fid, 0, &to, &err);
Tested via syzkaller, this patch fixes the bug (see patch testing requests):
https://syzkaller.appspot.com/bug?id=fbbef66d9e4d096242f3617de5d14d12705b4659
On Saturday, March 11, 2023 7:34:11 AM CET Ivan Orlov wrote:
> KASAN reported the following issue:
> [ 36.825817][ T5923] BUG: KASAN: wild-memory-access in v9fs_get_acl+0x1a4/0x390
> [ 36.827479][ T5923] Write of size 4 at addr 9fffeb37f97f1c00 by task syz-executor798/5923
> [ 36.829303][ T5923]
> [ 36.829846][ T5923] CPU: 0 PID: 5923 Comm: syz-executor798 Not tainted 6.2.0-syzkaller-18302-g596b6b709632 #0
> [ 36.832110][ T5923] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/21/2023
> [ 36.834464][ T5923] Call trace:
> [ 36.835196][ T5923] dump_backtrace+0x1c8/0x1f4
> [ 36.836229][ T5923] show_stack+0x2c/0x3c
> [ 36.837100][ T5923] dump_stack_lvl+0xd0/0x124
> [ 36.838103][ T5923] print_report+0xe4/0x4c0
> [ 36.839068][ T5923] kasan_report+0xd4/0x130
> [ 36.840052][ T5923] kasan_check_range+0x264/0x2a4
> [ 36.841199][ T5923] __kasan_check_write+0x2c/0x3c
> [ 36.842216][ T5923] v9fs_get_acl+0x1a4/0x390
> [ 36.843232][ T5923] v9fs_mount+0x77c/0xa5c
> [ 36.844163][ T5923] legacy_get_tree+0xd4/0x16c
> [ 36.845173][ T5923] vfs_get_tree+0x90/0x274
> [ 36.846137][ T5923] do_new_mount+0x25c/0x8c8
> [ 36.847066][ T5923] path_mount+0x590/0xe58
> [ 36.848147][ T5923] __arm64_sys_mount+0x45c/0x594
> [ 36.849273][ T5923] invoke_syscall+0x98/0x2c0
> [ 36.850421][ T5923] el0_svc_common+0x138/0x258
> [ 36.851397][ T5923] do_el0_svc+0x64/0x198
> [ 36.852398][ T5923] el0_svc+0x58/0x168
> [ 36.853224][ T5923] el0t_64_sync_handler+0x84/0xf0
> [ 36.854293][ T5923] el0t_64_sync+0x190/0x194
>
> Calling '__v9fs_get_acl' method in 'v9fs_get_acl' creates the
> following chain of function calls:
>
> __v9fs_get_acl
> v9fs_fid_get_acl
> v9fs_fid_xattr_get
> p9_client_xattrwalk
>
> Function p9_client_xattrwalk accepts a pointer to u64-typed
> variable attr_size and puts some u64 value into it. However,
> after the executing the p9_client_xattrwalk, in some circumstances
> we assign the value of u64-typed variable 'attr_size' to the
> variable 'retval', which we will return. However, the type of
> 'retval' is ssize_t, and if the value of attr_size is larger
> than SSIZE_MAX, we will face the signed type overflow. If the
> overflow occurs, the result of v9fs_fid_xattr_get may be
> negative, but not classified as an error. When we try to allocate
> an acl with 'broken' size we receive an error, but don't process
> it. When we try to free this acl, we face the 'wild-memory-access'
> error (because it wasn't allocated).
>
> This patch will add new condition to the 'v9fs_fid_xattr_get'
> function, so it will return an EOVERFLOW error if the 'attr_size'
> is larger than SSIZE_MAX.
>
> In this version of the patch I removed explicit type conversion and
> added separate condition to check the possible overflow and return
> an error (in v1 version I've just modified the existing condition).
>
> Reported-by: [email protected]
> Link: https://syzkaller.appspot.com/bug?id=fbbef66d9e4d096242f3617de5d14d12705b4659
> Signed-off-by: Ivan Orlov <[email protected]>
> ---
> fs/9p/xattr.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
> index 50f7f3f6b55e..6affd6b3f5e6 100644
> --- a/fs/9p/xattr.c
> +++ b/fs/9p/xattr.c
> @@ -35,10 +35,14 @@ ssize_t v9fs_fid_xattr_get(struct p9_fid *fid, const char *name,
> return retval;
> }
> if (attr_size > buffer_size) {
> - if (!buffer_size) /* request to get the attr_size */
> - retval = attr_size;
> - else
> + if (!buffer_size) {/* request to get the attr_size */
> + if (attr_size > SSIZE_MAX)
> + retval = -EOVERFLOW;
> + else
> + retval = attr_size;
> + } else {
> retval = -ERANGE;
> + }
I would have written it in different style:
if (buffer_size)
retval = -ERANGE;
else if (attr_size > SSIZE_MAX)
retval = -EOVERFLOW;
else
retval = attr_size; /* request to get the attr_size */
But the behaviour change itself makes sense, so:
Reviewed-by: Christian Schoenebeck <[email protected]>
> } else {
> iov_iter_truncate(&to, attr_size);
> retval = p9_client_read(attr_fid, 0, &to, &err);
>
On 3/11/23 15:17, Christian Schoenebeck wrote:
> I would have written it in different style:
>
> if (buffer_size)
> retval = -ERANGE;
> else if (attr_size > SSIZE_MAX)
> retval = -EOVERFLOW;
> else
> retval = attr_size; /* request to get the attr_size */
>
> But the behaviour change itself makes sense, so:
>
> Reviewed-by: Christian Schoenebeck <[email protected]>
>
>> } else {
>> iov_iter_truncate(&to, attr_size);
>> retval = p9_client_read(attr_fid, 0, &to, &err);
>>
>
>
You are right, the condition can be simplified, thank you! I will
rewrite it, send as v3 and mention you in 'suggested-by'.