Skip to content

Commit

Permalink
core: fix race in mobj_reg_shm_dec_map()
Browse files Browse the repository at this point in the history
Fixes a race in mobj_reg_shm_dec_map() when r->mm is NULL. This is
similar to the race fixed by commit 06ea466 ("core: fix race in
mobj_reg_shm_inc_map()"), but with one more possibility.

The problem goes like:
A. Thread 1 calls mobj_reg_shm_dec_map() at the same time as thread 2
   calls mobj_reg_shm_inc_map().
B. Thread 1 decreases mapcount to zero and tries to take the spinlock,
   but thread 1 is suspended before it has acquired the spinlock.
C. Thread 2 sees that mapcount is zero and takes the spinlock and maps
   the memory.
D. Thread 2 calls mobj_reg_shm_dec_map(), mapcount reaches zero again
   and the shared memory is unmapped and r->mm is set to NULL.
E. Thread 1 is finally resumed and acquires the spinlock, mapcount is still
   zero but r->mm is also NULL.

To fix the problem at step E above check that r->mm is still non-NULL.

Note that the same fix isn't needed for ffa_dec_map() since
unmap_helper() checks that mf->mm is non-NULL first.

Fixes: 06ea466 ("core: fix race in mobj_reg_shm_inc_map()")
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
  • Loading branch information
jenswi-linaro committed Jun 27, 2023
1 parent 504f159 commit be4b7e1
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions core/arch/arm/mm/mobj_dyn_shm.c
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ static TEE_Result mobj_reg_shm_inc_map(struct mobj *mobj)
}

/*
* If we have beated another thread calling mobj_reg_shm_dec_map()
* If we have beaten another thread calling mobj_reg_shm_dec_map()
* to get the lock we need only to reinitialize mapcount to 1.
*/
if (!r->mm) {
Expand Down Expand Up @@ -238,7 +238,14 @@ static TEE_Result mobj_reg_shm_dec_map(struct mobj *mobj)

exceptions = cpu_spin_lock_xsave(&reg_shm_map_lock);

if (!refcount_val(&r->mapcount))
/*
* Check that another thread hasn't been able to:
* - increase the mapcount
* - or, increase the mapcount, decrease it again, and set r->mm to
* NULL
* before we acquired the spinlock
*/
if (!refcount_val(&r->mapcount) && r->mm)
reg_shm_unmap_helper(r);

cpu_spin_unlock_xrestore(&reg_shm_map_lock, exceptions);
Expand Down

0 comments on commit be4b7e1

Please sign in to comment.