Xiaochen Zou (1):
can: fix a potential UAF access in j1939_session_deactivate(). Both
session and session->priv may be freed in
j1939_session_deactivate_locked(). It leads to potential UAF read
and write in j1939_session_list_unlock(). The free chain is
j1939_session_deactivate_locked()->j1939_session_put()->__j1939_session_release()->j1939_session_destroy().
To fix this bug, I moved j1939_session_put() behind
j1939_session_deactivate_locked(), and guarded it with a check of
active since the session would be freed only if active is true.
net/can/j1939/transport.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
--
2.17.1
>From 9c4733d093e05db22eb89825579c83e020c3c1a6 Mon Sep 17 00:00:00 2001
From: Xiaochen Zou <xzou017@xxxxxxx>
Date: Tue, 13 Jul 2021 13:15:59 -0700
Subject: [PATCH 1/1] can: fix a potential UAF access in
j1939_session_deactivate().
To: greg@xxxxxxxxx
Cc: stable@xxxxxxxxxxxxxxx,netdev@xxxxxxxxxxxxxxx,linux-can@xxxxxxxxxxxxxxx
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.17.1"
This is a multi-part message in MIME format.
Both session and session->priv may be freed in
j1939_session_deactivate_locked(). It leads to potential UAF read and write
in j1939_session_list_unlock(). The free chain is
j1939_session_deactivate_locked()->j1939_session_put()->__j1939_session_release()->j1939_session_destroy().
To fix this bug, I moved j1939_session_put() behind
j1939_session_deactivate_locked(), and guarded it with a check of active
since the session would be freed only if active is true.
Signed-off-by: Xiaochen Zou <xzou017@xxxxxxx>
---
net/can/j1939/transport.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index c3946c355882..c64bd5e8118a 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1067,7 +1067,6 @@ static bool j1939_session_deactivate_locked(struct j1939_session *session)
list_del_init(&session->active_session_list_entry);
session->state = J1939_SESSION_DONE;
- j1939_session_put(session);
}
return active;
@@ -1080,6 +1079,8 @@ static bool j1939_session_deactivate(struct j1939_session *session)
j1939_session_list_lock(session->priv);
active = j1939_session_deactivate_locked(session);
j1939_session_list_unlock(session->priv);
+ if (active)
+ j1939_session_put(session);
return active;
}
@@ -2127,6 +2128,7 @@ void j1939_simple_recv(struct j1939_priv *priv, struct sk_buff *skb)
int j1939_cancel_active_session(struct j1939_priv *priv, struct sock *sk)
{
struct j1939_session *session, *saved;
+ bool active;
netdev_dbg(priv->ndev, "%s, sk: %p\n", __func__, sk);
j1939_session_list_lock(priv);
@@ -2140,7 +2142,9 @@ int j1939_cancel_active_session(struct j1939_priv *priv, struct sock *sk)
j1939_session_put(session);
session->err = ESHUTDOWN;
- j1939_session_deactivate_locked(session);
+ active = j1939_session_deactivate_locked(session);
+ if (active)
+ j1939_session_put(session);
}
}
j1939_session_list_unlock(priv);
