[PATCH] thunderbolt: Prevent use-after-free in resume from hibernate

[PATCH] thunderbolt: Prevent use-after-free in resume from hibernate

[Mika Westerberg]

Kenneth noticed that his laptop crashes randomly when resuming from
hibernate if there is device connected and display tunneled. I was able
to reproduce this as well with the following steps:

  1. Boot the system up, nothing connected.
  2. Connect Thunderbolt 4 dock to the host.
  3. Connect monitor to the Thunderbolt 4 dock.
  4. Verify that there is picture on the screen.
  5. Enter hibernate.
  6. Exit hibernate.
  7. Wait for the system to resume.

  Expectation: System resumes just fine, the connected monitor still
               shows screen.
  Actual result: There is crash during resume, screen is blank.

What happens is that during resume from hibernate we tear down any
existing tunnels created by the boot kernel and this ends up calling
tb_dp_dprx_stop() which calls tb_tunnel_put() dropping the reference
count to zero even though we never called tb_dp_dprx_start() for it (we
never do that for discovery). This makes the discovered DP tunnel memory
to be released and any access after that causes use-after-free and
possible crash.

Fix this so that we only stop DPRX flow if it has been started in the
first place.

Reported-by: Kenneth Crudup <kenny@panix.com>
Closes: https://lore.kernel.org/linux-usb/8e175721-806f-45d6-892a-bd3356af80c9@panix.com/
Cc: stable@vger.kernel.org
Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/tunnel.c | 11 ++++++++---
 drivers/thunderbolt/tunnel.h |  2 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 8229a6fbda5a..717b31d78728 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -1009,6 +1009,8 @@ static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
 	 */
 	tb_tunnel_get(tunnel);
 
