This causes kernel crash when testing lima driver.
Cc: Christian König christian.koenig@amd.com Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") Signed-off-by: Qiang Yu yuq825@gmail.com --- drivers/dma-buf/dma-resv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 42a8f3f11681..709002515550 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, if (pfence_excl) *pfence_excl = fence_excl; else if (fence_excl) - shared[++shared_count] = fence_excl; + shared[shared_count++] = fence_excl;
if (!shared_count) { kfree(shared);
Quoting Qiang Yu (2019-09-22 08:49:00)
This causes kernel crash when testing lima driver.
Cc: Christian König christian.koenig@amd.com Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") Signed-off-by: Qiang Yu yuq825@gmail.com
drivers/dma-buf/dma-resv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 42a8f3f11681..709002515550 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, if (pfence_excl) *pfence_excl = fence_excl; else if (fence_excl)
shared[++shared_count] = fence_excl;
shared[shared_count++] = fence_excl;
Oops.
Reviewed-by: Chris Wilson chris@chris-wilson.co.uk -Chris
Quoting Chris Wilson (2019-09-22 13:17:19)
Quoting Qiang Yu (2019-09-22 08:49:00)
This causes kernel crash when testing lima driver.
Cc: Christian König christian.koenig@amd.com Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") Signed-off-by: Qiang Yu yuq825@gmail.com
drivers/dma-buf/dma-resv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 42a8f3f11681..709002515550 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, if (pfence_excl) *pfence_excl = fence_excl; else if (fence_excl)
shared[++shared_count] = fence_excl;
shared[shared_count++] = fence_excl;Oops.
Reviewed-by: Chris Wilson chris@chris-wilson.co.uk
Applied, thanks for the fix. -Chris
Hi Chris,
This fix has been pushed to drm-misc-next for a while. But Linux 5.4-rc kernels still does not have this fix. Should it be also pushed to drm-misc-fixes?
Thanks, Qiang
On Sun, Sep 22, 2019 at 8:50 PM Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Chris Wilson (2019-09-22 13:17:19)
Quoting Qiang Yu (2019-09-22 08:49:00)
This causes kernel crash when testing lima driver.
Cc: Christian König christian.koenig@amd.com Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") Signed-off-by: Qiang Yu yuq825@gmail.com
drivers/dma-buf/dma-resv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 42a8f3f11681..709002515550 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, if (pfence_excl) *pfence_excl = fence_excl; else if (fence_excl)
shared[++shared_count] = fence_excl;
shared[shared_count++] = fence_excl;Oops.
Reviewed-by: Chris Wilson chris@chris-wilson.co.uk
Applied, thanks for the fix. -Chris
Hi Qiang,
oh, good point. Yes it certainly should.
Looks like I accidentally pushed it to the wrong branch.
Thanks, Christian.
Am 10.10.19 um 16:27 schrieb Qiang Yu:
Hi Chris,
This fix has been pushed to drm-misc-next for a while. But Linux 5.4-rc kernels still does not have this fix. Should it be also pushed to drm-misc-fixes?
Thanks, Qiang
On Sun, Sep 22, 2019 at 8:50 PM Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Chris Wilson (2019-09-22 13:17:19)
Quoting Qiang Yu (2019-09-22 08:49:00)
This causes kernel crash when testing lima driver.
Cc: Christian König christian.koenig@amd.com Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") Signed-off-by: Qiang Yu yuq825@gmail.com
drivers/dma-buf/dma-resv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 42a8f3f11681..709002515550 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, if (pfence_excl) *pfence_excl = fence_excl; else if (fence_excl)
shared[++shared_count] = fence_excl;
shared[shared_count++] = fence_excl;Oops.
Reviewed-by: Chris Wilson chris@chris-wilson.co.uk
Applied, thanks for the fix. -Chris
On Sun, Sep 22, 2019 at 2:08 PM Qiang Yu yuq825@gmail.com wrote:
This causes kernel crash when testing lima driver.
Cc: Christian König christian.koenig@amd.com Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") Signed-off-by: Qiang Yu yuq825@gmail.com
Selftest for this would be lovely, now that the basic infrastructure is in place ... -Daniel
drivers/dma-buf/dma-resv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 42a8f3f11681..709002515550 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, if (pfence_excl) *pfence_excl = fence_excl; else if (fence_excl)
shared[++shared_count] = fence_excl;
shared[shared_count++] = fence_excl; if (!shared_count) { kfree(shared);-- 2.17.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Am 30.09.19 um 09:22 schrieb Daniel Vetter:
On Sun, Sep 22, 2019 at 2:08 PM Qiang Yu yuq825@gmail.com wrote:
This causes kernel crash when testing lima driver.
Cc: Christian König christian.koenig@amd.com Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") Signed-off-by: Qiang Yu yuq825@gmail.com
Selftest for this would be lovely, now that the basic infrastructure is in place ...
What do you have in mind? I wouldn't even know where to start to write an unit test for this.
Christian.
-Daniel
drivers/dma-buf/dma-resv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 42a8f3f11681..709002515550 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, if (pfence_excl) *pfence_excl = fence_excl; else if (fence_excl)
shared[++shared_count] = fence_excl;
shared[shared_count++] = fence_excl; if (!shared_count) { kfree(shared);-- 2.17.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Sep 30, 2019 at 08:57:32AM +0000, Koenig, Christian wrote:
Am 30.09.19 um 09:22 schrieb Daniel Vetter:
On Sun, Sep 22, 2019 at 2:08 PM Qiang Yu yuq825@gmail.com wrote:
This causes kernel crash when testing lima driver.
Cc: Christian König christian.koenig@amd.com Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") Signed-off-by: Qiang Yu yuq825@gmail.com
Selftest for this would be lovely, now that the basic infrastructure is in place ...
What do you have in mind? I wouldn't even know where to start to write an unit test for this.
1. set a few fences (both excl + shared) in a dma_resv 2. get them 3. check that we got them all 4. notice that the exlusive fence isn't actually in the array (because we increment the index before storing, so the exclusive fence ended past the array). For robustness the test should check that the fences are listed in any order, not just the one the current implementation gives you.
I guess the actual crash happens when we're unlucky and overflow the allocation, which is probably more rare. But KASAN should help catch that too (run that in your CI if you don't do that yet, it's pretty impressive).
Or am I totally misunderstanding what's going wrong here? -Daniel
Christian.
-Daniel
drivers/dma-buf/dma-resv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 42a8f3f11681..709002515550 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, if (pfence_excl) *pfence_excl = fence_excl; else if (fence_excl)
shared[++shared_count] = fence_excl;
shared[shared_count++] = fence_excl; if (!shared_count) { kfree(shared);-- 2.17.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Daniel Vetter -
Koenig, Christian -
Qiang Yu