From: zhiguojiang <justinjiang@vivo.com>
To: "Christian König" <christian.koenig@amd.com>,
"Sumit Semwal" <sumit.semwal@linaro.org>,
linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org
Cc: opensource.kernel@vivo.com
Subject: Re: [PATCH] dmabuf: fix dmabuf file poll uaf issue
Date: Tue, 2 Apr 2024 14:49:07 +0800 [thread overview]
Message-ID: <cc7defae-60c1-4cc8-aee5-475d4460e574@vivo.com> (raw)
In-Reply-To: <5cf29162-a29d-4af7-b68e-aac5c862d20e@amd.com>
> As far as I can see that's not because of the DMA-buf code, but
> because you are somehow using this interface incorrectly.
>
> When dma_buf_poll() is called it is mandatory for the caller to hold a
> reference to the file descriptor on which the poll operation is executed.
>
> So adding code like "if (!file_count(file))" in the beginning of
> dma_buf_poll() is certainly broken.
>
> My best guess is that you have some unbalanced
> dma_buf_get()/dma_buf_put() somewhere instead.
>
>
Hi Christian,
The kernel dma_buf_poll() code shound not cause system crashes due to
the user mode usage logical issues ?
Thanks
在 2024/4/1 20:22, Christian König 写道:
> Am 27.03.24 um 03:29 schrieb Zhiguo Jiang:
>> The issue is a UAF issue of dmabuf file fd. Throght debugging, we found
>> that the dmabuf file fd is added to the epoll event listener list, and
>> when it is released, it is not removed from the epoll list, which leads
>> to the UAF(Use-After-Free) issue.
>
> As far as I can see that's not because of the DMA-buf code, but
> because you are somehow using this interface incorrectly.
>
> When dma_buf_poll() is called it is mandatory for the caller to hold a
> reference to the file descriptor on which the poll operation is executed.
>
> So adding code like "if (!file_count(file))" in the beginning of
> dma_buf_poll() is certainly broken.
>
> My best guess is that you have some unbalanced
> dma_buf_get()/dma_buf_put() somewhere instead.
>
> Regards,
> Christian.
>
>>
>> The UAF issue can be solved by checking dmabuf file->f_count value and
>> skipping the poll operation for the closed dmabuf file in the
>> dma_buf_poll(). We have tested this solved patch multiple times and
>> have not reproduced the uaf issue.
>>
>> crash dump:
>> list_del corruption, ffffff8a6f143a90->next is LIST_POISON1
>> (dead000000000100)
>> ------------[ cut here ]------------
>> kernel BUG at lib/list_debug.c:55!
>> Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
>> pc : __list_del_entry_valid+0x98/0xd4
>> lr : __list_del_entry_valid+0x98/0xd4
>> sp : ffffffc01d413d00
>> x29: ffffffc01d413d00 x28: 00000000000000c0 x27: 0000000000000020
>> x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000080007
>> x23: ffffff8b22e5dcc0 x22: ffffff88a6be12d0 x21: ffffff8b22e572b0
>> x20: ffffff80254ed0a0 x19: ffffff8a6f143a00 x18: ffffffda5efed3c0
>> x17: 6165642820314e4f x16: 53494f505f545349 x15: 4c20736920747865
>> x14: 6e3e2d3039613334 x13: 2930303130303030 x12: 0000000000000018
>> x11: ffffff8b6c188000 x10: 00000000ffffffff x9 : 6c8413a194897b00
>> x8 : 6c8413a194897b00 x7 : 74707572726f6320 x6 : 6c65645f7473696c
>> x5 : ffffff8b6c3b2a3e x4 : ffffff8b6c3b2a40 x3 : ffff103000001005
>> x2 : 0000000000000001 x1 : 00000000000000c0 x0 : 000000000000004e
>> Call trace:
>> __list_del_entry_valid+0x98/0xd4
>> dma_buf_file_release+0x48/0x90
>> __fput+0xf4/0x280
>> ____fput+0x10/0x20
>> task_work_run+0xcc/0xf4
>> do_notify_resume+0x2a0/0x33c
>> el0_svc+0x5c/0xa4
>> el0t_64_sync_handler+0x68/0xb4
>> el0t_64_sync+0x1a0/0x1a4
>> Code: d0006fe0 912c5000 f2fbd5a2 94231a01 (d4210000)
>> ---[ end trace 0000000000000000 ]---
>> Kernel panic - not syncing: Oops - BUG: Fatal exception
>> SMP: stopping secondary CPUs
>>
>> Signed-off-by: Zhiguo Jiang <justinjiang@vivo.com>
>> ---
>> drivers/dma-buf/dma-buf.c | 28 ++++++++++++++++++++++++----
>> 1 file changed, 24 insertions(+), 4 deletions(-)
>> mode change 100644 => 100755 drivers/dma-buf/dma-buf.c
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 8fe5aa67b167..e469dd9288cc
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -240,6 +240,10 @@ static __poll_t dma_buf_poll(struct file *file,
>> poll_table *poll)
>> struct dma_resv *resv;
>> __poll_t events;
>> + /* Check if the file exists */
>> + if (!file_count(file))
>> + return EPOLLERR;
>> +
>> dmabuf = file->private_data;
>> if (!dmabuf || !dmabuf->resv)
>> return EPOLLERR;
>> @@ -266,8 +270,15 @@ static __poll_t dma_buf_poll(struct file *file,
>> poll_table *poll)
>> spin_unlock_irq(&dmabuf->poll.lock);
>> if (events & EPOLLOUT) {
>> - /* Paired with fput in dma_buf_poll_cb */
>> - get_file(dmabuf->file);
>> + /*
>> + * Paired with fput in dma_buf_poll_cb,
>> + * Skip poll for the closed file.
>> + */
>> + if (!get_file_rcu(&dmabuf->file)) {
>> + events &= ~EPOLLOUT;
>> + dcb->active = 0;
>> + goto clear_out_event;
>> + }
>> if (!dma_buf_poll_add_cb(resv, true, dcb))
>> /* No callback queued, wake up any other waiters */
>> @@ -277,6 +288,7 @@ static __poll_t dma_buf_poll(struct file *file,
>> poll_table *poll)
>> }
>> }
>> +clear_out_event:
>> if (events & EPOLLIN) {
>> struct dma_buf_poll_cb_t *dcb = &dmabuf->cb_in;
>> @@ -289,8 +301,15 @@ static __poll_t dma_buf_poll(struct file
>> *file, poll_table *poll)
>> spin_unlock_irq(&dmabuf->poll.lock);
>> if (events & EPOLLIN) {
>> - /* Paired with fput in dma_buf_poll_cb */
>> - get_file(dmabuf->file);
>> + /*
>> + * Paired with fput in dma_buf_poll_cb,
>> + * Skip poll for the closed file.
>> + */
>> + if (!get_file_rcu(&dmabuf->file)) {
>> + events &= ~EPOLLIN;
>> + dcb->active = 0;
>> + goto clear_in_event;
>> + }
>> if (!dma_buf_poll_add_cb(resv, false, dcb))
>> /* No callback queued, wake up any other waiters */
>> @@ -300,6 +319,7 @@ static __poll_t dma_buf_poll(struct file *file,
>> poll_table *poll)
>> }
>> }
>> +clear_in_event:
>> dma_resv_unlock(resv);
>> return events;
>> }
>
next prev parent reply other threads:[~2024-04-02 6:49 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-27 2:29 [PATCH] dmabuf: fix dmabuf file poll uaf issue Zhiguo Jiang
2024-03-29 23:36 ` T.J. Mercier
2024-04-01 6:52 ` zhiguojiang
2024-04-01 12:22 ` Christian König
2024-04-02 6:49 ` zhiguojiang [this message]
2024-04-02 8:07 ` Christian König
2024-04-02 18:22 ` T.J. Mercier
2024-04-12 6:19 ` zhiguojiang
2024-04-12 6:39 ` Christian König
2024-04-15 10:35 ` zhiguojiang
2024-04-15 11:57 ` Christian König
2024-04-18 1:33 ` zhiguojiang
2024-04-18 6:46 ` Christian König
2024-05-03 13:40 ` Charan Teja Kalla
2024-05-03 23:13 ` T.J. Mercier
2024-05-05 16:20 ` Charan Teja Kalla
2024-05-06 9:30 ` Charan Teja Kalla
2024-05-06 19:04 ` T.J. Mercier
2024-05-07 10:10 ` Christian König
2024-05-07 13:39 ` Daniel Vetter
2024-05-07 14:04 ` Christian König
2024-05-07 18:00 ` T.J. Mercier
2024-05-07 20:19 ` Rob Clark
2024-05-08 11:51 ` David Laight
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=cc7defae-60c1-4cc8-aee5-475d4460e574@vivo.com \
--to=justinjiang@vivo.com \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=opensource.kernel@vivo.com \
--cc=sumit.semwal@linaro.org \
/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).