dmaengine: dmatest: bug out when dma test times out
PATCH 1/1 dmaengine: dmatest: bug out when dma test times out
Commit adfa543e7314 ("dmatest: don't use set_freezable_with_signal()") introduced a bug (that is in fact documented by the patch commit text) that leaves behind a dangling pointer. Since the done_wait structure is allocated on the stack, future invocations to the DMATEST can produce undesirable results (e.g., corrupted spinlocks). Ideally, this would be cleaned up in the the thread handler, but at the very least, the kernel is left in a very precarious scenario that can lead to some long debug sessions when the crash comes later.
This bug has also been captured at
https://bugzilla.kernel.org/show_bug.cgi?id=197605
--- drivers/dma/dmatest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/dma/dmatest.c +1 -1
@@ -706,7 +706,7 @@ static int dmatest_func(void *data)result ("test timed out" , total_tests, src_off, dst_off, len,0 ); failed_tests++; -continue ; +BUG (); }else if (status !=DMA_COMPLETE ) {dmaengine_unmap_put (um); result(status == DMA_ERROR ?
Signature · 3 lines
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 dmaengine: dmatest: bug out when dma test times out
Quoted 22 lines · Adam Wallis
Commit
adfa543e7314("dmatest: don't useset_freezable_with_signal()") introduced a bug (that is in fact documented by the patch commit text) that leaves behind a dangling pointer. Since thedone_waitstructure is allocated on the stack, future invocations to the DMATEST can produce undesirable results (e.g., corrupted spinlocks). Ideally, this would be cleaned up in the the thread handler, but at the very least, the kernel is left in a very precarious scenario that can lead to some long debug sessions when the crash comes later.This bug has also been captured at
https://bugzilla.kernel.org/show_bug.cgi?id=197605Signed-off-by Adam Wallis <[email protected]>--- drivers/dma/dmatest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/dma/dmatest.c +0
-0
drivers/dma/dmatest.c static int dmatest_func(void *data) · line 709
Quoted 5 lines
drivers/dma/dmatest.c +1
-1@@ -706,7 +706,7 @@ static int dmatest_func(void *data)result ("test timed out" , total_tests, src_off, dst_off, len,0 ); failed_tests++; -continue ; +BUG ();
This will essentially kill the system. People have been trying to remove usage of BUG(), we should not do that. Complaining violently makes sense..
Quoted 8 lines
drivers/dma/dmatest.c +0
-0}else if (status !=DMA_COMPLETE ) {dmaengine_unmap_put (um); result(status == DMA_ERROR ?Signature · 3 lines
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Signature · 1 line
~Vinod
PATCH 1/1 dmaengine: dmatest: bug out when dma test times out
Quoted 18 lines · Vinod Koul
Quoted 17 lines · Adam Wallis
Commit
adfa543e7314("dmatest: don't useset_freezable_with_signal()") introduced a bug (that is in fact documented by the patch commit text) that leaves behind a dangling pointer. Since thedone_waitstructure is allocated on the stack, future invocations to the DMATEST can produce undesirable results (e.g., corrupted spinlocks). Ideally, this would be cleaned up in the the thread handler, but at the very least, the kernel is left in a very precarious scenario that can lead to some long debug sessions when the crash comes later.This bug has also been captured at
https://bugzilla.kernel.org/show_bug.cgi?id=197605Signed-off-by Adam Wallis <[email protected]>--- drivers/dma/dmatest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 14 lines
drivers/dma/dmatest.c static int dmatest_func(void *data) · line 709
Quoted 10 lines
drivers/dma/dmatest.c +1
-1@@ -706,7 +706,7 @@ static int dmatest_func(void *data)result ("test timed out" , total_tests, src_off, dst_off, len,0 ); failed_tests++; -continue ; +BUG ();This will essentially kill the system. People have been trying to remove usage of
BUG(), we should not do that. Complaining violently makes sense..
Understood. In my testing, the system is effectively shot if you run the dmatest in a tight enough loop, however, at least using a WARN would alert the user that the kernel might be in an unstable state. I will submit another patch with a WARN, even though, I hope a proper fix can be introduced at some point. I am not familiar enough with this code to propose such a fix at the moment though.
Quoted 10 lines
Quoted 9 lines
drivers/dma/dmatest.c +0
-0}else if (status !=DMA_COMPLETE ) {dmaengine_unmap_put (um); result(status == DMA_ERROR ?Signature · 3 lines
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Signature · 4 lines
Adam Wallis Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 dmaengine: dmatest: bug out when dma test times out
Quoted 8 lines · Adam Wallis
Quoted 3 lines
This will essentially kill the system. People have been trying to remove usage of
BUG(), we should not do that. Complaining violently makes sense..Understood. In my testing, the system is effectively shot if you run the dmatest in a tight enough loop, however, at least using a WARN would alert the user that the kernel might be in an unstable state. I will submit another patch with a WARN, even though, I hope a proper fix can be introduced at some point. I am not familiar enough with this code to propose such a fix at the moment though.
I think Adam's original patch is correct, because as he said, the code as it is today corrupts the kernel. The BUG() is a stop-cap measure to prevent the user from thinking that the kernel is stable when it isn't.
So I think that either dmatest gets fixed properly today (which Adam said he cannot do), or you should apply his patch until the code can get fixed.
Signature · 3 lines
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 dmaengine: dmatest: bug out when dma test times out
Quoted 10 lines · Timur Tabi
Quoted 8 lines · Adam Wallis
Quoted 3 lines
This will essentially kill the system. People have been trying to remove usage of
BUG(), we should not do that. Complaining violently makes sense..Understood. In my testing, the system is effectively shot if you run the dmatest in a tight enough loop, however, at least using a WARN would alert the user that the kernel might be in an unstable state. I will submit another patch with a WARN, even though, I hope a proper fix can be introduced at some point. I am not familiar enough with this code to propose such a fix at the moment though.
Quoted 3 lines
I think Adam's original patch is correct, because as he said, the code as it is today corrupts the kernel. The
BUG()is a stop-cap measure to prevent the user from thinking that the kernel is stable when it isn't.
It also helps people to get more information to debug the systems
Quoted 2 lines
So I think that either dmatest gets fixed properly today (which Adam said he cannot do), or you should apply his patch until the code can get fixed.
Ofcourse fixing is better.
Signature · 1 line
~Vinod