* [PATCH] fuse: fix use after free issue in fuse_dev_do_read()
@ 2017-02-08 15:00 Sahitya Tummala
2017-02-23 8:46 ` Miklos Szeredi
0 siblings, 1 reply; 2+ messages in thread
From: Sahitya Tummala @ 2017-02-08 15:00 UTC (permalink / raw)
To: Miklos Szeredi, linux-fsdevel, linux-kernel; +Cc: Sahitya Tummala
There is a potential race between fuse_dev_do_write()
and request_wait_answer() contexts as shown below:
TASK 1:
__fuse_request_send():
|--spin_lock(&fiq->waitq.lock);
|--queue_request();
|--spin_unlock(&fiq->waitq.lock);
|--request_wait_answer():
|--if (test_bit(FR_SENT, &req->flags))
<gets pre-empted after it is validated true> TASK 2:
fuse_dev_do_write():
|--clears bit FR_SENT,
|--request_end():
|--sets bit FR_FINISHED
|--spin_lock(&fiq->waitq.lock);
|--list_del_init(&req->intr_entry);
|--spin_unlock(&fiq->waitq.lock);
|--fuse_put_request();
|--queue_interrupt();
<request gets queued to interrupts list>
|--wake_up_locked(&fiq->waitq);
|--wait_event_freezable();
<as FR_FINISHED is set, it returns and then
the caller frees this request>
Now, the next fuse_dev_do_read(), see interrupts list is not empty
and then calls fuse_read_interrupt() which tries to access the request
which is already free'd and gets the below crash:
[11432.401266] Unable to handle kernel paging request at virtual address
6b6b6b6b6b6b6b6b
...
[11432.418518] Kernel BUG at ffffff80083720e0
[11432.456168] PC is at __list_del_entry+0x6c/0xc4
[11432.463573] LR is at fuse_dev_do_read+0x1ac/0x474
...
[11432.679999] [<ffffff80083720e0>] __list_del_entry+0x6c/0xc4
[11432.687794] [<ffffff80082c65e0>] fuse_dev_do_read+0x1ac/0x474
[11432.693180] [<ffffff80082c6b14>] fuse_dev_read+0x6c/0x78
[11432.699082] [<ffffff80081d5638>] __vfs_read+0xc0/0xe8
[11432.704459] [<ffffff80081d5efc>] vfs_read+0x90/0x108
[11432.709406] [<ffffff80081d67f0>] SyS_read+0x58/0x94
As FR_FINISHED bit is set before deleting the intr_entry with input
queue lock in request completion path, do the testing of this flag and
queueing atomically with the same lock in queue_interrupt().
Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
---
fs/fuse/dev.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 4e06a27..b656e18 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -399,6 +399,10 @@ static void request_end(struct fuse_conn *fc, struct fuse_req *req)
static void queue_interrupt(struct fuse_iqueue *fiq, struct fuse_req *req)
{
spin_lock(&fiq->waitq.lock);
+ if (test_bit(FR_FINISHED, &req->flags)) {
+ spin_unlock(&fiq->waitq.lock);
+ return;
+ }
if (list_empty(&req->intr_entry)) {
list_add_tail(&req->intr_entry, &fiq->interrupts);
wake_up_locked(&fiq->waitq);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] fuse: fix use after free issue in fuse_dev_do_read()
2017-02-08 15:00 [PATCH] fuse: fix use after free issue in fuse_dev_do_read() Sahitya Tummala
@ 2017-02-23 8:46 ` Miklos Szeredi
0 siblings, 0 replies; 2+ messages in thread
From: Miklos Szeredi @ 2017-02-23 8:46 UTC (permalink / raw)
To: Sahitya Tummala; +Cc: linux-fsdevel, linux-kernel
On Wed, Feb 8, 2017 at 4:00 PM, Sahitya Tummala <stummala@codeaurora.org> wrote:
> There is a potential race between fuse_dev_do_write()
> and request_wait_answer() contexts as shown below:
>
> TASK 1:
> __fuse_request_send():
> |--spin_lock(&fiq->waitq.lock);
> |--queue_request();
> |--spin_unlock(&fiq->waitq.lock);
> |--request_wait_answer():
> |--if (test_bit(FR_SENT, &req->flags))
> <gets pre-empted after it is validated true> TASK 2:
> fuse_dev_do_write():
> |--clears bit FR_SENT,
> |--request_end():
> |--sets bit FR_FINISHED
> |--spin_lock(&fiq->waitq.lock);
> |--list_del_init(&req->intr_entry);
> |--spin_unlock(&fiq->waitq.lock);
> |--fuse_put_request();
> |--queue_interrupt();
> <request gets queued to interrupts list>
> |--wake_up_locked(&fiq->waitq);
> |--wait_event_freezable();
> <as FR_FINISHED is set, it returns and then
> the caller frees this request>
>
> Now, the next fuse_dev_do_read(), see interrupts list is not empty
> and then calls fuse_read_interrupt() which tries to access the request
> which is already free'd and gets the below crash:
>
> [11432.401266] Unable to handle kernel paging request at virtual address
> 6b6b6b6b6b6b6b6b
> ...
> [11432.418518] Kernel BUG at ffffff80083720e0
> [11432.456168] PC is at __list_del_entry+0x6c/0xc4
> [11432.463573] LR is at fuse_dev_do_read+0x1ac/0x474
> ...
> [11432.679999] [<ffffff80083720e0>] __list_del_entry+0x6c/0xc4
> [11432.687794] [<ffffff80082c65e0>] fuse_dev_do_read+0x1ac/0x474
> [11432.693180] [<ffffff80082c6b14>] fuse_dev_read+0x6c/0x78
> [11432.699082] [<ffffff80081d5638>] __vfs_read+0xc0/0xe8
> [11432.704459] [<ffffff80081d5efc>] vfs_read+0x90/0x108
> [11432.709406] [<ffffff80081d67f0>] SyS_read+0x58/0x94
>
> As FR_FINISHED bit is set before deleting the intr_entry with input
> queue lock in request completion path, do the testing of this flag and
> queueing atomically with the same lock in queue_interrupt().
Thanks, fix added to 4.10.
Miklos
>
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> ---
> fs/fuse/dev.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 4e06a27..b656e18 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -399,6 +399,10 @@ static void request_end(struct fuse_conn *fc, struct fuse_req *req)
> static void queue_interrupt(struct fuse_iqueue *fiq, struct fuse_req *req)
> {
> spin_lock(&fiq->waitq.lock);
> + if (test_bit(FR_FINISHED, &req->flags)) {
> + spin_unlock(&fiq->waitq.lock);
> + return;
> + }
> if (list_empty(&req->intr_entry)) {
> list_add_tail(&req->intr_entry, &fiq->interrupts);
> wake_up_locked(&fiq->waitq);
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-02-23 8:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-08 15:00 [PATCH] fuse: fix use after free issue in fuse_dev_do_read() Sahitya Tummala
2017-02-23 8:46 ` Miklos Szeredi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.