From: Daniel Vetter <daniel@ffwll.ch>
To: "Christian König" <christian.koenig@amd.com>
Cc: "T.J. Mercier" <tjmercier@google.com>,
Charan Teja Kalla <quic_charante@quicinc.com>,
zhiguojiang <justinjiang@vivo.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,
opensource.kernel@vivo.com
Subject: Re: [PATCH] dmabuf: fix dmabuf file poll uaf issue
Date: Tue, 7 May 2024 15:39:27 +0200 [thread overview]
Message-ID: <ZjovD5WaWjknd-qv@phenom.ffwll.local> (raw)
In-Reply-To: <8915fcc1-d8f1-48c2-9e51-65159aaa5a3b@amd.com>
On Tue, May 07, 2024 at 12:10:07PM +0200, Christian König wrote:
> Am 06.05.24 um 21:04 schrieb T.J. Mercier:
> > On Mon, May 6, 2024 at 2:30 AM Charan Teja Kalla
> > <quic_charante@quicinc.com> wrote:
> > > Hi TJ,
> > >
> > > Seems I have got answers from [1], where it is agreed upon epoll() is
> > > the source of issue.
> > >
> > > Thanks a lot for the discussion.
> > >
> > > [1] https://lore.kernel.org/lkml/0000000000002d631f0615918f1e@google.com/
> > >
> > > Thanks
> > > Charan
> > Oh man, quite a set of threads on this over the weekend. Thanks for the link.
>
> Yeah and it also has some interesting side conclusion: We should probably
> tell people to stop using DMA-buf with epoll.
>
> The background is that the mutex approach epoll uses to make files disappear
> from the interest list on close results in the fact that each file can only
> be part of a single epoll at a time.
>
> Now since DMA-buf is build around the idea that we share the buffer
> representation as file between processes it means that only one process at a
> time can use epoll with each DMA-buf.
>
> So for example if a window manager uses epoll everything is fine. If a
> client is using epoll everything is fine as well. But if *both* use epoll at
> the same time it won't work.
>
> This can lead to rather funny and hard to debug combinations of failures and
> I think we need to document this limitation and explicitly point it out.
Ok, I tested this with a small C program, and you're mixing things up.
Here's what I got
- You cannot add a file twice to the same epoll file/fd. So that part is
correct, and also my understanding from reading the kernel code.
- You can add the same file to two different epoll file instaces. Which
means it's totally fine to use epoll on a dma_buf in different processes
like both in the compositor and in clients.
- Substantially more entertaining, you can nest epoll instances, and e.g.
add a 2nd epoll file as an event to the first one. That way you can add
the same file to both epoll fds, and so end up with the same file
essentially being added twice to the top-level epoll file. So even
within one application there's no real issue when e.g. different
userspace drivers all want to use epoll on the same fd, because you can
just throw in another level of epoll and it's fine again and you won't
get an EEXISTS on EPOLL_CTL_ADD.
But I also don't think we have this issue right now anywhere, since it's
kinda a general epoll issue that happens with any duplicated file.
So I don't think there's any reasons to recommend against using epoll on
dma-buf fd (or sync_file or drm_syncobj or any of the sharing primitives
we have really).
Cheers, Sima
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
next prev parent reply other threads:[~2024-05-07 13:39 UTC|newest]
Thread overview: 25+ 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
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 [this message]
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
2024-05-08 12:21 ` Christian König
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=ZjovD5WaWjknd-qv@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=justinjiang@vivo.com \
--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=quic_charante@quicinc.com \
--cc=sumit.semwal@linaro.org \
--cc=tjmercier@google.com \
/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).