From c9fc4a5779db08a6c8a0029b779826a188d8b438 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Sun, 23 Apr 2017 18:30:50 -0400 Subject: Avoid deadlock triggered by low-probability race condition. Static code analysis (Doxygen call graph) detected a low-probability race condition which could have triggered a deadlock on the keystore mutex if the mkmif code returns with an error like HAL_ERROR_CORE_BUSY when we're trying to fetch the KEK. This is a knock-on effect of the awful kludge of backing up the KEK in the keystore flash as an alternative to powering the MKM with a battery as called for in the design. This code path should not exist at all, but, for now, we avoid the deadlock by making it the caller's responsibility to grab the keystore mutex before looking up the KEK. --- mkm.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'mkm.c') diff --git a/mkm.c b/mkm.c index 2b2141f..05c733d 100644 --- a/mkm.c +++ b/mkm.c @@ -201,7 +201,15 @@ hal_error_t hal_mkm_get_kek(uint8_t *kek, #if HAL_MKM_FLASH_BACKUP_KLUDGE - if (hal_mkm_flash_read(kek, len) == LIBHAL_OK) { + /* + * It turns out that, in every case where this function is called, + * we already hold the keystore lock, so attempting to grab it again + * would deadlock. This almost never happens when the volatile MKM + * is set, but there's a race condition that might drop us here if + * hal_mkm_volatile_read() returns HAL_ERROR_CORE_BUSY. Whee! + */ + + if (hal_mkm_flash_read_no_lock(kek, len) == LIBHAL_OK) { *kek_len = len; return LIBHAL_OK; } -- cgit v1.2.3