[Bug] pin_api_attach_irq legacy path publishes hdr and args without IRQ protection #11784

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:

1. Application calls pin_api_attach_irq() to update the pin interrupt handler
2. The function writes legacy_isr->hdr = new_handler
3. Before writing legacy_isr->args = new_args, a pin interrupt fires
4. 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.