| Message ID | 20200905201421.29495-1-gustav.wiklander@axis.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: Fix memory leak on splited transfers | expand |
On Sat, Sep 05, 2020 at 10:14:21PM +0200, Gustav Wiklander wrote: > From: Gustav Wiklander <gustavwi@axis.com> > > In the prepare_message callback the bus driver has the > opportunity to split a transfer into smaller chunks. > spi_map_msg is done after prepare_message. You've not included a signed-off-by so I can't do anything with this, see submitting-patches.rst for details on what this means and why it's important. > [ 76.611642][ T170] kmemleak: unreferenced object 0xfffffff06ef81480 > (size 128): > [ 76.618965][ T170] kmemleak: comm "insmod", pid 493, jiffies > 4294941102 (age 38.540s) > [ 76.627031][ T170] kmemleak: backtrace: > [ 76.631206][ T170] kmemleak: [<ffffffa542c5f8f8>] Please think hard before including complete backtraces in upstream reports, they are very large and contain almost no useful information relative to their size so often obscure the relevant content in your message. If part of the backtrace is usefully illustrative (it often is for search engines if nothing else) then it's usually better to pull out the relevant sections.
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index dc12af018350..0cab239d8e7f 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1327,8 +1327,6 @@ static int spi_transfer_one_message(struct spi_controller *ctlr, if (msg->status && ctlr->handle_err) ctlr->handle_err(ctlr, msg); - spi_res_release(ctlr, msg); - spi_finalize_current_message(ctlr); return ret; @@ -1725,6 +1723,13 @@ void spi_finalize_current_message(struct spi_controller *ctlr) spi_unmap_msg(ctlr, mesg); + /* In the prepare_messages callback the spi bus has the opportunity to + * split a transfer to smaller chunks. + * Release splited transfers here since spi_map_msg is done on the + * splited transfers. + */ + spi_res_release(ctlr, mesg); + if (ctlr->cur_msg_prepared && ctlr->unprepare_message) { ret = ctlr->unprepare_message(ctlr, mesg); if (ret) {
From: Gustav Wiklander <gustavwi@axis.com> In the prepare_message callback the bus driver has the opportunity to split a transfer into smaller chunks. spi_map_msg is done after prepare_message. Function spi_res_release releases the splited transfers in the message. Therefore spi_res_release should be called after spi_map_msg. The previous try at this was commit c9ba7a16d0f1 which released the splited transfers after spi_finalize_current_message had been called. This introduced a race since the message struct could be out of scope because the spi_sync call got completed. Fixes this leak on spi bus driver spi-bcm2835.c when transfer size is greater than 65532: [ 76.611642][ T170] kmemleak: unreferenced object 0xfffffff06ef81480 (size 128): [ 76.618965][ T170] kmemleak: comm "insmod", pid 493, jiffies 4294941102 (age 38.540s) [ 76.627031][ T170] kmemleak: backtrace: [ 76.631206][ T170] kmemleak: [<ffffffa542c5f8f8>] create_object+0x100/0x288 [ 76.638596][ T170] kmemleak: [<ffffffa5432a9ee4>] kmemleak_alloc+0x8c/0xe0 [ 76.645723][ T170] kmemleak: [<ffffffa542c4cbe8>] __kmalloc+0x1c8/0x370 [ 76.652754][ T170] kmemleak: [<ffffffa542e14c94>] sg_kmalloc+0x1c/0x68 [ 76.659782][ T170] kmemleak: [<ffffffa542e1543c>] __sg_alloc_table+0xf4/0x128 [ 76.667420][ T170] kmemleak: [<ffffffa542e15820>] sg_alloc_table+0x28/0xc8 [ 76.674636][ T170] kmemleak: [<ffffffa542f938a4>] spi_map_buf+0xa4/0x300 [ 76.681838][ T170] kmemleak: [<ffffffa542f94648>] __spi_pump_messages+0x370/0x748 [ 76.689573][ T170] kmemleak: [<ffffffa542f94c24>] __spi_sync+0x1d4/0x270 [ 76.696863][ T170] kmemleak: [<ffffffa542f94cf4>] spi_sync+0x34/0x58 [ 76.703562][ T170] kmemleak: [<ffffffa4cd94a638>] spi_test_execute_msg+0x60/0x340 [spi_loopback_test] [ 76.713193][ T170] kmemleak: [<ffffffa4cd94ae60>] spi_test_run_iter+0x548/0x578 [spi_loopback_test] [ 76.722740][ T170] kmemleak: [<ffffffa4cd94af24>] spi_test_run_test+0x94/0x140 [spi_loopback_test] [ 76.732037][ T170] kmemleak: [<ffffffa4cd94b120>] spi_test_run_tests+0x150/0x180 [spi_loopback_test] [ 76.741498][ T170] kmemleak: [<ffffffa4cd94b1a0>] spi_loopback_test_probe+0x50/0xd0 [spi_loopback_test] [ 76.751392][ T170] kmemleak: [<ffffffa542f911f4>] spi_drv_probe+0x84/0xe0 --- drivers/spi/spi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)