From: "Andrew F. Davis" <afd@ti.com> To: Liam Mark <lmark@codeaurora.org>, <labbott@redhat.com>, <sumit.semwal@linaro.org> Cc: <arve@android.com>, <tkjos@android.com>, <maco@android.com>, <joel@joelfernandes.org>, <christian@brauner.io>, <devel@driverdev.osuosl.org>, <dri-devel@lists.freedesktop.org>, <linaro-mm-sig@lists.linaro.org>, <linux-kernel@vger.kernel.org>, <john.stultz@linaro.org> Subject: Re: [PATCH 1/4] staging: android: ion: Support cpu access during dma_buf_detach Date: Fri, 18 Jan 2019 13:34:16 -0600 [thread overview] Message-ID: <b006960e-131b-dc63-fadb-7d9e696b33fb@ti.com> (raw) In-Reply-To: <1547836667-13695-2-git-send-email-lmark@codeaurora.org> On 1/18/19 12:37 PM, Liam Mark wrote: > Often userspace doesn't know when the kernel will be calling dma_buf_detach > on the buffer. > If userpace starts its CPU access at the same time as the sg list is being > freed it could end up accessing the sg list after it has been freed. > > Thread A Thread B > - DMA_BUF_IOCTL_SYNC IOCT > - ion_dma_buf_begin_cpu_access > - list_for_each_entry > - ion_dma_buf_detatch > - free_duped_table > - dma_sync_sg_for_cpu > The window for this seems really small, but it does seem technically possible, good find. for what it's worth: Acked-by: Andrew F. Davis <afd@ti.com> > Fix this by getting the ion_buffer lock before freeing the sg table memory. > > Fixes: 2a55e7b5e544 ("staging: android: ion: Call dma_map_sg for syncing and mapping") > Signed-off-by: Liam Mark <lmark@codeaurora.org> > --- > drivers/staging/android/ion/ion.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index a0802de8c3a1..6f5afab7c1a1 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -248,10 +248,10 @@ static void ion_dma_buf_detatch(struct dma_buf *dmabuf, > struct ion_dma_buf_attachment *a = attachment->priv; > struct ion_buffer *buffer = dmabuf->priv; > > - free_duped_table(a->table); > mutex_lock(&buffer->lock); > list_del(&a->list); > mutex_unlock(&buffer->lock); > + free_duped_table(a->table); > > kfree(a); > } >
WARNING: multiple messages have this Message-ID (diff)
From: "Andrew F. Davis" <afd@ti.com> To: Liam Mark <lmark@codeaurora.org>, labbott@redhat.com, sumit.semwal@linaro.org Cc: devel@driverdev.osuosl.org, tkjos@android.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, arve@android.com, joel@joelfernandes.org, maco@android.com, christian@brauner.io Subject: Re: [PATCH 1/4] staging: android: ion: Support cpu access during dma_buf_detach Date: Fri, 18 Jan 2019 13:34:16 -0600 [thread overview] Message-ID: <b006960e-131b-dc63-fadb-7d9e696b33fb@ti.com> (raw) In-Reply-To: <1547836667-13695-2-git-send-email-lmark@codeaurora.org> On 1/18/19 12:37 PM, Liam Mark wrote: > Often userspace doesn't know when the kernel will be calling dma_buf_detach > on the buffer. > If userpace starts its CPU access at the same time as the sg list is being > freed it could end up accessing the sg list after it has been freed. > > Thread A Thread B > - DMA_BUF_IOCTL_SYNC IOCT > - ion_dma_buf_begin_cpu_access > - list_for_each_entry > - ion_dma_buf_detatch > - free_duped_table > - dma_sync_sg_for_cpu > The window for this seems really small, but it does seem technically possible, good find. for what it's worth: Acked-by: Andrew F. Davis <afd@ti.com> > Fix this by getting the ion_buffer lock before freeing the sg table memory. > > Fixes: 2a55e7b5e544 ("staging: android: ion: Call dma_map_sg for syncing and mapping") > Signed-off-by: Liam Mark <lmark@codeaurora.org> > --- > drivers/staging/android/ion/ion.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index a0802de8c3a1..6f5afab7c1a1 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -248,10 +248,10 @@ static void ion_dma_buf_detatch(struct dma_buf *dmabuf, > struct ion_dma_buf_attachment *a = attachment->priv; > struct ion_buffer *buffer = dmabuf->priv; > > - free_duped_table(a->table); > mutex_lock(&buffer->lock); > list_del(&a->list); > mutex_unlock(&buffer->lock); > + free_duped_table(a->table); > > kfree(a); > } > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-01-18 19:34 UTC|newest] Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-18 18:37 [PATCH 0/4] ION stability and perf changes Liam Mark 2019-01-18 18:37 ` [PATCH 1/4] staging: android: ion: Support cpu access during dma_buf_detach Liam Mark 2019-01-18 18:37 ` Liam Mark 2019-01-18 19:34 ` Andrew F. Davis [this message] 2019-01-18 19:34 ` Andrew F. Davis 2019-01-18 20:40 ` Laura Abbott 2019-01-18 18:37 ` [PATCH 2/4] staging: android: ion: Restrict cache maintenance to dma mapped memory Liam Mark 2019-01-18 18:37 ` Liam Mark 2019-01-18 20:20 ` Andrew F. Davis 2019-01-18 20:20 ` Andrew F. Davis 2019-01-18 21:18 ` Liam Mark 2019-01-18 21:18 ` Liam Mark 2019-01-29 23:44 ` Liam Mark 2019-01-29 23:44 ` Liam Mark 2019-01-30 11:31 ` Brian Starkey 2019-01-30 11:31 ` Brian Starkey 2019-02-06 15:40 ` [Linaro-mm-sig] " Ørjan Eide 2019-02-07 7:31 ` Christoph Hellwig 2019-02-07 7:31 ` Christoph Hellwig 2019-02-07 15:45 ` Ørjan Eide 2019-02-28 23:49 ` Liam Mark 2019-01-30 14:31 ` Andrew F. Davis 2019-01-30 14:31 ` Andrew F. Davis 2019-01-18 18:37 ` [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes Liam Mark 2019-01-18 18:37 ` Liam Mark 2019-01-18 20:48 ` Laura Abbott 2019-01-18 21:32 ` Liam Mark 2019-01-18 22:45 ` Laura Abbott 2019-01-18 22:45 ` Laura Abbott 2019-01-19 10:25 ` Christoph Hellwig 2019-01-19 10:25 ` Christoph Hellwig 2019-01-19 16:50 ` Laura Abbott 2019-01-21 8:30 ` Christoph Hellwig 2019-01-21 19:44 ` Liam Mark 2019-01-21 19:49 ` Andrew F. Davis 2019-01-21 19:49 ` Andrew F. Davis 2019-01-21 20:20 ` Liam Mark 2019-01-21 20:24 ` Andrew F. Davis 2019-01-21 20:24 ` Andrew F. Davis 2019-01-21 22:18 ` Liam Mark 2019-01-22 15:42 ` Andrew F. Davis 2019-01-22 15:42 ` Andrew F. Davis 2019-01-22 22:47 ` Liam Mark 2019-01-22 22:47 ` Liam Mark 2019-01-21 21:30 ` Christoph Hellwig 2019-01-21 22:14 ` Liam Mark 2019-01-21 22:14 ` Liam Mark 2019-01-21 21:29 ` Christoph Hellwig 2019-01-21 21:29 ` Christoph Hellwig 2019-01-21 22:12 ` Liam Mark 2019-01-21 22:12 ` Liam Mark 2019-01-22 16:06 ` Andrew F. Davis 2019-01-22 16:06 ` Andrew F. Davis 2019-01-22 22:50 ` Liam Mark 2019-01-18 18:37 ` [PATCH 4/4] staging: android: ion: Support " Liam Mark 2019-01-18 18:37 ` Liam Mark 2019-01-21 12:19 ` Brian Starkey 2019-01-21 12:19 ` Brian Starkey 2019-01-22 22:37 ` Liam Mark
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=b006960e-131b-dc63-fadb-7d9e696b33fb@ti.com \ --to=afd@ti.com \ --cc=arve@android.com \ --cc=christian@brauner.io \ --cc=devel@driverdev.osuosl.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=joel@joelfernandes.org \ --cc=john.stultz@linaro.org \ --cc=labbott@redhat.com \ --cc=linaro-mm-sig@lists.linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lmark@codeaurora.org \ --cc=maco@android.com \ --cc=sumit.semwal@linaro.org \ --cc=tkjos@android.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.