subsystem: sd: sd_ops: mutex unlocked twice after being locked only once #72287
Assignees
Labels
area: Disk Access
bug
The issue is a bug, or the PR is fixing a bug
priority: medium
Medium impact/importance bug
Comments
|
This came up via discord. Indeed, this looks like a mixed up unlock call to me, maybe a leftover of a previous variant. It's likely benign as the function will fail with an -EINVAL on the misatched unlock, though it does open the possibility of hijinx if it's possible to fool the subsystem into incorrectly releasing a correctly-held lock. Relevant stray unlock is here: https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/sd/sd_ops.c#L538 |
danieldegrasse
added a commit
to nxp-upstream/zephyr
that referenced
this issue
May 14, 2024
SD ops card_read() implementation does not need to unlock mutex, as this is managed by the calling function card_write_blocks. Remove this stray k_mutex_unlock() call. Fixes zephyrproject-rtos#72287 Signed-off-by: Daniel DeGrasse <[email protected]>
mariopaja
pushed a commit
to mariopaja/zephyr
that referenced
this issue
May 26, 2024
SD ops card_read() implementation does not need to unlock mutex, as this is managed by the calling function card_write_blocks. Remove this stray k_mutex_unlock() call. Fixes zephyrproject-rtos#72287 Signed-off-by: Daniel DeGrasse <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The function
card_readinsubsys/sd/sd_ops.cunlocks the&card->lockif thesdmmc_wait_readyoperation fails.It does so without having locked it first. It is locked by the caller once but also unlocked again after the
card_readfunction call no matter the result. This means, that there is a code-path that would unlock it twice after having it locked only once.Expected Fix: Remove the
unlockcall fromcard_read. It might be a forgotten line when refactoring.The impact depends on what happens if a lock is unlocked one time too many. Might be Undefined Behavior.
Please let me know, if you need additional information. Thank you.
Edit: Apparently it will fail with
-EINVAL.The text was updated successfully, but these errors were encountered: