Memory leaks related to Trousers Scoped RAII classes (TPM 1.2) |
|||
Issue description* Some bugs inside the ScopedTss* classes in the Trousers. * Some bugs in cryptohomed and attestation with regard to how the ScopedTss* classes are used. Severity is probably not high, given that the resource handling of TSS_HCONTEXT seems to be correct, and releasing of the context frees all memory allocated using it. However, there's one long-living TSS_HCONTEXT, which could be the reason of gradual accumulation of leaked resources.
,
Mar 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/a43f60fa3f7bdff033a9678dad36282f68f5a701 commit a43f60fa3f7bdff033a9678dad36282f68f5a701 Author: Maksim Ivanov <emaxx@google.com> Date: Wed Mar 21 05:21:20 2018 attestation: Fix memory leaks in TPM 1.2 ScopedTssMemory usages This fixes several instantiations of ScopedTssMemory to not leak the allocated memory. They were leaking because the zero TSS_HCONTEXT was provided to them, which was making the ScopedTssMemory destructor effectively a no-op. BUG= chromium:821825 ,chromium:806788 TEST=existing tests Change-Id: I945e9513caa3ea2255cbd7f66fde370715bce93d Reviewed-on: https://chromium-review.googlesource.com/962426 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Maksim Ivanov <emaxx@chromium.org> Reviewed-by: Andrey Pronin <apronin@chromium.org> [modify] https://crrev.com/a43f60fa3f7bdff033a9678dad36282f68f5a701/attestation/common/tpm_utility_v1.cc
,
Mar 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/aa81d1626b191acb2858c2d49ea442d72b3030bb commit aa81d1626b191acb2858c2d49ea442d72b3030bb Author: Maksim Ivanov <emaxx@google.com> Date: Wed Mar 21 11:39:41 2018 cryptohome: Fix memory leaks in TPM 1.2 ScopedTssMemory usages This fixes several instantiations of ScopedTssMemory to not leak the allocated memory. They were leaking because the zero TSS_HCONTEXT was provided to them, which was making the ScopedTssMemory destructor effectively a no-op. BUG= chromium:821825 ,chromium:806788 TEST=existing tests Change-Id: I9bbf3f09f2151d72a456b9d5396f386bd9dc8fe3 Reviewed-on: https://chromium-review.googlesource.com/957722 Commit-Ready: Maksim Ivanov <emaxx@chromium.org> Tested-by: Maksim Ivanov <emaxx@chromium.org> Reviewed-by: Will Drewry <wad@chromium.org> Reviewed-by: Andrey Pronin <apronin@chromium.org> [modify] https://crrev.com/aa81d1626b191acb2858c2d49ea442d72b3030bb/cryptohome/tpm_impl.cc
,
Mar 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/trousers/+/d7fa9879234533afab08f137e0f1efc36a5c17b9 commit d7fa9879234533afab08f137e0f1efc36a5c17b9 Author: Maksim Ivanov <emaxx@google.com> Date: Fri Mar 23 21:50:46 2018 Trousers: Fix resource leakage in ScopedTss* objects * ScopedTssObject (and all its template instantiations) was effectively always leaking the associated resource by forgetting to initialize the TSS_HCONTEXT. * Replace silent resource cleanup ignoring when TSS_HCONTEXT is zero with NOTREACHED - i.e. log on leaking when under Release mode, and crash when under Debug mode. BUG= chromium:821825 ,chromium:806788 TEST=existing tests Change-Id: Ie2c45a20412af09b543869167007ad4ba08ac4ab Reviewed-on: https://chromium-review.googlesource.com/959964 Commit-Ready: Maksim Ivanov <emaxx@chromium.org> Tested-by: Maksim Ivanov <emaxx@chromium.org> Reviewed-by: Andrey Pronin <apronin@chromium.org> [modify] https://crrev.com/d7fa9879234533afab08f137e0f1efc36a5c17b9/src/include/trousers/scoped_tss_type.h
,
Apr 26 2018
Closing this bug - the leaks should've been fixed. There is still one improvement to be done - which is to collect stats on whether the added assertions are ever hit, and after checking this data transforming the assertions into production-enabled CHECKs. However, this gets postponed for an indefinite time till libchrome gets uprev'ed (see https://crbug.com/724678#c7 ). |
|||
►
Sign in to add a comment |
|||
Comment 1 by emaxx@chromium.org
, Mar 14 2018