RT-Thread Version
v5.3.0 (master branch, commit 54e5164)
Affected area
Device drivers
Hardware/BSP vendor
Not applicable / Other
Architecture
Not applicable / Other
Board and hardware details
This issue affects any board where the GPIO controller does not implement the pin_attach_irq operation, forcing the code to fall back to the legacy ISR handler path.
Develop Toolchain
Other
Describe the bug
In components/drivers/pin/dev_pin_dm.c, when gpio->ops->pin_attach_irq is NULL (legacy fallback path), the function pin_api_attach_irq() writes the handler pair without masking IRQs:
legacy_isr->hdr = hdr;
legacy_isr->args = args;
Meanwhile, pin_pic_handle_isr() reads these fields from ISR context:
if (legacy_isr->hdr)
{
legacy_isr->hdr(legacy_isr->args);
}Steps to reproduce the behavior
Race condition scenario:
- Application calls
pin_api_attach_irq() to update the pin interrupt handler
- The function writes
legacy_isr->hdr = new_handler
- Before writing
legacy_isr->args = new_args, a pin interrupt fires
pin_pic_handle_isr() executes and calls new_handler(old_args) with mismatched arguments
This can lead to:
- Incorrect argument being passed to the new handler
- Potential crashes if the new handler expects a different argument type/structure
- Unpredictable behavior
Expected behavior
The handler function pointer and its argument should be updated atomically. Either both should reflect the old values, or both should reflect the new values. No intermediate state should be visible to the ISR.
Other additional context
Affected code path: This bug only applies to the legacy fallback path when the GPIO controller does not implement the pin_attach_irq operation. Modern controllers that provide this operation are not affected.
Root cause: The two stores (hdr and args) are not atomic. On any architecture, an interrupt can occur between them.
Suggested fix: Protect the critical section with IRQ masking:
rt_base_t level = rt_hw_interrupt_disable();
legacy_isr->hdr = hdr;
legacy_isr->args = args;
rt_hw_interrupt_enable(level);
Or use the pin lock with rt_spin_lock_irqsave if appropriate.
Note: The volatile qualifier does not make the pair of stores atomic against interrupts.
RT-Thread Version
v5.3.0 (master branch, commit 54e5164)
Affected area
Device drivers
Hardware/BSP vendor
Not applicable / Other
Architecture
Not applicable / Other
Board and hardware details
This issue affects any board where the GPIO controller does not implement the pin_attach_irq operation, forcing the code to fall back to the legacy ISR handler path.
Develop Toolchain
Other
Describe the bug
In
components/drivers/pin/dev_pin_dm.c, whengpio->ops->pin_attach_irqis NULL (legacy fallback path), the functionpin_api_attach_irq()writes the handler pair without masking IRQs:Meanwhile,
pin_pic_handle_isr()reads these fields from ISR context:Steps to reproduce the behavior
Race condition scenario:
pin_api_attach_irq()to update the pin interrupt handlerlegacy_isr->hdr = new_handlerlegacy_isr->args = new_args, a pin interrupt firespin_pic_handle_isr()executes and callsnew_handler(old_args)with mismatched argumentsThis can lead to:
Expected behavior
The handler function pointer and its argument should be updated atomically. Either both should reflect the old values, or both should reflect the new values. No intermediate state should be visible to the ISR.
Other additional context
Affected code path: This bug only applies to the legacy fallback path when the GPIO controller does not implement the
pin_attach_irqoperation. Modern controllers that provide this operation are not affected.Root cause: The two stores (
hdrandargs) are not atomic. On any architecture, an interrupt can occur between them.Suggested fix: Protect the critical section with IRQ masking:
Or use the pin lock with
rt_spin_lock_irqsaveif appropriate.Note: The
volatilequalifier does not make the pair of stores atomic against interrupts.