fix(fapi): fix memory leak in FAPI and tests by hyperfinitism · Pull Request #695 · tpm2-software/tpm2-pytss

hyperfinitism · 2026-07-06T14:05:54Z

This pull request fixes native memory leaks in FAPI and tests:

FAPI
- decrypt()
- get_platform_certificate()
- get_tpm_blobs())
test_policy.py
- test_callbacks()
test_tcti.py
- test_custom_pytcti_esapi()
- test_custom_pytcti_ctx_manager_finalize()

For example, Fapi_Decrypt() returns the plaintext through a FAPI-allocated output buffer. The Python wrapper copied that buffer into bytes, but did not release the native allocation with Fapi_Free(). This caused every successful FAPI.decrypt() call to leak the decrypted plaintext buffer. This PR wraps the returned plaintext pointer with _get_dptr(..., lib.Fapi_Free) before unpacking it, matching the ownership handling already used by FAPI.encrypt() and other wrappers that receive FAPI-allocated output buffers.

Reproduction (for FAPI_Decrypt)

Code

Gist: https://gist.github.com/hyperfinitism/5d4bc1d7c396f02a841fd32d4f137cd1
run_fapi_decrypt_valgrind.sh
fapi_decrypt_leak_probe.py

Download both files from the Gist into the tpm2-pytss repository root, then run the shell script from the repository root in an environment with tpm2-pytss, swtpm, tss2-fapi, libtss2-tcti-swtpm, and valgrind installed.

ITERATIONS=200 SIZE=128 LOG=/tmp/fapi-decrypt-valgrind.%p.log \
  bash ./run_fapi_decrypt_valgrind.sh

Valgrind result before the fix

With ITERATIONS=200 SIZE=128, Valgrind reported an iteration-scaled definite leak from Fapi_Decrypt_Finish:

==104== 25,600 bytes in 200 blocks are definitely lost in loss record 5,748 of 5,751
==104==    at 0x4848899: malloc
==104==    by 0x5AA7DEC: Fapi_Decrypt_Finish (in /usr/local/lib/libtss2-fapi.so.1.0.0)
==104==    by 0x5AA863D: Fapi_Decrypt (in /usr/local/lib/libtss2-fapi.so.1.0.0)
==104==    by 0x58F5ACA: _cffi_f_Fapi_Decrypt (tpm2_pytss._libtpm2_pytss.c:34176)

The 25,600 bytes in 200 blocks leak matches 200 decrypt operations of a 128 byte plaintext.

Overall leak summary before this fix:

==104== LEAK SUMMARY:
==104==    definitely lost: 30,226 bytes in 236 blocks
==104==    indirectly lost: 64 bytes in 1 blocks
==104==      possibly lost: 938,724 bytes in 8,886 blocks
==104==    still reachable: 222,964 bytes in 2,292 blocks
==104== ERROR SUMMARY: 34 errors from 34 contexts (suppressed: 0 from 0)

Validation after the fix

The existing FAPI encrypt/decrypt test passes:

python3 -m pytest test/test_fapi.py::TestFapiRSA::test_encrypt_decrypt -q

1 passed

After applying this fix and rerunning the shell script, the Fapi_Decrypt_Finish leak record is gone. The definite leak total drops by exactly 25,600 bytes and 200 blocks for ITERATIONS=200 SIZE=128, which confirms that the per-decrypt plaintext buffer is now released.

Before:

definitely lost: 30,226 bytes in 236 blocks

After:

definitely lost: 4,626 bytes in 36 blocks

Follow-up proposal: add Valgrind job in CI

