p4wq has a race with work item re-use #32052
Assignees
Labels
Comments
andyross
pushed a commit
to andyross/zephyr
that referenced
this issue
Feb 11, 2021
Work items can be legally resubmitted from within their own handler. Currently the p4wq detects this case by checking their thread field to see if it's been set to NULL. But that's a race, because if the item was NOT resubmitted then it no longer belongs to the queue and may have been freed or reused or otherwise clobbered legally by user code. Instead, steal a single bit in the thread struct for this purpose. This patch adds a K_CALLBACK_STATE bit in user_options and documents it in such a way (as being intended for "callback manager" utilities) that it can't be used recursively or otherwise collide. Fixes zephyrproject-rtos#32052 Signed-off-by: Andy Ross <[email protected]>
nashif
pushed a commit
that referenced
this issue
Feb 14, 2021
Work items can be legally resubmitted from within their own handler. Currently the p4wq detects this case by checking their thread field to see if it's been set to NULL. But that's a race, because if the item was NOT resubmitted then it no longer belongs to the queue and may have been freed or reused or otherwise clobbered legally by user code. Instead, steal a single bit in the thread struct for this purpose. This patch adds a K_CALLBACK_STATE bit in user_options and documents it in such a way (as being intended for "callback manager" utilities) that it can't be used recursively or otherwise collide. Fixes #32052 Signed-off-by: Andy Ross <[email protected]>
github-actions bot
pushed a commit
that referenced
this issue
Feb 14, 2021
Work items can be legally resubmitted from within their own handler. Currently the p4wq detects this case by checking their thread field to see if it's been set to NULL. But that's a race, because if the item was NOT resubmitted then it no longer belongs to the queue and may have been freed or reused or otherwise clobbered legally by user code. Instead, steal a single bit in the thread struct for this purpose. This patch adds a K_CALLBACK_STATE bit in user_options and documents it in such a way (as being intended for "callback manager" utilities) that it can't be used recursively or otherwise collide. Fixes #32052 Signed-off-by: Andy Ross <[email protected]>
nashif
pushed a commit
that referenced
this issue
Feb 17, 2021
Work items can be legally resubmitted from within their own handler. Currently the p4wq detects this case by checking their thread field to see if it's been set to NULL. But that's a race, because if the item was NOT resubmitted then it no longer belongs to the queue and may have been freed or reused or otherwise clobbered legally by user code. Instead, steal a single bit in the thread struct for this purpose. This patch adds a K_CALLBACK_STATE bit in user_options and documents it in such a way (as being intended for "callback manager" utilities) that it can't be used recursively or otherwise collide. Fixes #32052 Signed-off-by: Andy Ross <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The p4wq code, to handle the case where a work item was re-submitted, needs to inspect the thread field of the work item after the user callback has returned. But this will race with the case where the user code needs to (in some other context, on some other CPU) re-use that work item (for example by freeing it into the heap, or filling it out again for resubmission).
In the case where the item was not resubmitted, the p4wq code must never touch the memory of a work item after its callback has returned.
(In practice, we just need one bit for "was resubmitted", which we can track per-thread somewhere I guess. Needs a little thought, but shouldn't be difficult.)
The text was updated successfully, but these errors were encountered: