Messages in this thread | | | Date | Sat, 11 Mar 2023 13:19:44 +0400 | Subject | Re: [PATCH v2] 9P FS: Fix wild-memory-access write in v9fs_get_acl | From | Ivan Orlov <> |
| |
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: 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 | 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
| |