libnfc
libnfc copied to clipboard
Fixed memory leak in nfc_initiator_select_passive_target()
I checked my app, which is using libnfc, with Valgrind and it discovered, that function nfc_initiator_select_passive_target()
is leaking 12B.
https://github.com/nfc-tools/libnfc/blob/d9a04a54ffd99348cbbbf7c2e5e4747c2b8fe404/libnfc/nfc.c#L576-L591
Problem is in calling macro HAL
before free(), because the macro is returning from nfc_initiator_select_passive_target()
, so the free() afterwars is never called. And in case nm.nmt == NMT_ISO14443A
, abtTmpInit
isn't deallocated.
But as I'm looking at it now, my solution isn't good, because abtInit
is used after my added free() by iso14443_cascade_uid()
. And it is used as abtInit
, which is pointing to deallocated address, also by macro HAL
and initiator_select_passive_target
.
Ah, I see… thanks you for the context, I did not checked in that area. Returning from a macro is never a good idea… We can either do some big refactoring to avoid this macro completely, or a short-term easier fix would be to call the macro from another internal function, save the returned value, free memory and return the saved value.
Yeah, reworking the HAL
macro would be a good idea, but as a much easier fix, I simply replaced malloc()
with alloca()
, so the memory is allocated on the stack and automatically deallocated when exiting the function. As the size of the allocated memory is small, use of alloca()
should be fine.
Hum :thinking: I am concerned about portability here… I don't know if many people use the libnfc code in embedded systems, but they might not have an alloca()
function available at hand. @nfc-tools/core though?