Existing tests cannot detect this type of errors. A Valgrind job in CI would help catch similar ownership bugs in CFFI wrappers. (Note added on Jul 7: PR #696 implements this CI job.)

whooo · 2026-07-08T11:35:58Z

-        p = policy(polstr, TPM2_ALG.SHA256)
-        p.set_callback(policy_cb_types.CALC_PCR, test)
-        cb = p._get_callback(policy_cb_types.CALC_PCR)
-        self.assertEqual(cb, test)
+        with policy(polstr, TPM2_ALG.SHA256) as p:
+            p.set_callback(policy_cb_types.CALC_PCR, test)
+            cb = p._get_callback(policy_cb_types.CALC_PCR)
+            self.assertEqual(cb, test)

-        p.set_callback(policy_cb_types.CALC_PCR, None)
-        cb = p._get_callback(policy_cb_types.CALC_PCR)
-        self.assertEqual(cb, None)
+            p.set_callback(policy_cb_types.CALC_PCR, None)
+            cb = p._get_callback(policy_cb_types.CALC_PCR)
+            self.assertEqual(cb, None)

-        with self.assertRaises(ValueError) as e:
-            p.set_callback(1234, test)
-        self.assertEqual(str(e.exception), "unsupported callback type 1234")
+            with self.assertRaises(ValueError) as e:
+                p.set_callback(1234, test)
+            self.assertEqual(str(e.exception), "unsupported callback type 1234")


I would like to see this in a separate commit (can be the same PR)

I have split it into two commits.

whooo · 2026-07-08T11:40:58Z

+        with MyTCTI(self.tcti) as t:
+            with ESAPI(t) as e:


No need for two lines, both contexts can be on the same block-level

whooo · 2026-07-08T11:42:07Z

        with MyTCTI(self.tcti) as t:
-            e = ESAPI(t)
-            r = e.get_random(4)
-            self.assertEqual(len(r), 4)
-            e.startup(TPM2_SU.CLEAR)
+            with ESAPI(t) as e:


Same here, move both contexts to the same level

whooo · 2026-07-08T11:43:56Z

-        t = MyTCTI(self.tcti)
-        e = ESAPI(t)
-        e.get_random(4)
+        with MyTCTI(self.tcti) as t:
+            with ESAPI(t) as e:
+                e.get_random(4)

-        e.startup(TPM2_SU.CLEAR)
+                e.startup(TPM2_SU.CLEAR)

    def test_custom_pytcti_C_wrapper_transmit_receive(self):

        t = MyTCTI(self.tcti)

        # Go through the C API directly and call transmit and recv
        t.transmit(b"\x80\x01\x00\x00\x00\x0C\x00\x00\x01\x44\x00\x00")
        resp = t.receive(-1)
        self.assertEqual(resp, b"\x80\x01\x00\x00\x00\n\x00\x00\x01\x00")

    def test_custom_pytcti_cancel(self):
        if getattr(self.tcti, "name", "") == "swtpm":
            self.skipTest("cancel not supported by swtpm")

        t = MyTCTI(self.tcti)

        t.transmit(b"\x80\x01\x00\x00\x00\x0C\x00\x00\x01\x44\x00\x00")
        t.cancel()

    def test_custom_pytcti_finalize(self):
        t = MyTCTI(self.tcti)
        t.finalize()
        self.assertTrue(t.is_finalized)

    def test_custom_pytcti_get_poll_handles(self):
        tcti_name = getattr(self.tcti, "name", "")
        t = MyTCTI(self.tcti)
        try:
            handles = t.get_poll_handles()
            for h in handles:
                self.assertTrue(isinstance(h, PollData))
        except TSS2_Exception as e:
            if e.rc != lib.TSS2_TCTI_RC_NOT_IMPLEMENTED:
                raise e
            else:
                self.skipTest(f"get_poll_handles not supported by {tcti_name}")

    def test_custom_pytcti_set_locality(self):
        t = MyTCTI(self.tcti)
        t.set_locality(TPMA_LOCALITY.TWO)

    def test_custom_pytcti_make_sticky(self):
        t = MyTCTI(None)
        t._error = None
        t.make_sticky(0, 0)
        t.make_sticky(0, 1)
        t.make_sticky(0, False)

        # Test that throwing an exception shows the originating exception
        t._error = RuntimeError("Bills Error")
        with self.assertRaises(RuntimeError, msg="Bills Error"):
            t.make_sticky(5, True)

        t._v2 = None
        with self.assertRaises(TSS2_Exception):
            t.make_sticky(0, 0)

    def test_custom_pytcti_version(self):
        t = MyTCTI(None)
        self.assertEqual(t.version, 2)

    def test_custom_pytcti_magic(self):
        t = MyTCTI(None)
        magic = b"PYTCTI\x00\x00"
        self.assertEqual(t.magic, magic)

        # max magic len
        magic = b"THISISIT"
        t = MyTCTI(None, magic)
        self.assertEqual(t.magic, magic)

        # small magic len
        magic = b"COOL"
        t = MyTCTI(None, magic)
        self.assertEqual(t.magic, magic)

        # min magic
        magic = b""
        t = MyTCTI(None, magic)
        self.assertEqual(t.magic, magic)

        with self.assertRaises(ValueError):
            MyTCTI(None, b"THISISTOOBIG")

    def test_custom_pytcti_ctx_manager_finalize(self):
        with MyTCTI(self.tcti) as t:
-            e = ESAPI(t)
-            r = e.get_random(4)
-            self.assertEqual(len(r), 4)
-            e.startup(TPM2_SU.CLEAR)
+            with ESAPI(t) as e:
+                r = e.get_random(4)
+                self.assertEqual(len(r), 4)
+                e.startup(TPM2_SU.CLEAR)

        self.assertTrue(t.is_finalized)


Please move this to another commit (can be the same as for the policy tests)

Signed-off-by: Takuma IMAMURA <209989118+hyperfinitism@users.noreply.github.com>

hyperfinitism force-pushed the fix/fapi-decrypt-leak branch from f6df147 to b707919 Compare July 6, 2026 18:48

hyperfinitism changed the title ~~fix(fapi): fix memory leak in FAPI.decrypt()~~ fix(fapi): fix memory leak in FAPI and tests Jul 6, 2026

github-advanced-security Bot found potential problems Jul 6, 2026

View reviewed changes

Comment thread test/test_policy.py Fixed

hyperfinitism force-pushed the fix/fapi-decrypt-leak branch 3 times, most recently from 9884b3e to 6ffc3c8 Compare July 7, 2026 02:41

whooo added this to the next (master branch) milestone Jul 8, 2026

whooo requested changes Jul 8, 2026

View reviewed changes

fix: fix FAPI output buffer leaks

c12ba77

Signed-off-by: Takuma IMAMURA <209989118+hyperfinitism@users.noreply.github.com>

hyperfinitism force-pushed the fix/fapi-decrypt-leak branch 2 times, most recently from 154b428 to 4155a26 Compare July 8, 2026 11:57

fix: close test contexts to prevent leaks

e8a99cd

Signed-off-by: Takuma IMAMURA <209989118+hyperfinitism@users.noreply.github.com>

hyperfinitism force-pushed the fix/fapi-decrypt-leak branch from 4155a26 to e8a99cd Compare July 8, 2026 12:04

whooo approved these changes Jul 8, 2026

View reviewed changes

whooo merged commit fb53eea into tpm2-software:master Jul 8, 2026
30 checks passed

Navigation Menu

Uh oh!

fix(fapi): fix memory leak in FAPI and tests - #695

fix(fapi): fix memory leak in FAPI and tests#695
whooo merged 2 commits into
tpm2-software:masterfrom
hyperfinitism:fix/fapi-decrypt-leak

hyperfinitism commented Jul 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

whooo Jul 8, 2026

Uh oh!

hyperfinitism Jul 8, 2026

Uh oh!

whooo Jul 8, 2026

Uh oh!

hyperfinitism Jul 8, 2026

Uh oh!

whooo Jul 8, 2026

Uh oh!

hyperfinitism Jul 8, 2026

Uh oh!

whooo Jul 8, 2026

Uh oh!

hyperfinitism Jul 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

hyperfinitism commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reproduction (for FAPI_Decrypt)

Code

Valgrind result before the fix

Validation after the fix

Follow-up proposal: add Valgrind job in CI

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hyperfinitism commented Jul 6, 2026 •

edited

Loading