Commit d966635b authored by Oleksij Rempel's avatar Oleksij Rempel Committed by Marc Kleine-Budde

can: j1939: transport: make sure the aborted session will be deactivated only once

j1939_session_cancel() was modifying session->state without protecting
it by locks and without checking actual state of the session.

This patch moves j1939_tp_set_rxtimeout() into j1939_session_cancel()
and adds the missing locking.

Fixes: 9d71dd0c ("can: add support of SAE J1939 protocol")
Signed-off-by: default avatarOleksij Rempel <o.rempel@pengutronix.de>
parent fd81ebfe
...@@ -1042,12 +1042,13 @@ j1939_session_deactivate_activate_next(struct j1939_session *session) ...@@ -1042,12 +1042,13 @@ j1939_session_deactivate_activate_next(struct j1939_session *session)
j1939_sk_queue_activate_next(session); j1939_sk_queue_activate_next(session);
} }
static void j1939_session_cancel(struct j1939_session *session, static void __j1939_session_cancel(struct j1939_session *session,
enum j1939_xtp_abort err) enum j1939_xtp_abort err)
{ {
struct j1939_priv *priv = session->priv; struct j1939_priv *priv = session->priv;
WARN_ON_ONCE(!err); WARN_ON_ONCE(!err);
lockdep_assert_held(&session->priv->active_session_list_lock);
session->err = j1939_xtp_abort_to_errno(priv, err); session->err = j1939_xtp_abort_to_errno(priv, err);
/* do not send aborts on incoming broadcasts */ /* do not send aborts on incoming broadcasts */
...@@ -1062,6 +1063,20 @@ static void j1939_session_cancel(struct j1939_session *session, ...@@ -1062,6 +1063,20 @@ static void j1939_session_cancel(struct j1939_session *session,
j1939_sk_send_loop_abort(session->sk, session->err); j1939_sk_send_loop_abort(session->sk, session->err);
} }
static void j1939_session_cancel(struct j1939_session *session,
enum j1939_xtp_abort err)
{
j1939_session_list_lock(session->priv);
if (session->state >= J1939_SESSION_ACTIVE &&
session->state < J1939_SESSION_WAITING_ABORT) {
j1939_tp_set_rxtimeout(session, J1939_XTP_ABORT_TIMEOUT_MS);
__j1939_session_cancel(session, err);
}
j1939_session_list_unlock(session->priv);
}
static enum hrtimer_restart j1939_tp_txtimer(struct hrtimer *hrtimer) static enum hrtimer_restart j1939_tp_txtimer(struct hrtimer *hrtimer)
{ {
struct j1939_session *session = struct j1939_session *session =
...@@ -1108,8 +1123,6 @@ static enum hrtimer_restart j1939_tp_txtimer(struct hrtimer *hrtimer) ...@@ -1108,8 +1123,6 @@ static enum hrtimer_restart j1939_tp_txtimer(struct hrtimer *hrtimer)
netdev_alert(priv->ndev, "%s: 0x%p: tx aborted with unknown reason: %i\n", netdev_alert(priv->ndev, "%s: 0x%p: tx aborted with unknown reason: %i\n",
__func__, session, ret); __func__, session, ret);
if (session->skcb.addr.type != J1939_SIMPLE) { if (session->skcb.addr.type != J1939_SIMPLE) {
j1939_tp_set_rxtimeout(session,
J1939_XTP_ABORT_TIMEOUT_MS);
j1939_session_cancel(session, J1939_XTP_ABORT_OTHER); j1939_session_cancel(session, J1939_XTP_ABORT_OTHER);
} else { } else {
session->err = ret; session->err = ret;
...@@ -1169,7 +1182,7 @@ static enum hrtimer_restart j1939_tp_rxtimer(struct hrtimer *hrtimer) ...@@ -1169,7 +1182,7 @@ static enum hrtimer_restart j1939_tp_rxtimer(struct hrtimer *hrtimer)
hrtimer_start(&session->rxtimer, hrtimer_start(&session->rxtimer,
ms_to_ktime(J1939_XTP_ABORT_TIMEOUT_MS), ms_to_ktime(J1939_XTP_ABORT_TIMEOUT_MS),
HRTIMER_MODE_REL_SOFT); HRTIMER_MODE_REL_SOFT);
j1939_session_cancel(session, J1939_XTP_ABORT_TIMEOUT); __j1939_session_cancel(session, J1939_XTP_ABORT_TIMEOUT);
} }
j1939_session_list_unlock(session->priv); j1939_session_list_unlock(session->priv);
} }
...@@ -1375,7 +1388,6 @@ j1939_xtp_rx_cts_one(struct j1939_session *session, struct sk_buff *skb) ...@@ -1375,7 +1388,6 @@ j1939_xtp_rx_cts_one(struct j1939_session *session, struct sk_buff *skb)
out_session_cancel: out_session_cancel:
j1939_session_timers_cancel(session); j1939_session_timers_cancel(session);
j1939_tp_set_rxtimeout(session, J1939_XTP_ABORT_TIMEOUT_MS);
j1939_session_cancel(session, err); j1939_session_cancel(session, err);
} }
...@@ -1572,7 +1584,6 @@ static int j1939_xtp_rx_rts_session_active(struct j1939_session *session, ...@@ -1572,7 +1584,6 @@ static int j1939_xtp_rx_rts_session_active(struct j1939_session *session,
/* RTS on active session */ /* RTS on active session */
j1939_session_timers_cancel(session); j1939_session_timers_cancel(session);
j1939_tp_set_rxtimeout(session, J1939_XTP_ABORT_TIMEOUT_MS);
j1939_session_cancel(session, J1939_XTP_ABORT_BUSY); j1939_session_cancel(session, J1939_XTP_ABORT_BUSY);
} }
...@@ -1583,7 +1594,6 @@ static int j1939_xtp_rx_rts_session_active(struct j1939_session *session, ...@@ -1583,7 +1594,6 @@ static int j1939_xtp_rx_rts_session_active(struct j1939_session *session,
session->last_cmd); session->last_cmd);
j1939_session_timers_cancel(session); j1939_session_timers_cancel(session);
j1939_tp_set_rxtimeout(session, J1939_XTP_ABORT_TIMEOUT_MS);
j1939_session_cancel(session, J1939_XTP_ABORT_BUSY); j1939_session_cancel(session, J1939_XTP_ABORT_BUSY);
return -EBUSY; return -EBUSY;
...@@ -1785,7 +1795,6 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session, ...@@ -1785,7 +1795,6 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session,
out_session_cancel: out_session_cancel:
j1939_session_timers_cancel(session); j1939_session_timers_cancel(session);
j1939_tp_set_rxtimeout(session, J1939_XTP_ABORT_TIMEOUT_MS);
j1939_session_cancel(session, J1939_XTP_ABORT_FAULT); j1939_session_cancel(session, J1939_XTP_ABORT_FAULT);
j1939_session_put(session); j1939_session_put(session);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment