netdev - Re: Use-after-free access in j1939_session

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAE1SXrv3Ouwt4Y9NEWGi0WO701w1YP1ruMSxraZr4PZTGsUZgg@mail.gmail.com>
Date:   Tue, 13 Jul 2021 00:46:07 -0700
From:   Xiaochen Zou <xzou017@....edu>
To:     Marc Kleine-Budde <mkl@...gutronix.de>
Cc:     Greg KH <greg@...ah.com>, netdev@...r.kernel.org,
        stable@...r.kernel.org, kernel@...gutronix.de,
        linux-can@...r.kernel.org
Subject: Re: Use-after-free access in j1939_session_deactivate

j1939_session_destroy() will free both session and session->priv. It
leads to multiple use-after-free read and write in
j1939_session_deactivate() when session was freed in
j1939_session_deactivate_locked(). 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@....edu>

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index e5f1a56994c6..b6448f29a4bd 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1018,7 +1018,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;
@@ -1031,6 +1030,9 @@ 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;
 }
@@ -2021,6 +2023,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);
@@ -2030,7 +2033,10 @@ int j1939_cancel_active_session(struct
j1939_priv *priv, struct sock *sk)
        if (!sk || sk == session->sk) {
            j1939_session_timers_cancel(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);

On Tue, Jul 13, 2021 at 12:35 AM Marc Kleine-Budde <mkl@...gutronix.de> wrote:
>
> On 7/13/21 9:30 AM, Xiaochen Zou wrote:
> > j1939_session_destroy() will free both session and session->priv. It
> > leads to multiple use-after-free read and write in
> > j1939_session_deactivate() when session was freed in
> > j1939_session_deactivate_locked(). 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.
>
> Please include your Signed-off-by.
> See
> https://elixir.bootlin.com/linux/v5.12/source/Documentation/process/submitting-patches.rst#L356
>
> Marc
>
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   | https://www.pengutronix.de  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |
>


-- 
Xiaochen Zou
PhD Student
Department of Computer Science & Engineering
University of California, Riverside