[PATCH] autofs: ignore notification errors from a replaced pipe

[PATCH] autofs: ignore notification errors from a replaced pipe

[tjdqudcks0424]

From: Sung Byeongchan <tjdqudcks0424@naver.com>

autofs_notify_daemon() takes a reference to the current notification
pipe under wq_mutex, then drops the mutex before writing the request. The
write can block while the daemon enters catatonic mode and installs a
replacement pipe.

If the old pipe's reader is then closed, the delayed write returns -EPIPE
and the generic error path enters catatonic mode again. That transition
acts on the current superblock state, closes the replacement pipe, and
releases the new daemon generation's wait queues.

Only enter catatonic mode when the pipe which failed is still the current
pipe. Factor the already-locked transition so the identity check and state
change are atomic with respect to another replacement.

This was reproduced on v7.3-rc5-337-gff47652a4b66c in a local QEMU guest.
An unprivileged UID 65534 lookup filled the old packet pipe before a
normal CATATONIC/SETPIPEFD restart. On the unmodified kernel, the new pipe
received HUP and both a pending victim lookup and a later lookup failed
with ENOENT in two out of two fresh-mount runs.

With this change, the new pipe remained active, received an intact
304-byte request, and both victim lookups completed in two out of two
runs. Normal lookup and restart-without-a-blocked-writer controls passed.
No KASAN, oops, refcount, or lock diagnostic was observed. A source
reproducer and complete local logs are available privately on request.

The demonstrated impact is limited to denial of a shared autofs service
during a legitimate daemon restart. No memory corruption, information leak,
privilege escalation, or code execution primitive was observed.

Fixes: 8d7b48e0bc5fa ("autofs4: add miscellaneous device for ioctls")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Sung Byeongchan <tjdqudcks0424@naver.com>
---
 fs/autofs/waitq.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/fs/autofs/waitq.c b/fs/autofs/waitq.c
index d46241342dfc9..fd78d96e105d 100644
--- a/fs/autofs/waitq.c
+++ b/fs/autofs/waitq.c
@@ -12,15 +12,13 @@
  */
 static autofs_wqt_t autofs_next_wait_queue = 1;
 
-void autofs_catatonic_mode(struct autofs_sb_info *sbi)
+static void autofs_catatonic_mode_locked(struct autofs_sb_info *sbi)
 {
 	struct autofs_wait_queue *wq, *nwq;
 
-	mutex_lock(&sbi->wq_mutex);
-	if (sbi->flags & AUTOFS_SBI_CATATONIC) {
-		mutex_unlock(&sbi->wq_mutex);
+	lockdep_assert_held(&sbi->wq_mutex);
+	if (sbi->flags & AUTOFS_SBI_CATATONIC)
 		return;
-	}
 
 	pr_debug("entering catatonic mode\n");
 
@@ -40,6 +38,21 @@ void autofs_catatonic_mode(struct autofs_sb_info *sbi)
 	fput(sbi->pipe);	/* Close the pipe */
 	sbi->pipe = NULL;
 	sbi->pipefd = -1;
+}
+
+void autofs_catatonic_mode(struct autofs_sb_info *sbi)
+{
+	mutex_lock(&sbi->wq_mutex);
+	autofs_catatonic_mode_locked(sbi);
+	mutex_unlock(&sbi->wq_mutex);
+}
+
+static void autofs_catatonic_mode_for_pipe(struct autofs_sb_info *sbi,
+					   struct file *pipe)
+{
+	mutex_lock(&sbi->wq_mutex);
+	if (sbi->pipe == pipe)
+		autofs_catatonic_mode_locked(sbi);
 	mutex_unlock(&sbi->wq_mutex);
 }
 
@@ -170,7 +183,7 @@ static void autofs_notify_daemon(struct autofs_sb_info *sbi,
 		autofs_wait_release(sbi, wq->wait_queue_token, ret);
 		break;
 	default:
-		autofs_catatonic_mode(sbi);
+		autofs_catatonic_mode_for_pipe(sbi, pipe);
 		break;
 	}
 	fput(pipe);
-- 
2.43.0

Re: [PATCH] autofs: ignore notification errors from a replaced pipe

[Ian Kent]

