On 24.07.2015 18:02, Mike Looijmans wrote: > On 24-07-15 10:39, Alexey Khoroshilov wrote: >> Dear Mike, >> >> On 24.07.2015 14:01, Mike Looijmans wrote: >>> Regarding this commit: >>> >>> https://lkml.org/lkml/2014/12/12/709 >>> >>> rsi: fix memory leak in rsi_load_ta_instructions() >>> >>> Memory allocated by kmemdup() in rsi_load_ta_instructions() is >>> leaked. >>> But duplication of firmware data here is useless, >>> so the patch removes kmemdup() at all. >>> >>> Found by Linux Driver Verification project (linuxtesting.org). >>> >>> Signed-off-by: Alexey Khoroshilov <khoroshilov@xxxxxxxxx> >>> Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxx> >>> >>> We use this driver for the Redpine Wifi chip on our "florida" board, and >>> after this commit it stopped working. Symptom was that the "wlan0" >>> device was not created at all. Reverting the commit makes it work again. >>> >>> Apparently, the kmemdup action is needed for something. I suspect the >>> DMA controller is still copying the firmware data before the method >>> returned. >> >> To test your hypothesis, could you please check if it is still broken >> with kfree(fw); added just after release_firmware(fw_entry); in >> rsi_load_ta_instructions(). > > Tried, and appears to work if i just kfree() the firmware copy. It does > leave a bad taste though. I'd expect fw_entry->data to point to a > kmalloc'd area as well. So it might work now just because it happens to > be that the memory if "far enough away" and isn't being touched by > anything else until the transfer is done. And on some other setup, it > may suddenly fail unexpectedly. > > I thought to move the kfree to a point where the driver unregisters, but > apparently it doesn't have any internal hook for that (sdio_done or so). > > I'd really like to see some comment from the Redpine folks on this, but > since there hasn't been any event in the past year or so, I don't expect > much. May be if firmware comes from userspace it is mapped to both kernel and userspace and by some reason it is not good for DMA. Another idea is fw_entry->data appears to be misaligned somehow. -- Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
