libminkadaptor: Fix memory object leak in TA autoload CBO

[Jiaxing Li (jiaxli-QC)]

libtaautoload: Fix memory object leak in TA autoload CBO

CRequestTABuffer_get() hands out a memObj for each requested TA image but retains a reference on it, so every TA autoload leaked one memObj that was never released. Track each handed-out memObj in a per-instance memObjList and add an IRequestTABuffer.cleanup() method that walks the list and drops those references.

libminkadaptor invokes the new cleanup() from qcomtee_callback_obj_cleanup(), which QCOMTEE calls after the callback response has been marshalled and sent, so the references are released once the memObj is no longer needed.

Wire-up:

• Add the cleanup() method to IRequestTABuffer.idl.
• Track memObj nodes with a QList; free them in CRequestTABuffer_cleanup().
• Override container_of() in CRequestTABuffer.h with a char*-based definition, since qlist.h's void*-arithmetic version is ill-formed in C++ and rejected under -Werror=pointer-arith.
• Add the required include directories to both CMakeLists.txt files.

CRs-Fixed: 4622877
Fix-issue: #9

[Harshal Dev (harshaldev27)]

This is completely wrong. The underlying issue here is that when CRequestTABuffer_get(..., Object *appElf) callback is called, it allocates a memory object within the createTAImageReader() call, a reference to which is copied to *appElf pointer, and then the only reference that CRequestTABuffer_get() has to the object is destroyed when delete(TAImage); is called before exiting CRequestTABuffer_get(). So we have no way to call Object_release on the memory object allocated. The memory object reference however still lives in the libqcomtee's object-list. It can only be free'd if the CBO executes an explicit Object_release call.

So, we have two options:

1. We need to keep references to the allocated memory object within CRequestTABuffer which can be free'd when the deinit() defined in TAAutoload.cpp calls Object_ASSIGN_NULL(requestTABuffer);.

2. A better approach to this, is to add support for an error() or notify() callback in CRequestTABuffer which can be invoked from mink adaptor which already gets the notification from qcomtee when a successful CBO response has been provided to QTEE. Since in this particular use-case, once we have shared the memory object to QTEE after placing the TA binary inside it, QTEE will never come back and ask us to operate on it again, it is safe to release this memory object reference once we are done with sending the MO back to QTEE.

[Harshal Dev (harshaldev27)]

Please make the changes as requested.

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

[Jiaxing Li (jiaxli-QC)]

This is completely wrong. The underlying issue here is that when CRequestTABuffer_get(..., Object *appElf) callback is called, it allocates a memory object within the createTAImageReader() call, a reference to which is copied to *appElf pointer, and then the only reference that CRequestTABuffer_get() has to the object is destroyed when delete(TAImage); is called before exiting CRequestTABuffer_get(). So we have no way to call Object_release on the memory object allocated. The memory object reference however still lives in the libqcomtee's object-list. It can only be free'd if the CBO executes an explicit Object_release call.

So, we have two options:

1. We need to keep references to the allocated memory object within CRequestTABuffer which can be free'd when the deinit() defined in TAAutoload.cpp calls Object_ASSIGN_NULL(requestTABuffer);.
2. A better approach to this, is to add support for an error() or notify() callback in CRequestTABuffer which can be invoked from mink adaptor which already gets the notification from qcomtee when a successful CBO response has been provided to QTEE. Since in this particular use-case, once we have shared the memory object to QTEE after placing the TA binary inside it, QTEE will never come back and ask us to operate on it again, it is safe to release this memory object reference once we are done with sending the MO back to QTEE.

Hi Harshal Dev (@harshaldev27) , Please review the changes. Thanks for your support.

Fix this issue based on the option 2. and Updated the description for this PR.

[Jiaxing Li (jiaxli-QC)]

Please make the changes as requested.

Yes, Thanks for your comments. Could you review the changes? Thank you very much.