Skip to content

Commit

Permalink
storage: protect TA directory with a mutex
Browse files Browse the repository at this point in the history
There is a race condition in the code that creates and deletes trusted
storage. If multiple threads invoke a multi-session TA to create and
delete different files (such as xtest 6016), the following can occur:

    Thread #1 (create file #1) |   Thread #2 (delete file #2)
                               |
                               |   remove("/TA_dir/file");
    mkdir("/TA_dir");          |
                               |   rmdir("/TA_dir");
    create("/TA_dir/file1");   |
      => ENOENT                |

Add a mutex to prevent this race condition.

Note: the bug is currently not triggered by xtest 1016 because the test
is run for RPMB FS only, and because directory operations are no-ops in
the RPMB implementation. The fix will be needed when enabling single-TA
concurrency with the REE and SQL backends.

Signed-off-by: Jerome Forissier <[email protected]>
  • Loading branch information
jforissier committed Sep 13, 2016
1 parent bfdfbe7 commit cf2145f
Showing 1 changed file with 16 additions and 1 deletion.
17 changes: 16 additions & 1 deletion core/tee/tee_svc_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* POSSIBILITY OF SUCH DAMAGE.
*/

#include <kernel/mutex.h>
#include <kernel/tee_ta_manager.h>
#include <kernel/tee_misc.h>
#include <mm/tee_mmu.h>
Expand Down Expand Up @@ -97,6 +98,12 @@ struct tee_storage_enum {
const struct tee_file_operations *fops;
};

/*
* Protect TA storage directory: avoid race conditions between (create
* directory + create file) and (remove directory)
*/
static struct mutex ta_dir_mutex = MUTEX_INITIALIZER;

static TEE_Result tee_svc_storage_get_enum(struct user_ta_ctx *utc,
uint32_t enum_id,
struct tee_storage_enum **e_out)
Expand Down Expand Up @@ -210,7 +217,9 @@ static TEE_Result tee_svc_storage_remove_corrupt_obj(
free(file);
dir = tee_svc_storage_create_dirname(sess);
if (dir != NULL) {
mutex_lock(&ta_dir_mutex);
fops->rmdir(dir);
mutex_unlock(&ta_dir_mutex);
free(dir);
}

Expand Down Expand Up @@ -274,6 +283,8 @@ static TEE_Result tee_svc_storage_create_file(struct tee_ta_session *sess,
goto exit;
}

mutex_lock(&ta_dir_mutex);

/* try and make directory */
err = fops->access(dir, TEE_FS_F_OK);
if (err) {
Expand All @@ -284,7 +295,7 @@ static TEE_Result tee_svc_storage_create_file(struct tee_ta_session *sess,
if (tmp < 0) {
/* error codes needs better granularity */
res = TEE_ERROR_GENERIC;
goto exit;
goto unlock;
}
}

Expand All @@ -293,6 +304,8 @@ static TEE_Result tee_svc_storage_create_file(struct tee_ta_session *sess,
if (*fd < 0)
res = errno;

unlock:
mutex_unlock(&ta_dir_mutex);
exit:
free(dir);

Expand Down Expand Up @@ -862,8 +875,10 @@ TEE_Result syscall_storage_obj_del(unsigned long obj)
dir = tee_svc_storage_create_dirname(sess);
if (dir == NULL)
return TEE_ERROR_OUT_OF_MEMORY;
mutex_lock(&ta_dir_mutex);
/* ignore result */
fops->rmdir(dir);
mutex_unlock(&ta_dir_mutex);
free(dir);

return TEE_SUCCESS;
Expand Down

0 comments on commit cf2145f

Please sign in to comment.