[PATCH] tpm: flush timeout work outside buffer_mutex to avoid deadlock

[PATCH] tpm: flush timeout work outside buffer_mutex to avoid deadlock

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

tpm_common_read() flushes priv->timeout_work while holding
priv->buffer_mutex:

	mutex_lock(&priv->buffer_mutex);
	...
	if (!priv->response_length) {
		*off = 0;
		timer_delete_sync(&priv->user_read_timer);
		flush_work(&priv->timeout_work);
	}
	mutex_unlock(&priv->buffer_mutex);

The flushed handler, tpm_timeout_work(), takes the same buffer_mutex at
its start. user_reader_timeout() schedules timeout_work from the
user_read_timer, which is armed while a response is pending. If that
timer fires and timeout_work starts running while a reader holds
buffer_mutex, flush_work() waits for timeout_work to finish while the
reader holds the mutex, and timeout_work blocks acquiring that same
mutex -> deadlock. lockdep would report this as a circular locking
dependency between buffer_mutex and the work.

Before commit 9488585b21be ("tpm: add support for partial reads") the
timer deletion and the work flush ran before buffer_mutex was taken;
that commit moved them into the mutex-held section, introducing the
flush-under-lock cycle.

Flush timeout_work after dropping buffer_mutex. The timer is still
deleted under the lock, so no new timeout work can be queued once the
mutex is released, and the flush no longer runs under the lock the work
needs.

