Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: fix race in mobj_reg_shm_dec_map() #6137

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

jenswi-linaro
Copy link
Contributor

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.

Fixes: 06ea466 ("core: fix race in mobj_reg_shm_inc_map()")

@jenswi-linaro
Copy link
Contributor Author

Another fix for #6120

@jenswi-linaro
Copy link
Contributor Author

Updated the commit message for ffa_dec_map().

Copy link
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Jerome Forissier <[email protected]>

@jenswi-linaro
Copy link
Contributor Author

Tag applied.

@omasse-linaro
Copy link
Contributor

omasse-linaro commented Jun 27, 2023

Acked-by: Olivier Masse <[email protected]>

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]>
Acked-by: Olivier Masse <[email protected]>
@jenswi-linaro
Copy link
Contributor Author

Tag applied

@jforissier jforissier merged commit 83a3d56 into OP-TEE:master Jun 27, 2023
@jenswi-linaro jenswi-linaro deleted the fix-reg_shm-race branch June 27, 2023 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants