| Message ID | 20170821105705.4822-1-p.zabel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2017-08-21 12:57 GMT+02:00 Philipp Zabel <p.zabel@pengutronix.de>: > If drmPrimeFDToHandle fails in etna_bo_from_dmabuf, the function must > not return with the table_lock mutex held. There is no reason to call > drmPrimeFDToHandle under that lock, so just take the lock after trying > to obtain the handle. > > Based on ceb70a6b1015 ("freedreno: prevent deadlock in error path"). > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com> > --- > etnaviv/etnaviv_bo.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/etnaviv/etnaviv_bo.c b/etnaviv/etnaviv_bo.c > index 75669572..6c07d27f 100644 > --- a/etnaviv/etnaviv_bo.c > +++ b/etnaviv/etnaviv_bo.c > @@ -225,13 +225,13 @@ struct etna_bo *etna_bo_from_dmabuf(struct etna_device *dev, int fd) > int ret, size; > uint32_t handle; > > - pthread_mutex_lock(&table_lock); > - > ret = drmPrimeFDToHandle(dev->fd, fd, &handle); > if (ret) { > return NULL; > } > > + pthread_mutex_lock(&table_lock); > + > bo = lookup_bo(dev->handle_table, handle); > if (bo) > goto out_unlock; > -- > 2.11.0 > > _______________________________________________ > etnaviv mailing list > etnaviv@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/etnaviv
On 21 August 2017 at 11:57, Philipp Zabel <p.zabel@pengutronix.de> wrote: > If drmPrimeFDToHandle fails in etna_bo_from_dmabuf, the function must > not return with the table_lock mutex held. There is no reason to call > drmPrimeFDToHandle under that lock, so just take the lock after trying > to obtain the handle. > > Based on ceb70a6b1015 ("freedreno: prevent deadlock in error path"). > Can you skim through cf40cf05a4d7f3945d534790e7768a048adc3ab0 and it's commit message. Doesn't a similar issue apply here as well? -Emil
On Mon, 2017-08-21 at 12:05 +0100, Emil Velikov wrote: > > On 21 August 2017 at 11:57, Philipp Zabel <p.zabel@pengutronix.de> wrote: > > If drmPrimeFDToHandle fails in etna_bo_from_dmabuf, the function must > > not return with the table_lock mutex held. There is no reason to call > > drmPrimeFDToHandle under that lock, so just take the lock after trying > > to obtain the handle. > > > > Based on ceb70a6b1015 ("freedreno: prevent deadlock in error path"). > > > > Can you skim through cf40cf05a4d7f3945d534790e7768a048adc3ab0 and it's > commit message. > > Doesn't a similar issue apply here as well? I think you are right. If fd_bo_del is called at the same time as fd_bo_from_dmabuf with an fd that maps to the same BO, bo_del may call GEM_CLOSE with the same handle that drmPrimeFDToHandle just returned, possibly invalidating it before fd_bo_from_dmabuf can take the lock. regards Philipp
diff --git a/etnaviv/etnaviv_bo.c b/etnaviv/etnaviv_bo.c index 75669572..6c07d27f 100644 --- a/etnaviv/etnaviv_bo.c +++ b/etnaviv/etnaviv_bo.c @@ -225,13 +225,13 @@ struct etna_bo *etna_bo_from_dmabuf(struct etna_device *dev, int fd) int ret, size; uint32_t handle; - pthread_mutex_lock(&table_lock); - ret = drmPrimeFDToHandle(dev->fd, fd, &handle); if (ret) { return NULL; } + pthread_mutex_lock(&table_lock); + bo = lookup_bo(dev->handle_table, handle); if (bo) goto out_unlock;
If drmPrimeFDToHandle fails in etna_bo_from_dmabuf, the function must not return with the table_lock mutex held. There is no reason to call drmPrimeFDToHandle under that lock, so just take the lock after trying to obtain the handle. Based on ceb70a6b1015 ("freedreno: prevent deadlock in error path"). Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- etnaviv/etnaviv_bo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)