net: ethernet: oa_tc6: Protect skb pointer used by two different kernel instances

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 if threaded IRQ sets disable_traffic true after
start_xmit already checked and found that disable_traffic being false,
if they happen to run on different cores.

On fatal error, where disable_traffic is set, transmit function drops the
packet and return NETDEV_TX_OK. Due to this change, skb_linearize call
is moved up to the beginning of the transmit function.

Since skb buffer may be freed from different contexts, dev_kfree_skb_any
is used to free skb buffer now, replacing one of the kfree_skb call.

oa_tc6_exit disables the irq before setting disable_traffic true.

Fixes: b542d13fab ("net: ethernet: oa_tc6: Interrupt is active low, level triggered.")
Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
Link: https://patch.msgid.link/20260824-fix-race-condition-and-crash-v7-1-4323279b18f2@onsemi.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Selvamani Rajagopal 2026-08-24 14:57:58 -07:00 committed by Jakub Kicinski
parent fa5acd038e
commit 5443d9c4f5

View File

@ -693,6 +693,26 @@ static int oa_tc6_enable_data_transfer(struct oa_tc6 *tc6)
return oa_tc6_write_register(tc6, OA_TC6_REG_CONFIG0, value);
}
/* Called when a frame that is meant to be transmitted, is dropped. */
static void oa_tc6_drop_tx_skb(struct oa_tc6 *tc6, struct sk_buff *skb)
{
if (skb) {
tc6->netdev->stats.tx_dropped++;
dev_kfree_skb_any(skb);
}
}
static struct sk_buff *oa_tc6_detach_waiting_tx_skb(struct oa_tc6 *tc6)
{
struct sk_buff *skb;
lockdep_assert_held(&tc6->tx_skb_lock);
skb = tc6->waiting_tx_skb;
tc6->waiting_tx_skb = NULL;
return skb;
}
static void oa_tc6_cleanup_ongoing_rx_skb(struct oa_tc6 *tc6)
{
if (tc6->rx_skb) {
@ -704,26 +724,30 @@ static void oa_tc6_cleanup_ongoing_rx_skb(struct oa_tc6 *tc6)
static void oa_tc6_cleanup_ongoing_tx_skb(struct oa_tc6 *tc6)
{
if (tc6->ongoing_tx_skb) {
tc6->netdev->stats.tx_dropped++;
kfree_skb(tc6->ongoing_tx_skb);
tc6->ongoing_tx_skb = NULL;
}
oa_tc6_drop_tx_skb(tc6, tc6->ongoing_tx_skb);
tc6->ongoing_tx_skb = NULL;
}
static void oa_tc6_cleanup_waiting_tx_skb(struct oa_tc6 *tc6)
{
if (tc6->waiting_tx_skb) {
tc6->netdev->stats.tx_dropped++;
kfree_skb(tc6->waiting_tx_skb);
tc6->waiting_tx_skb = NULL;
}
struct sk_buff *skb;
spin_lock_bh(&tc6->tx_skb_lock);
skb = oa_tc6_detach_waiting_tx_skb(tc6);
spin_unlock_bh(&tc6->tx_skb_lock);
oa_tc6_drop_tx_skb(tc6, skb);
}
static void oa_tc6_free_ongoing_skbs(struct oa_tc6 *tc6)
{
oa_tc6_cleanup_ongoing_tx_skb(tc6);
oa_tc6_cleanup_ongoing_rx_skb(tc6);
}
static void oa_tc6_free_pending_skbs(struct oa_tc6 *tc6)
{
oa_tc6_cleanup_ongoing_tx_skb(tc6);
oa_tc6_cleanup_ongoing_rx_skb(tc6);
oa_tc6_free_ongoing_skbs(tc6);
oa_tc6_cleanup_waiting_tx_skb(tc6);
}
@ -734,9 +758,15 @@ static void oa_tc6_free_pending_skbs(struct oa_tc6 *tc6)
static void oa_tc6_disable_traffic(struct oa_tc6 *tc6)
{
u32 regval = OA_TC6_INT_MASK0_ALL_INTERRUPTS;
struct sk_buff *skb;
spin_lock_bh(&tc6->tx_skb_lock);
tc6->disable_traffic = true;
oa_tc6_free_pending_skbs(tc6);
skb = oa_tc6_detach_waiting_tx_skb(tc6);
spin_unlock_bh(&tc6->tx_skb_lock);
oa_tc6_drop_tx_skb(tc6, skb);
oa_tc6_free_ongoing_skbs(tc6);
oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
oa_tc6_read_register(tc6, OA_TC6_REG_STATUS0, &regval);
oa_tc6_write_register(tc6, OA_TC6_REG_STATUS0, regval);
@ -1177,8 +1207,7 @@ static int oa_tc6_try_spi_transfer(struct oa_tc6 *tc6)
if (ret == -EAGAIN)
continue;
oa_tc6_cleanup_ongoing_tx_skb(tc6);
oa_tc6_cleanup_ongoing_rx_skb(tc6);
oa_tc6_free_ongoing_skbs(tc6);
netdev_err(tc6->netdev, "Device error: %d\n", ret);
return ret;
}
@ -1200,15 +1229,20 @@ static irqreturn_t oa_tc6_macphy_threaded_irq(int irq, void *data)
* no need to attempt spi transfer, once it fails. Pending skbs
* are already freed.
*/
if (!tc6->disable_traffic) {
while (tc6->int_flag ||
(tc6->waiting_tx_skb && tc6->tx_credits)) {
ret = oa_tc6_try_spi_transfer(tc6);
if (ret) {
disable_irq_nosync(tc6->spi->irq);
oa_tc6_disable_traffic(tc6);
break;
}
spin_lock_bh(&tc6->tx_skb_lock);
if (tc6->disable_traffic) {
spin_unlock_bh(&tc6->tx_skb_lock);
return IRQ_HANDLED;
}
spin_unlock_bh(&tc6->tx_skb_lock);
while (tc6->int_flag ||
(tc6->waiting_tx_skb && tc6->tx_credits)) {
ret = oa_tc6_try_spi_transfer(tc6);
if (ret) {
disable_irq_nosync(tc6->spi->irq);
oa_tc6_disable_traffic(tc6);
break;
}
}
@ -1287,23 +1321,30 @@ EXPORT_SYMBOL_GPL(oa_tc6_zero_align_receive_frame_enable);
* @tc6: oa_tc6 struct.
* @skb: socket buffer in which the ethernet frame is stored.
*
* Return: NETDEV_TX_OK if the transmit ethernet frame skb added in the tx_skb_q
* otherwise returns NETDEV_TX_BUSY.
* Return: NETDEV_TX_OK either on successful queueing of the packet for
* transmission, or on packet getting dropped. Packet can be dropped due to
* failure in linearizing the buffer or disable_traffic is set due to
* earlier fatal error. Returns NETDEV_TX_BUSY when there is no room
* to queue the packet.
*/
netdev_tx_t oa_tc6_start_xmit(struct oa_tc6 *tc6, struct sk_buff *skb)
{
if (tc6->disable_traffic || tc6->waiting_tx_skb) {
netif_stop_queue(tc6->netdev);
return NETDEV_TX_BUSY;
}
if (skb_linearize(skb)) {
dev_kfree_skb_any(skb);
tc6->netdev->stats.tx_dropped++;
oa_tc6_drop_tx_skb(tc6, skb);
return NETDEV_TX_OK;
}
spin_lock_bh(&tc6->tx_skb_lock);
if (tc6->waiting_tx_skb) {
netif_stop_queue(tc6->netdev);
spin_unlock_bh(&tc6->tx_skb_lock);
return NETDEV_TX_BUSY;
}
if (tc6->disable_traffic) {
spin_unlock_bh(&tc6->tx_skb_lock);
oa_tc6_drop_tx_skb(tc6, skb);
return NETDEV_TX_OK;
}
tc6->waiting_tx_skb = skb;
spin_unlock_bh(&tc6->tx_skb_lock);
@ -1462,8 +1503,10 @@ EXPORT_SYMBOL_GPL(oa_tc6_init);
*/
void oa_tc6_exit(struct oa_tc6 *tc6)
{
tc6->disable_traffic = true;
disable_irq(tc6->spi->irq);
spin_lock_bh(&tc6->tx_skb_lock);
tc6->disable_traffic = true;
spin_unlock_bh(&tc6->tx_skb_lock);
oa_tc6_phy_exit(tc6);
oa_tc6_free_pending_skbs(tc6);
}