* [PATCH v3] 9P FS: Fix wild-memory-access write in v9fs_get_acl
@ 2023-03-11 12:50 Ivan Orlov
2023-03-13 11:03 ` Christian Schoenebeck
2023-03-27 1:13 ` [V9fs-developer] " Eric Van Hensbergen
0 siblings, 2 replies; 3+ messages in thread
From: Ivan Orlov @ 2023-03-11 12:50 UTC (permalink / raw)
To: ericvh, lucho, asmadeus, linux_oss
Cc: Ivan Orlov, v9fs-developer, linux-kernel, skhan, himadrispandya,
syzbot+cb1d16facb3cc90de5fb
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 simplified the condition.
In previous (v2) 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).
Tested via syzkaller.
Suggested-by: Christian Schoenebeck <linux_oss@crudebyte.com>
Reported-by: syzbot+cb1d16facb3cc90de5fb@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=fbbef66d9e4d096242f3617de5d14d12705b4659
Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
fs/9p/xattr.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
index 50f7f3f6b55e..1974a38bce20 100644
--- a/fs/9p/xattr.c
+++ b/fs/9p/xattr.c
@@ -35,10 +35,12 @@ 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)
retval = -ERANGE;
+ else if (attr_size > SSIZE_MAX)
+ retval = -EOVERFLOW;
+ else /* request to get the attr_size */
+ retval = attr_size;
} else {
iov_iter_truncate(&to, attr_size);
retval = p9_client_read(attr_fid, 0, &to, &err);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] 9P FS: Fix wild-memory-access write in v9fs_get_acl
2023-03-11 12:50 [PATCH v3] 9P FS: Fix wild-memory-access write in v9fs_get_acl Ivan Orlov
@ 2023-03-13 11:03 ` Christian Schoenebeck
2023-03-27 1:13 ` [V9fs-developer] " Eric Van Hensbergen
1 sibling, 0 replies; 3+ messages in thread
From: Christian Schoenebeck @ 2023-03-13 11:03 UTC (permalink / raw)
To: ericvh, lucho, asmadeus, Ivan Orlov
Cc: Ivan Orlov, v9fs-developer, linux-kernel, skhan, himadrispandya,
syzbot+cb1d16facb3cc90de5fb
On Saturday, March 11, 2023 1:50:25 PM 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 simplified the condition.
>
> In previous (v2) 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).
>
> Tested via syzkaller.
>
> Suggested-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> Reported-by: syzbot+cb1d16facb3cc90de5fb@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?id=fbbef66d9e4d096242f3617de5d14d12705b4659
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> ---
> fs/9p/xattr.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
> index 50f7f3f6b55e..1974a38bce20 100644
> --- a/fs/9p/xattr.c
> +++ b/fs/9p/xattr.c
> @@ -35,10 +35,12 @@ 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)
> retval = -ERANGE;
> + else if (attr_size > SSIZE_MAX)
> + retval = -EOVERFLOW;
> + else /* request to get the attr_size */
> + retval = attr_size;
> } else {
> iov_iter_truncate(&to, attr_size);
> retval = p9_client_read(attr_fid, 0, &to, &err);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [V9fs-developer] [PATCH v3] 9P FS: Fix wild-memory-access write in v9fs_get_acl
2023-03-11 12:50 [PATCH v3] 9P FS: Fix wild-memory-access write in v9fs_get_acl Ivan Orlov
2023-03-13 11:03 ` Christian Schoenebeck
@ 2023-03-27 1:13 ` Eric Van Hensbergen
1 sibling, 0 replies; 3+ messages in thread
From: Eric Van Hensbergen @ 2023-03-27 1:13 UTC (permalink / raw)
To: Ivan Orlov
Cc: ericvh, lucho, asmadeus, linux_oss, linux-kernel,
syzbot+cb1d16facb3cc90de5fb, skhan, v9fs-developer,
himadrispandya
Applied, thanks!
[1/1] 9P FS: Fix wild-memory-access write in v9fs_get_acl
commit: 707823e7f22f3864ddc7d85e8e9b614afe4f1b16
Best regards,
--
Eric Van Hensbergen <ericvh@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-03-27 1:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-11 12:50 [PATCH v3] 9P FS: Fix wild-memory-access write in v9fs_get_acl Ivan Orlov
2023-03-13 11:03 ` Christian Schoenebeck
2023-03-27 1:13 ` [V9fs-developer] " Eric Van Hensbergen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).