Found by static analysis (a checker for cancel/flush-sync of a work
performed under a lock that the work's handler also takes) and confirmed
by code inspection. Not reproduced at runtime: it requires the
deprecated user-space read timeout to fire while a read holds
buffer_mutex, so the deadlock is established by inspection rather than an
observed lockdep splat.

Fixes: 9488585b21be ("tpm: add support for partial reads")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 drivers/char/tpm/tpm-dev-common.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index 6569212..f9b18ef 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -132,6 +132,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
 {
 	struct file_priv *priv = file->private_data;
 	ssize_t ret_size = 0;
+	bool flush_timeout = false;
 	int rc;
 
 	if (!size)
@@ -164,9 +165,19 @@ out:
 	if (!priv->response_length) {
 		*off = 0;
 		timer_delete_sync(&priv->user_read_timer);
-		flush_work(&priv->timeout_work);
+		flush_timeout = true;
 	}
 	mutex_unlock(&priv->buffer_mutex);
+
+	/*
+	 * tpm_timeout_work() acquires buffer_mutex, so flushing it while that
+	 * mutex is held deadlocks if the timeout work is already running and
+	 * waiting for the lock. Flush after dropping the mutex; the timer is
+	 * deleted above under the lock, so no new timeout work can be queued.
+	 */
+	if (flush_timeout)
+		flush_work(&priv->timeout_work);
+
 	return ret_size;
 }
 
-- 
2.43.0

Re: [PATCH] tpm: flush timeout work outside buffer_mutex to avoid deadlock

[Jarkko Sakkinen]

On Sun, Nov 08, 2026 at 04:58:48PM +0900, hyunjungg wrote:
> From: Hyunjung Ko <hj351016@gmail.com>
> 
> tpm_common_read() flushes priv->timeout_work while holding
> priv->buffer_mutex:
> 
> 	mutex_lock(&priv->buffer_mutex);
> 	...
> 	if (!priv->response_length) {
> 		*off = 0;
> 		timer_delete_sync(&priv->user_read_timer);
> 		flush_work(&priv->timeout_work);
> 	}
> 	mutex_unlock(&priv->buffer_mutex);
> 
> The flushed handler, tpm_timeout_work(), takes the same buffer_mutex at
> its start. user_reader_timeout() schedules timeout_work from the
> user_read_timer, which is armed while a response is pending. If that
> timer fires and timeout_work starts running while a reader holds
> buffer_mutex, flush_work() waits for timeout_work to finish while the
> reader holds the mutex, and timeout_work blocks acquiring that same
> mutex -> deadlock. lockdep would report this as a circular locking
> dependency between buffer_mutex and the work.
> 
> Before commit 9488585b21be ("tpm: add support for partial reads") the
> timer deletion and the work flush ran before buffer_mutex was taken;
> that commit moved them into the mutex-held section, introducing the
> flush-under-lock cycle.
> 
> Flush timeout_work after dropping buffer_mutex. The timer is still
> deleted under the lock, so no new timeout work can be queued once the
> mutex is released, and the flush no longer runs under the lock the work
> needs.
> 
> Found by static analysis (a checker for cancel/flush-sync of a work
> performed under a lock that the work's handler also takes) and confirmed
> by code inspection. Not reproduced at runtime: it requires the
> deprecated user-space read timeout to fire while a read holds
> buffer_mutex, so the deadlock is established by inspection rather than an
> observed lockdep splat.
> 
> Fixes: 9488585b21be ("tpm: add support for partial reads")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
> ---
>  drivers/char/tpm/tpm-dev-common.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
> index 6569212..f9b18ef 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -132,6 +132,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
>  {
>  	struct file_priv *priv = file->private_data;
>  	ssize_t ret_size = 0;
> +	bool flush_timeout = false;

Rename this as flush.

>  	int rc;
>  
>  	if (!size)
> @@ -164,9 +165,19 @@ out:
>  	if (!priv->response_length) {
>  		*off = 0;
>  		timer_delete_sync(&priv->user_read_timer);
> -		flush_work(&priv->timeout_work);
> +		flush_timeout = true;
>  	}
>  	mutex_unlock(&priv->buffer_mutex);
> +
> +	/*
> +	 * tpm_timeout_work() acquires buffer_mutex, so flushing it while that
> +	 * mutex is held deadlocks if the timeout work is already running and
> +	 * waiting for the lock. Flush after dropping the mutex; the timer is
> +	 * deleted above under the lock, so no new timeout work can be queued.
> +	 */
> +	if (flush_timeout)
> +		flush_work(&priv->timeout_work);
> +
>  	return ret_size;
>  }
>  
> -- 
> 2.43.0
> 

Br, Jarkko

Re: [PATCH] tpm: flush timeout work outside buffer_mutex to avoid deadlock

[khj]

Thanks, renamed to 'flush' in v2.

2026년 11월 8일 (일) 오후 9:29, Jarkko Sakkinen <jarkko@kernel.org>님이 작성:
>
> On Sun, Nov 08, 2026 at 04:58:48PM +0900, hyunjungg wrote:
> > From: Hyunjung Ko <hj351016@gmail.com>
> >
> > tpm_common_read() flushes priv->timeout_work while holding
> > priv->buffer_mutex:
> >
> >       mutex_lock(&priv->buffer_mutex);
> >       ...
> >       if (!priv->response_length) {
> >               *off = 0;
> >               timer_delete_sync(&priv->user_read_timer);
> >               flush_work(&priv->timeout_work);
> >       }
> >       mutex_unlock(&priv->buffer_mutex);
> >
> > The flushed handler, tpm_timeout_work(), takes the same buffer_mutex at
> > its start. user_reader_timeout() schedules timeout_work from the
> > user_read_timer, which is armed while a response is pending. If that
> > timer fires and timeout_work starts running while a reader holds
> > buffer_mutex, flush_work() waits for timeout_work to finish while the
> > reader holds the mutex, and timeout_work blocks acquiring that same
> > mutex -> deadlock. lockdep would report this as a circular locking
> > dependency between buffer_mutex and the work.
> >
> > Before commit 9488585b21be ("tpm: add support for partial reads") the
> > timer deletion and the work flush ran before buffer_mutex was taken;
> > that commit moved them into the mutex-held section, introducing the
> > flush-under-lock cycle.
> >
> > Flush timeout_work after dropping buffer_mutex. The timer is still
> > deleted under the lock, so no new timeout work can be queued once the
> > mutex is released, and the flush no longer runs under the lock the work
> > needs.
> >
> > Found by static analysis (a checker for cancel/flush-sync of a work
> > performed under a lock that the work's handler also takes) and confirmed
> > by code inspection. Not reproduced at runtime: it requires the
> > deprecated user-space read timeout to fire while a read holds
> > buffer_mutex, so the deadlock is established by inspection rather than an
> > observed lockdep splat.
> >
> > Fixes: 9488585b21be ("tpm: add support for partial reads")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
> > ---
> >  drivers/char/tpm/tpm-dev-common.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
> > index 6569212..f9b18ef 100644
> > --- a/drivers/char/tpm/tpm-dev-common.c
> > +++ b/drivers/char/tpm/tpm-dev-common.c
> > @@ -132,6 +132,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
> >  {
> >       struct file_priv *priv = file->private_data;
> >       ssize_t ret_size = 0;
> > +     bool flush_timeout = false;
>
> Rename this as flush.
>
> >       int rc;
> >
> >       if (!size)
> > @@ -164,9 +165,19 @@ out:
> >       if (!priv->response_length) {
> >               *off = 0;
> >               timer_delete_sync(&priv->user_read_timer);
> > -             flush_work(&priv->timeout_work);
> > +             flush_timeout = true;
> >       }
> >       mutex_unlock(&priv->buffer_mutex);
> > +
> > +     /*
> > +      * tpm_timeout_work() acquires buffer_mutex, so flushing it while that
> > +      * mutex is held deadlocks if the timeout work is already running and
> > +      * waiting for the lock. Flush after dropping the mutex; the timer is
> > +      * deleted above under the lock, so no new timeout work can be queued.
> > +      */
> > +     if (flush_timeout)
> > +             flush_work(&priv->timeout_work);
> > +
> >       return ret_size;
> >  }
> >
> > --
> > 2.43.0
> >
>
> Br, Jarkko