* [PATCH v2 0/2] Fixes for dma-buf locking issues found by Smatch
@ 2022-10-30 15:44 ` Dmitry Osipenko
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-10-30 15:44 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sumit Semwal,
Thomas Zimmermann, Christian König, David Airlie,
Daniel Vetter, noralf, Dan Carpenter
Cc: dri-devel, linux-kernel
Hello,
Here are the two patches fixing minor problems introduced by my
"dma-buf locking convention" series. Thanks to Dan Carpenter who
checked linux-next with Smatch and reported the found issues.
Changelog:
v2: - Added ack from Christian König to the dma-buf patch and improved
the dma-buf/attachment pointer check.
- Dropped v1 patch that added GEM NULL-checking to
drm_gem_vunmap_unlocked() and replaced it with the NULL-checking
in drm_client_buffer_delete(), like was suggested by Christian König.
Dmitry Osipenko (2):
dma-buf: Make locking consistent in dma_buf_detach()
drm/client: Prevent NULL dereference in drm_client_buffer_delete()
drivers/dma-buf/dma-buf.c | 4 ++--
drivers/gpu/drm/drm_client.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
--
2.37.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 0/2] Fixes for dma-buf locking issues found by Smatch
@ 2022-10-30 15:44 ` Dmitry Osipenko
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-10-30 15:44 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sumit Semwal,
Thomas Zimmermann, Christian König, David Airlie,
Daniel Vetter, noralf, Dan Carpenter
Cc: linux-kernel, dri-devel
Hello,
Here are the two patches fixing minor problems introduced by my
"dma-buf locking convention" series. Thanks to Dan Carpenter who
checked linux-next with Smatch and reported the found issues.
Changelog:
v2: - Added ack from Christian König to the dma-buf patch and improved
the dma-buf/attachment pointer check.
- Dropped v1 patch that added GEM NULL-checking to
drm_gem_vunmap_unlocked() and replaced it with the NULL-checking
in drm_client_buffer_delete(), like was suggested by Christian König.
Dmitry Osipenko (2):
dma-buf: Make locking consistent in dma_buf_detach()
drm/client: Prevent NULL dereference in drm_client_buffer_delete()
drivers/dma-buf/dma-buf.c | 4 ++--
drivers/gpu/drm/drm_client.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
--
2.37.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] dma-buf: Make locking consistent in dma_buf_detach()
2022-10-30 15:44 ` Dmitry Osipenko
@ 2022-10-30 15:44 ` Dmitry Osipenko
-1 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-10-30 15:44 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sumit Semwal,
Thomas Zimmermann, Christian König, David Airlie,
Daniel Vetter, noralf, Dan Carpenter
Cc: dri-devel, linux-kernel
The dma_buf_detach() locks attach->dmabuf->resv and then unlocks
dmabuf->resv, which could be a two different locks from a static
code checker perspective. In particular this triggers Smatch to
report the "double unlock" error. Make the locking pointers consistent.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Link: https://lore.kernel.org/dri-devel/Y1fLfsccW3AS%2Fo+%2F@kili/
Fixes: 809d9c72c2f8 ("dma-buf: Move dma_buf_attach() to dynamic locking specification")
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
drivers/dma-buf/dma-buf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index c40d72d318fd..13bfd2d09c56 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -995,10 +995,10 @@ static void __unmap_dma_buf(struct dma_buf_attachment *attach,
*/
void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
{
- if (WARN_ON(!dmabuf || !attach))
+ if (WARN_ON(!dmabuf || !attach || dmabuf != attach->dmabuf))
return;
- dma_resv_lock(attach->dmabuf->resv, NULL);
+ dma_resv_lock(dmabuf->resv, NULL);
if (attach->sgt) {
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] dma-buf: Make locking consistent in dma_buf_detach()
@ 2022-10-30 15:44 ` Dmitry Osipenko
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-10-30 15:44 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sumit Semwal,
Thomas Zimmermann, Christian König, David Airlie,
Daniel Vetter, noralf, Dan Carpenter
Cc: linux-kernel, dri-devel
The dma_buf_detach() locks attach->dmabuf->resv and then unlocks
dmabuf->resv, which could be a two different locks from a static
code checker perspective. In particular this triggers Smatch to
report the "double unlock" error. Make the locking pointers consistent.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Link: https://lore.kernel.org/dri-devel/Y1fLfsccW3AS%2Fo+%2F@kili/
Fixes: 809d9c72c2f8 ("dma-buf: Move dma_buf_attach() to dynamic locking specification")
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
drivers/dma-buf/dma-buf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index c40d72d318fd..13bfd2d09c56 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -995,10 +995,10 @@ static void __unmap_dma_buf(struct dma_buf_attachment *attach,
*/
void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
{
- if (WARN_ON(!dmabuf || !attach))
+ if (WARN_ON(!dmabuf || !attach || dmabuf != attach->dmabuf))
return;
- dma_resv_lock(attach->dmabuf->resv, NULL);
+ dma_resv_lock(dmabuf->resv, NULL);
if (attach->sgt) {
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] drm/client: Prevent NULL dereference in drm_client_buffer_delete()
2022-10-30 15:44 ` Dmitry Osipenko
@ 2022-10-30 15:44 ` Dmitry Osipenko
-1 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-10-30 15:44 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sumit Semwal,
Thomas Zimmermann, Christian König, David Airlie,
Daniel Vetter, noralf, Dan Carpenter
Cc: dri-devel, linux-kernel
The drm_gem_vunmap() will crash with a NULL dereference if the passed
object pointer is NULL. It wasn't a problem before we added the locking
support to drm_gem_vunmap function because the mapping argument was always
NULL together with the object. Make drm_client_buffer_delete() to check
whether GEM is NULL before trying to unmap the GEM, it will happen on
framebuffer creation error.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/dri-devel/Y1kFEGxT8MVlf32V@kili/
Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap operations")
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
drivers/gpu/drm/drm_client.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 38e1be991caa..fd67efe37c63 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -235,10 +235,10 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
{
struct drm_device *dev = buffer->client->dev;
- drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
-
- if (buffer->gem)
+ if (buffer->gem) {
+ drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
drm_gem_object_put(buffer->gem);
+ }
if (buffer->handle)
drm_mode_destroy_dumb(dev, buffer->handle, buffer->client->file);
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] drm/client: Prevent NULL dereference in drm_client_buffer_delete()
@ 2022-10-30 15:44 ` Dmitry Osipenko
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-10-30 15:44 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sumit Semwal,
Thomas Zimmermann, Christian König, David Airlie,
Daniel Vetter, noralf, Dan Carpenter
Cc: linux-kernel, dri-devel
The drm_gem_vunmap() will crash with a NULL dereference if the passed
object pointer is NULL. It wasn't a problem before we added the locking
support to drm_gem_vunmap function because the mapping argument was always
NULL together with the object. Make drm_client_buffer_delete() to check
whether GEM is NULL before trying to unmap the GEM, it will happen on
framebuffer creation error.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/dri-devel/Y1kFEGxT8MVlf32V@kili/
Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap operations")
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
drivers/gpu/drm/drm_client.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 38e1be991caa..fd67efe37c63 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -235,10 +235,10 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
{
struct drm_device *dev = buffer->client->dev;
- drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
-
- if (buffer->gem)
+ if (buffer->gem) {
+ drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
drm_gem_object_put(buffer->gem);
+ }
if (buffer->handle)
drm_mode_destroy_dumb(dev, buffer->handle, buffer->client->file);
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/client: Prevent NULL dereference in drm_client_buffer_delete()
2022-10-30 15:44 ` Dmitry Osipenko
@ 2022-11-02 9:15 ` Christian König
-1 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2022-11-02 9:15 UTC (permalink / raw)
To: Dmitry Osipenko, Maarten Lankhorst, Maxime Ripard, Sumit Semwal,
Thomas Zimmermann, David Airlie, Daniel Vetter, noralf,
Dan Carpenter
Cc: linux-kernel, dri-devel
Am 30.10.22 um 16:44 schrieb Dmitry Osipenko:
> The drm_gem_vunmap() will crash with a NULL dereference if the passed
> object pointer is NULL. It wasn't a problem before we added the locking
> support to drm_gem_vunmap function because the mapping argument was always
> NULL together with the object. Make drm_client_buffer_delete() to check
> whether GEM is NULL before trying to unmap the GEM, it will happen on
> framebuffer creation error.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Link: https://lore.kernel.org/dri-devel/Y1kFEGxT8MVlf32V@kili/
> Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap operations")
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/drm_client.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 38e1be991caa..fd67efe37c63 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -235,10 +235,10 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
> {
> struct drm_device *dev = buffer->client->dev;
>
> - drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
> -
> - if (buffer->gem)
> + if (buffer->gem) {
> + drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
> drm_gem_object_put(buffer->gem);
> + }
>
> if (buffer->handle)
> drm_mode_destroy_dumb(dev, buffer->handle, buffer->client->file);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/client: Prevent NULL dereference in drm_client_buffer_delete()
@ 2022-11-02 9:15 ` Christian König
0 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2022-11-02 9:15 UTC (permalink / raw)
To: Dmitry Osipenko, Maarten Lankhorst, Maxime Ripard, Sumit Semwal,
Thomas Zimmermann, David Airlie, Daniel Vetter, noralf,
Dan Carpenter
Cc: dri-devel, linux-kernel
Am 30.10.22 um 16:44 schrieb Dmitry Osipenko:
> The drm_gem_vunmap() will crash with a NULL dereference if the passed
> object pointer is NULL. It wasn't a problem before we added the locking
> support to drm_gem_vunmap function because the mapping argument was always
> NULL together with the object. Make drm_client_buffer_delete() to check
> whether GEM is NULL before trying to unmap the GEM, it will happen on
> framebuffer creation error.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Link: https://lore.kernel.org/dri-devel/Y1kFEGxT8MVlf32V@kili/
> Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap operations")
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/drm_client.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 38e1be991caa..fd67efe37c63 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -235,10 +235,10 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
> {
> struct drm_device *dev = buffer->client->dev;
>
> - drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
> -
> - if (buffer->gem)
> + if (buffer->gem) {
> + drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
> drm_gem_object_put(buffer->gem);
> + }
>
> if (buffer->handle)
> drm_mode_destroy_dumb(dev, buffer->handle, buffer->client->file);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/client: Prevent NULL dereference in drm_client_buffer_delete()
2022-11-02 9:15 ` Christian König
@ 2022-11-02 11:02 ` Dmitry Osipenko
-1 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-02 11:02 UTC (permalink / raw)
To: Christian König, Maarten Lankhorst, Maxime Ripard,
Sumit Semwal, Thomas Zimmermann, David Airlie, Daniel Vetter,
noralf, Dan Carpenter
Cc: dri-devel, linux-kernel
On 11/2/22 12:15, Christian König wrote:
> Am 30.10.22 um 16:44 schrieb Dmitry Osipenko:
>> The drm_gem_vunmap() will crash with a NULL dereference if the passed
>> object pointer is NULL. It wasn't a problem before we added the locking
>> support to drm_gem_vunmap function because the mapping argument was
>> always
>> NULL together with the object. Make drm_client_buffer_delete() to check
>> whether GEM is NULL before trying to unmap the GEM, it will happen on
>> framebuffer creation error.
>>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Link: https://lore.kernel.org/dri-devel/Y1kFEGxT8MVlf32V@kili/
>> Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap
>> operations")
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
Applied to drm-misc-next, thanks!
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/client: Prevent NULL dereference in drm_client_buffer_delete()
@ 2022-11-02 11:02 ` Dmitry Osipenko
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-02 11:02 UTC (permalink / raw)
To: Christian König, Maarten Lankhorst, Maxime Ripard,
Sumit Semwal, Thomas Zimmermann, David Airlie, Daniel Vetter,
noralf, Dan Carpenter
Cc: linux-kernel, dri-devel
On 11/2/22 12:15, Christian König wrote:
> Am 30.10.22 um 16:44 schrieb Dmitry Osipenko:
>> The drm_gem_vunmap() will crash with a NULL dereference if the passed
>> object pointer is NULL. It wasn't a problem before we added the locking
>> support to drm_gem_vunmap function because the mapping argument was
>> always
>> NULL together with the object. Make drm_client_buffer_delete() to check
>> whether GEM is NULL before trying to unmap the GEM, it will happen on
>> framebuffer creation error.
>>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Link: https://lore.kernel.org/dri-devel/Y1kFEGxT8MVlf32V@kili/
>> Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap
>> operations")
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
Applied to drm-misc-next, thanks!
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-11-02 11:03 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-30 15:44 [PATCH v2 0/2] Fixes for dma-buf locking issues found by Smatch Dmitry Osipenko
2022-10-30 15:44 ` Dmitry Osipenko
2022-10-30 15:44 ` [PATCH v2 1/2] dma-buf: Make locking consistent in dma_buf_detach() Dmitry Osipenko
2022-10-30 15:44 ` Dmitry Osipenko
2022-10-30 15:44 ` [PATCH v2 2/2] drm/client: Prevent NULL dereference in drm_client_buffer_delete() Dmitry Osipenko
2022-10-30 15:44 ` Dmitry Osipenko
2022-11-02 9:15 ` Christian König
2022-11-02 9:15 ` Christian König
2022-11-02 11:02 ` Dmitry Osipenko
2022-11-02 11:02 ` Dmitry Osipenko
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.