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

atomMutexPut() corrupts unclaimed mutex when called from interrupt context #38

Open
A-Dunstan opened this issue Oct 18, 2023 · 2 comments

Comments

@A-Dunstan
Copy link

A-Dunstan commented Oct 18, 2023

When atomMutexPut() is called from an interrupt context, curr_tcb_ptr is not checked against NULL. It is checked against mutex->owner, and if the mutex is unclaimed it will match and result in mutex->count being decremented - which will underflow to 255 since the original value will be 0. At this stage the mutex can no longer be claimed (due to count==255) but if it happens multiple times it can leave the mutex in a state where it can be claimed (once!) but unable to be released due to a corrupt count value.

@tidklaas
Copy link
Contributor

The documentation clearly states that atomMutexGet() and atomMutexPut() must not be called from interrupt context. If you need mutex-like behaviour in interrupt context, you should use semaphores with a count of 1.

@A-Dunstan
Copy link
Author

A-Dunstan commented Oct 20, 2023

The documentation also clearly states: "Any attempt to make a call which cannot be made from interrupt context will be automatically and safely prevented" which is definitely not the case here.
atomMutexGet() (along with most of the atom kernel API functions) has checks for incorrect usage and returns an error. By contrast atomMutexPut() will silently corrupt the mutex state with no error returned.

There are even tests that perform this incorrect usage in tests/mutex2.c (tests atomMutexGet() called from ISR) and tests/mutex7.c (tests atomMutexPut() called from ISR, but the mutex is owned at the time), so the behaviour is defined rather than undefined.

The test in mutex7.c is particularly relevant as I believe it's relying on the wrong error code to be returned - it tests for ATOM_ERR_OWNERSHIP when the correct error to be returned in that case should be ATOM_ERR_CONTEXT.

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

No branches or pull requests

2 participants