[PATCH net-next] net: ethernet: oa_tc6: fix race condition on ongoing_tx_skb

From: Dheeraj Reddy Jonnalagadda
Date: Thu Dec 19 2024 - 02:00:09 EST


A race condition exists in function oa_tc6_prepare_spi_tx_buf_for_tx_skbs
due to an unsynchronized access to shared variable tc6->ongoing_tx_skb.

The issue arises because the condition (!tc6->ongoing_tx_skb) is checked
outside the critical section. Two or more threads can simultaneously
evaluate this condition as true before acquiring the lock. This results
in both threads entering the critical section and modifying
tc6->ongoing_tx_skb, causing inconsistent state updates or overwriting
each other's changes.

Consider the following scenario. A race window exists in the sequence:

Thread1 Thread2
------------------------ ------------------------
- if ongoing_tx_skb is NULL
- if ongoing_tx_skb is NULL
- spin_lock_bh()
- ongoing_tx_skb = waiting_tx_skb
- waiting_tx_skb = NULL
- spin_unlock_bh()
- spin_lock_bh()
- ongoing_tx_skb = waiting_tx_skb
- waiting_tx_skb = NULL
- spin_unlock_bh()

This leads to lost updates between ongoing_tx_skb and waiting_tx_skb
fields. Moving the NULL check inside the critical section ensures both
the NULL check and the assignment are protected by the same lock,
maintaining atomic check-and-set operations.

Fixes: e592b5110b3e ("net: ethernet: oa_tc6: fix tx skb race condition between reference pointers")
Closes: https://scan7.scan.coverity.com/#/project-view/52337/11354?selectedIssue=1602611
Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@xxxxxxxxx>
---
drivers/net/ethernet/oa_tc6.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
index db200e4ec284..66d55ec9bc88 100644
--- a/drivers/net/ethernet/oa_tc6.c
+++ b/drivers/net/ethernet/oa_tc6.c
@@ -1004,12 +1004,12 @@ static u16 oa_tc6_prepare_spi_tx_buf_for_tx_skbs(struct oa_tc6 *tc6)
*/
for (used_tx_credits = 0; used_tx_credits < tc6->tx_credits;
used_tx_credits++) {
+ spin_lock_bh(&tc6->tx_skb_lock);
if (!tc6->ongoing_tx_skb) {
- spin_lock_bh(&tc6->tx_skb_lock);
tc6->ongoing_tx_skb = tc6->waiting_tx_skb;
tc6->waiting_tx_skb = NULL;
- spin_unlock_bh(&tc6->tx_skb_lock);
}
+ spin_unlock_bh(&tc6->tx_skb_lock);
if (!tc6->ongoing_tx_skb)
break;
oa_tc6_add_tx_skb_to_spi_buf(tc6);
--
2.34.1