On 8/10/26 01:35, tjdqudcks0424@naver.com wrote:
> From: Sung Byeongchan <tjdqudcks0424@naver.com>
>
> autofs_notify_daemon() takes a reference to the current notification
> pipe under wq_mutex, then drops the mutex before writing the request. The
> write can block while the daemon enters catatonic mode and installs a
> replacement pipe.
>
> If the old pipe's reader is then closed, the delayed write returns -EPIPE
> and the generic error path enters catatonic mode again. That transition
> acts on the current superblock state, closes the replacement pipe, and
> releases the new daemon generation's wait queues.
>
> Only enter catatonic mode when the pipe which failed is still the current
> pipe. Factor the already-locked transition so the identity check and state
> change are atomic with respect to another replacement.
>
> This was reproduced on v7.3-rc5-337-gff47652a4b66c in a local QEMU guest.
> An unprivileged UID 65534 lookup filled the old packet pipe before a
> normal CATATONIC/SETPIPEFD restart. On the unmodified kernel, the new pipe
> received HUP and both a pending victim lookup and a later lookup failed
> with ENOENT in two out of two fresh-mount runs.
>
> With this change, the new pipe remained active, received an intact
> 304-byte request, and both victim lookups completed in two out of two
> runs. Normal lookup and restart-without-a-blocked-writer controls passed.
> No KASAN, oops, refcount, or lock diagnostic was observed. A source
> reproducer and complete local logs are available privately on request.
>
> The demonstrated impact is limited to denial of a shared autofs service
> during a legitimate daemon restart. No memory corruption, information leak,
> privilege escalation, or code execution primitive was observed.

The description looks sound and a after a quick look over the patch it

looks fine. So I'll add my acked-by and also look more closely at the

change later, mostly, because of the problem I mention below.


This sounds like a problem I have been struggling with for ages and had

stopped working on it because I ended up starting an quite ugly refactor.

Looking at this I think that was misguided.


Acked-by: Ian Kent <raven@themaw.net>


Thanks for this Sung, much appreciated.

Christian, it would be great if you could pick this up as you usually

do, ;)


Thanks

Ian

>
> Fixes: 8d7b48e0bc5fa ("autofs4: add miscellaneous device for ioctls")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Sung Byeongchan <tjdqudcks0424@naver.com>
> ---
>   fs/autofs/waitq.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/fs/autofs/waitq.c b/fs/autofs/waitq.c
> index d46241342dfc9..fd78d96e105d 100644
> --- a/fs/autofs/waitq.c
> +++ b/fs/autofs/waitq.c
> @@ -12,15 +12,13 @@
>    */
>   static autofs_wqt_t autofs_next_wait_queue = 1;
>   
> -void autofs_catatonic_mode(struct autofs_sb_info *sbi)
> +static void autofs_catatonic_mode_locked(struct autofs_sb_info *sbi)
>   {
>   	struct autofs_wait_queue *wq, *nwq;
>   
> -	mutex_lock(&sbi->wq_mutex);
> -	if (sbi->flags & AUTOFS_SBI_CATATONIC) {
> -		mutex_unlock(&sbi->wq_mutex);
> +	lockdep_assert_held(&sbi->wq_mutex);
> +	if (sbi->flags & AUTOFS_SBI_CATATONIC)
>   		return;
> -	}
>   
>   	pr_debug("entering catatonic mode\n");
>   
> @@ -40,6 +38,21 @@ void autofs_catatonic_mode(struct autofs_sb_info *sbi)
>   	fput(sbi->pipe);	/* Close the pipe */
>   	sbi->pipe = NULL;
>   	sbi->pipefd = -1;
> +}
> +
> +void autofs_catatonic_mode(struct autofs_sb_info *sbi)
> +{
> +	mutex_lock(&sbi->wq_mutex);
> +	autofs_catatonic_mode_locked(sbi);
> +	mutex_unlock(&sbi->wq_mutex);
> +}
> +
> +static void autofs_catatonic_mode_for_pipe(struct autofs_sb_info *sbi,
> +					   struct file *pipe)
> +{
> +	mutex_lock(&sbi->wq_mutex);
> +	if (sbi->pipe == pipe)
> +		autofs_catatonic_mode_locked(sbi);
>   	mutex_unlock(&sbi->wq_mutex);
>   }
>   
> @@ -170,7 +183,7 @@ static void autofs_notify_daemon(struct autofs_sb_info *sbi,
>   		autofs_wait_release(sbi, wq->wait_queue_token, ret);
>   		break;
>   	default:
> -		autofs_catatonic_mode(sbi);
> +		autofs_catatonic_mode_for_pipe(sbi, pipe);
>   		break;
>   	}
>   	fput(pipe);