Re: [PATCH net v4 1/3] net: ethernet: oa_tc6: Protect skb pointer used by two different kernel instances

[Parthiban Veerasooran]

Hi Selvamani,

On 21/07/26 7:50 am, Selvamani Rajagopal via B4 Relay wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> From: Selvamani Rajagopal <Selvamani.Rajagopal@xxxxxxxxxx>
>
> Threaded IRQ uses waiting_tx_skb. Transmit path also uses
> this pointer without any mutual exclusion protection. As a
> result, it might leak skb buffer, particularly threaded IRQ
> runs in the middle of tranmsmit path, near skb_linearize.
Typo: "tranmsmit" -> "transmit"
> Fixes: b542d13fab0f ("net: ethernet: oa_tc6: Interrupt is active low, level triggered.")
> Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@xxxxxxxxxx>
>
> ---
> changes in v4
>    - No change
> changes in v3
> @@ -1393,7 +1431,9 @@ EXPORT_SYMBOL_GPL(oa_tc6_init);
>    */
>   void oa_tc6_exit(struct oa_tc6 *tc6)
>   {
> +       spin_lock_bh(&tc6->tx_skb_lock);
>          tc6->disable_traffic = true;
I don't know whether this is the right place to provide this comment, 
but FYI: the disable_traffic read/check in the hard-IRQ handler 
(oa_tc6_macphy_isr()) is not protected, while it may be written in 
oa_tc6_exit() under tx_skb_lock.

Of course, spin_lock_bh() cannot be used in hard-IRQ context, so perhaps 
a different synchronization approach is needed?

Best regards,
Parthiban V
> +       spin_unlock_bh(&tc6->tx_skb_lock);
>          disable_irq(tc6->spi->irq);
>          oa_tc6_phy_exit(tc6);
>          oa_tc6_free_pending_skbs(tc6);
>
> --
> 2.43.0