mirror of
https://github.com/torvalds/linux.git
synced 2026-05-30 01:53:29 +02:00
batman-adv: tp_meter: fix race condition in send error reporting
batadv_tp_sender_shutdown() previously used two separate variables to track
session state: sending (an atomic flag indicating whether the session was
active) and reason (a plain enum storing the stop reason). This introduced
a race window between the two writes: after sending was cleared to 0,
batadv_tp_send() could observe the stopped state and call
batadv_tp_sender_end() before reason was written, causing the wrong stop
reason to be reported to the caller.
Fix this by consolidating both variables into a single atomic send_result,
which holds 0 while the session is running and the stop reason once it
ends.
Cc: stable@kernel.org
Fixes: 33a3bb4a33 ("batman-adv: throughput meter implementation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
This commit is contained in:
parent
f50487e356
commit
71dce47f07
|
|
@ -413,11 +413,14 @@ static void batadv_tp_sender_cleanup(struct batadv_tp_vars *tp_vars)
|
|||
static void batadv_tp_sender_end(struct batadv_priv *bat_priv,
|
||||
struct batadv_tp_vars *tp_vars)
|
||||
{
|
||||
enum batadv_tp_meter_reason reason;
|
||||
u32 session_cookie;
|
||||
|
||||
reason = atomic_read(&tp_vars->send_result);
|
||||
|
||||
batadv_dbg(BATADV_DBG_TP_METER, bat_priv,
|
||||
"Test towards %pM finished..shutting down (reason=%d)\n",
|
||||
tp_vars->other_end, tp_vars->reason);
|
||||
tp_vars->other_end, reason);
|
||||
|
||||
batadv_dbg(BATADV_DBG_TP_METER, bat_priv,
|
||||
"Last timing stats: SRTT=%ums RTTVAR=%ums RTO=%ums\n",
|
||||
|
|
@ -430,7 +433,7 @@ static void batadv_tp_sender_end(struct batadv_priv *bat_priv,
|
|||
session_cookie = batadv_tp_session_cookie(tp_vars->session,
|
||||
tp_vars->icmp_uid);
|
||||
|
||||
batadv_tp_batctl_notify(tp_vars->reason,
|
||||
batadv_tp_batctl_notify(reason,
|
||||
tp_vars->other_end,
|
||||
bat_priv,
|
||||
tp_vars->start_time,
|
||||
|
|
@ -446,10 +449,18 @@ static void batadv_tp_sender_end(struct batadv_priv *bat_priv,
|
|||
static void batadv_tp_sender_shutdown(struct batadv_tp_vars *tp_vars,
|
||||
enum batadv_tp_meter_reason reason)
|
||||
{
|
||||
if (atomic_xchg(&tp_vars->sending, 0) != 1)
|
||||
return;
|
||||
atomic_cmpxchg(&tp_vars->send_result, 0, reason);
|
||||
}
|
||||
|
||||
tp_vars->reason = reason;
|
||||
/**
|
||||
* batadv_tp_sender_stopped() - check if tp session was stopped with reason
|
||||
* @tp_vars: the private data of the current TP meter session
|
||||
*
|
||||
* Return: whether stop reason was found
|
||||
*/
|
||||
static bool batadv_tp_sender_stopped(struct batadv_tp_vars *tp_vars)
|
||||
{
|
||||
return atomic_read(&tp_vars->send_result) != 0;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -479,7 +490,7 @@ static void batadv_tp_reset_sender_timer(struct batadv_tp_vars *tp_vars)
|
|||
/* most of the time this function is invoked while normal packet
|
||||
* reception...
|
||||
*/
|
||||
if (unlikely(atomic_read(&tp_vars->sending) == 0))
|
||||
if (unlikely(batadv_tp_sender_stopped(tp_vars)))
|
||||
/* timer ref will be dropped in batadv_tp_sender_cleanup */
|
||||
return;
|
||||
|
||||
|
|
@ -499,7 +510,7 @@ static void batadv_tp_sender_timeout(struct timer_list *t)
|
|||
struct batadv_tp_vars *tp_vars = timer_container_of(tp_vars, t, timer);
|
||||
struct batadv_priv *bat_priv = tp_vars->bat_priv;
|
||||
|
||||
if (atomic_read(&tp_vars->sending) == 0)
|
||||
if (batadv_tp_sender_stopped(tp_vars))
|
||||
return;
|
||||
|
||||
/* if the user waited long enough...shutdown the test */
|
||||
|
|
@ -661,7 +672,7 @@ static void batadv_tp_recv_ack(struct batadv_priv *bat_priv,
|
|||
if (unlikely(tp_vars->role != BATADV_TP_SENDER))
|
||||
goto out;
|
||||
|
||||
if (unlikely(atomic_read(&tp_vars->sending) == 0))
|
||||
if (unlikely(batadv_tp_sender_stopped(tp_vars)))
|
||||
goto out;
|
||||
|
||||
/* old ACK? silently drop it.. */
|
||||
|
|
@ -827,21 +838,21 @@ static int batadv_tp_send(void *arg)
|
|||
|
||||
if (unlikely(tp_vars->role != BATADV_TP_SENDER)) {
|
||||
err = BATADV_TP_REASON_DST_UNREACHABLE;
|
||||
tp_vars->reason = err;
|
||||
batadv_tp_sender_shutdown(tp_vars, err);
|
||||
goto out;
|
||||
}
|
||||
|
||||
orig_node = batadv_orig_hash_find(bat_priv, tp_vars->other_end);
|
||||
if (unlikely(!orig_node)) {
|
||||
err = BATADV_TP_REASON_DST_UNREACHABLE;
|
||||
tp_vars->reason = err;
|
||||
batadv_tp_sender_shutdown(tp_vars, err);
|
||||
goto out;
|
||||
}
|
||||
|
||||
primary_if = batadv_primary_if_get_selected(bat_priv);
|
||||
if (unlikely(!primary_if)) {
|
||||
err = BATADV_TP_REASON_DST_UNREACHABLE;
|
||||
tp_vars->reason = err;
|
||||
batadv_tp_sender_shutdown(tp_vars, err);
|
||||
goto out;
|
||||
}
|
||||
|
||||
|
|
@ -860,7 +871,7 @@ static int batadv_tp_send(void *arg)
|
|||
queue_delayed_work(batadv_event_workqueue, &tp_vars->finish_work,
|
||||
msecs_to_jiffies(tp_vars->test_length));
|
||||
|
||||
while (atomic_read(&tp_vars->sending) != 0) {
|
||||
while (!batadv_tp_sender_stopped(tp_vars)) {
|
||||
if (unlikely(!batadv_tp_avail(tp_vars, payload_len))) {
|
||||
batadv_tp_wait_available(tp_vars, payload_len);
|
||||
continue;
|
||||
|
|
@ -883,8 +894,7 @@ static int batadv_tp_send(void *arg)
|
|||
"Meter: %s() cannot send packets (%d)\n",
|
||||
__func__, err);
|
||||
/* ensure nobody else tries to stop the thread now */
|
||||
if (atomic_xchg(&tp_vars->sending, 0) == 1)
|
||||
tp_vars->reason = err;
|
||||
batadv_tp_sender_shutdown(tp_vars, err);
|
||||
break;
|
||||
}
|
||||
|
||||
|
|
@ -1006,7 +1016,7 @@ void batadv_tp_start(struct batadv_priv *bat_priv, const u8 *dst,
|
|||
ether_addr_copy(tp_vars->other_end, dst);
|
||||
kref_init(&tp_vars->refcount);
|
||||
tp_vars->role = BATADV_TP_SENDER;
|
||||
atomic_set(&tp_vars->sending, 1);
|
||||
atomic_set(&tp_vars->send_result, 0);
|
||||
memcpy(tp_vars->session, session_id, sizeof(session_id));
|
||||
tp_vars->icmp_uid = icmp_uid;
|
||||
|
||||
|
|
|
|||
|
|
@ -1320,15 +1320,15 @@ struct batadv_tp_vars {
|
|||
/** @role: receiver/sender modi */
|
||||
enum batadv_tp_meter_role role;
|
||||
|
||||
/** @sending: sending binary semaphore: 1 if sending, 0 is not */
|
||||
atomic_t sending;
|
||||
/**
|
||||
* @send_result: 0 when sending is ongoing and otherwise
|
||||
* enum batadv_tp_meter_reason
|
||||
*/
|
||||
atomic_t send_result;
|
||||
|
||||
/** @receiving: receiving binary semaphore: 1 if receiving, 0 is not */
|
||||
atomic_t receiving;
|
||||
|
||||
/** @reason: reason for a stopped session */
|
||||
enum batadv_tp_meter_reason reason;
|
||||
|
||||
/** @finish_work: work item for the finishing procedure */
|
||||
struct delayed_work finish_work;
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user