+	tunnel->dprx_started = true;
+
 	if (tunnel->callback) {
 		tunnel->dprx_timeout = dprx_timeout_to_ktime(dprx_timeout);
 		queue_delayed_work(tunnel->tb->wq, &tunnel->dprx_work, 0);
@@ -1021,9 +1023,12 @@ static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
 
 static void tb_dp_dprx_stop(struct tb_tunnel *tunnel)
 {
-	tunnel->dprx_canceled = true;
-	cancel_delayed_work(&tunnel->dprx_work);
-	tb_tunnel_put(tunnel);
+	if (tunnel->dprx_started) {
+		tunnel->dprx_started = false;
+		tunnel->dprx_canceled = true;
+		cancel_delayed_work(&tunnel->dprx_work);
+		tb_tunnel_put(tunnel);
+	}
 }
 
 static int tb_dp_activate(struct tb_tunnel *tunnel, bool active)
diff --git a/drivers/thunderbolt/tunnel.h b/drivers/thunderbolt/tunnel.h
index 7f6d3a18a41e..8a0a0cb21a89 100644
--- a/drivers/thunderbolt/tunnel.h
+++ b/drivers/thunderbolt/tunnel.h
@@ -63,6 +63,7 @@ enum tb_tunnel_state {
  * @allocated_down: Allocated downstream bandwidth (only for USB3)
  * @bw_mode: DP bandwidth allocation mode registers can be used to
  *	     determine consumed and allocated bandwidth
+ * @dprx_started: DPRX negotiation was started (tb_dp_dprx_start() was called for it)
  * @dprx_canceled: Was DPRX capabilities read poll canceled
  * @dprx_timeout: If set DPRX capabilities read poll work will timeout after this passes
  * @dprx_work: Worker that is scheduled to poll completion of DPRX capabilities read
@@ -100,6 +101,7 @@ struct tb_tunnel {
 	int allocated_up;
 	int allocated_down;
 	bool bw_mode;
+	bool dprx_started;
 	bool dprx_canceled;
 	ktime_t dprx_timeout;
 	struct delayed_work dprx_work;
-- 
2.47.2

Re: [PATCH] thunderbolt: Prevent use-after-free in resume from hibernate

[Yehezkel Bernat]

On Thu, Mar 6, 2025 at 10:41 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Kenneth noticed that his laptop crashes randomly when resuming from
> hibernate if there is device connected and display tunneled. I was able
> to reproduce this as well with the following steps:
>
>   1. Boot the system up, nothing connected.
>   2. Connect Thunderbolt 4 dock to the host.
>   3. Connect monitor to the Thunderbolt 4 dock.
>   4. Verify that there is picture on the screen.
>   5. Enter hibernate.
>   6. Exit hibernate.
>   7. Wait for the system to resume.
>
>   Expectation: System resumes just fine, the connected monitor still
>                shows screen.
>   Actual result: There is crash during resume, screen is blank.
>
> What happens is that during resume from hibernate we tear down any
> existing tunnels created by the boot kernel and this ends up calling
> tb_dp_dprx_stop() which calls tb_tunnel_put() dropping the reference
> count to zero even though we never called tb_dp_dprx_start() for it (we
> never do that for discovery). This makes the discovered DP tunnel memory
> to be released and any access after that causes use-after-free and
> possible crash.
>
> Fix this so that we only stop DPRX flow if it has been started in the
> first place.
>
> Reported-by: Kenneth Crudup <kenny@panix.com>
> Closes: https://lore.kernel.org/linux-usb/8e175721-806f-45d6-892a-bd3356af80c9@panix.com/
> Cc: stable@vger.kernel.org
> Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---

Reviewed-by: Yehezkel Bernat <YehezkelShB@gmail.com>

Re: [PATCH] thunderbolt: Prevent use-after-free in resume from hibernate

[Mika Westerberg]

On Thu, Mar 06, 2025 at 12:29:14PM +0200, Yehezkel Bernat wrote:
> On Thu, Mar 6, 2025 at 10:41 AM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> >
> > Kenneth noticed that his laptop crashes randomly when resuming from
> > hibernate if there is device connected and display tunneled. I was able
> > to reproduce this as well with the following steps:
> >
> >   1. Boot the system up, nothing connected.
> >   2. Connect Thunderbolt 4 dock to the host.
> >   3. Connect monitor to the Thunderbolt 4 dock.
> >   4. Verify that there is picture on the screen.
> >   5. Enter hibernate.
> >   6. Exit hibernate.
> >   7. Wait for the system to resume.
> >
> >   Expectation: System resumes just fine, the connected monitor still
> >                shows screen.
> >   Actual result: There is crash during resume, screen is blank.
> >
> > What happens is that during resume from hibernate we tear down any
> > existing tunnels created by the boot kernel and this ends up calling
> > tb_dp_dprx_stop() which calls tb_tunnel_put() dropping the reference
> > count to zero even though we never called tb_dp_dprx_start() for it (we
> > never do that for discovery). This makes the discovered DP tunnel memory
> > to be released and any access after that causes use-after-free and
> > possible crash.
> >
> > Fix this so that we only stop DPRX flow if it has been started in the
> > first place.
> >
> > Reported-by: Kenneth Crudup <kenny@panix.com>
> > Closes: https://lore.kernel.org/linux-usb/8e175721-806f-45d6-892a-bd3356af80c9@panix.com/
> > Cc: stable@vger.kernel.org
> > Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> 
> Reviewed-by: Yehezkel Bernat <YehezkelShB@gmail.com>

Thanks! Applied to thunderbolt.git/fixes.

Re: [PATCH] thunderbolt: Prevent use-after-free in resume from hibernate

[Greg KH]

On Thu, Mar 06, 2025 at 10:41:45AM +0200, Mika Westerberg wrote:
> Kenneth noticed that his laptop crashes randomly when resuming from
> hibernate if there is device connected and display tunneled. I was able
> to reproduce this as well with the following steps:
> 
>   1. Boot the system up, nothing connected.
>   2. Connect Thunderbolt 4 dock to the host.
>   3. Connect monitor to the Thunderbolt 4 dock.
>   4. Verify that there is picture on the screen.
>   5. Enter hibernate.
>   6. Exit hibernate.
>   7. Wait for the system to resume.
> 
>   Expectation: System resumes just fine, the connected monitor still
>                shows screen.
>   Actual result: There is crash during resume, screen is blank.
> 
> What happens is that during resume from hibernate we tear down any
> existing tunnels created by the boot kernel and this ends up calling
> tb_dp_dprx_stop() which calls tb_tunnel_put() dropping the reference
> count to zero even though we never called tb_dp_dprx_start() for it (we
> never do that for discovery). This makes the discovered DP tunnel memory
> to be released and any access after that causes use-after-free and
> possible crash.
> 
> Fix this so that we only stop DPRX flow if it has been started in the
> first place.
> 
> Reported-by: Kenneth Crudup <kenny@panix.com>
> Closes: https://lore.kernel.org/linux-usb/8e175721-806f-45d6-892a-bd3356af80c9@panix.com/
> Cc: stable@vger.kernel.org
> Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/thunderbolt/tunnel.c | 11 ++++++++---
>  drivers/thunderbolt/tunnel.h |  2 ++
>  2 files changed, 10 insertions(+), 3 deletions(-)

Want me to take this now?  Or are you going to send it on in a later
pull request to me?  Whichever is easier for you is fine with me.

thanks,

greg k-h

Re: [PATCH] thunderbolt: Prevent use-after-free in resume from hibernate

[Mika Westerberg]

On Thu, Mar 06, 2025 at 04:45:22PM +0100, Greg KH wrote:
> On Thu, Mar 06, 2025 at 10:41:45AM +0200, Mika Westerberg wrote:
> > Kenneth noticed that his laptop crashes randomly when resuming from
> > hibernate if there is device connected and display tunneled. I was able
> > to reproduce this as well with the following steps:
> > 
> >   1. Boot the system up, nothing connected.
> >   2. Connect Thunderbolt 4 dock to the host.
> >   3. Connect monitor to the Thunderbolt 4 dock.
> >   4. Verify that there is picture on the screen.
> >   5. Enter hibernate.
> >   6. Exit hibernate.
> >   7. Wait for the system to resume.
> > 
> >   Expectation: System resumes just fine, the connected monitor still
> >                shows screen.
> >   Actual result: There is crash during resume, screen is blank.
> > 
> > What happens is that during resume from hibernate we tear down any
> > existing tunnels created by the boot kernel and this ends up calling
> > tb_dp_dprx_stop() which calls tb_tunnel_put() dropping the reference
> > count to zero even though we never called tb_dp_dprx_start() for it (we
> > never do that for discovery). This makes the discovered DP tunnel memory
> > to be released and any access after that causes use-after-free and
> > possible crash.
> > 
> > Fix this so that we only stop DPRX flow if it has been started in the
> > first place.
> > 
> > Reported-by: Kenneth Crudup <kenny@panix.com>
> > Closes: https://lore.kernel.org/linux-usb/8e175721-806f-45d6-892a-bd3356af80c9@panix.com/
> > Cc: stable@vger.kernel.org
> > Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/thunderbolt/tunnel.c | 11 ++++++++---
> >  drivers/thunderbolt/tunnel.h |  2 ++
> >  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> Want me to take this now?  Or are you going to send it on in a later
> pull request to me?  Whichever is easier for you is fine with me.

I will send it later in a pull request, thanks!