| From 0c71437dd50dd687c15d8ca80b3b68f10bb21d63 Mon Sep 17 00:00:00 2001 |
| From: Oleksij Rempel <o.rempel@pengutronix.de> |
| Date: Wed, 14 Jul 2021 13:16:02 +0200 |
| Subject: can: j1939: j1939_session_deactivate(): clarify lifetime of session object |
| |
| From: Oleksij Rempel <o.rempel@pengutronix.de> |
| |
| commit 0c71437dd50dd687c15d8ca80b3b68f10bb21d63 upstream. |
| |
| The j1939_session_deactivate() is decrementing the session ref-count and |
| potentially can free() the session. This would cause use-after-free |
| situation. |
| |
| However, the code calling j1939_session_deactivate() does always hold |
| another reference to the session, so that it would not be free()ed in |
| this code path. |
| |
| This patch adds a comment to make this clear and a WARN_ON, to ensure |
| that future changes will not violate this requirement. Further this |
| patch avoids dereferencing the session pointer as a precaution to avoid |
| use-after-free if the session is actually free()ed. |
| |
| Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") |
| Link: https://lore.kernel.org/r/20210714111602.24021-1-o.rempel@pengutronix.de |
| Reported-by: Xiaochen Zou <xzou017@ucr.edu> |
| Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> |
| Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/can/j1939/transport.c | 9 +++++++-- |
| 1 file changed, 7 insertions(+), 2 deletions(-) |
| |
| --- a/net/can/j1939/transport.c |
| +++ b/net/can/j1939/transport.c |
| @@ -1075,11 +1075,16 @@ static bool j1939_session_deactivate_loc |
| |
| static bool j1939_session_deactivate(struct j1939_session *session) |
| { |
| + struct j1939_priv *priv = session->priv; |
| bool active; |
| |
| - j1939_session_list_lock(session->priv); |
| + j1939_session_list_lock(priv); |
| + /* This function should be called with a session ref-count of at |
| + * least 2. |
| + */ |
| + WARN_ON_ONCE(kref_read(&session->kref) < 2); |
| active = j1939_session_deactivate_locked(session); |
| - j1939_session_list_unlock(session->priv); |
| + j1939_session_list_unlock(priv); |
| |
| return active; |
| } |