linux-kernel - Re: [PATCH] spin_lock_irqsave() in autofs

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] [day] [month] [year] [list]

Message-ID: <c1ccb74a-d715-41a7-862b-3038323d8091@molgen.mpg.de>
Date: Sun, 17 Aug 2025 20:34:35 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Al Viro <viro@...iv.linux.org.uk>, Ian Kent <raven@...maw.net>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, autofs@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] spin_lock_irqsave() in autofs_write() is bogus

Dear Al,


Am 17.08.25 um 18:36 schrieb Al Viro:
> 	That function should never be (and never is) called with irqs
> disabled - we have an explicit mutex_lock() in there, if nothing else.
> Which makes spin_lock_irqsave() use in there pointless - we do need to
> disable irqs for ->siglock, but that should be spin_lock_irq().
> 
> 	The history is interesting - it goes all way back to 2.1.68pre1,
> and that obviously was a tree-wide work.  Might be interesting to look
> for other places with just-in-case spin_lock_irqsave()...
> 
> Signed-off-by: Al Viro <viro@...iv.linux.org.uk>
> ---
> diff --git a/fs/autofs/waitq.c b/fs/autofs/waitq.c
> index 33dd4660d82f..4dc226e86360 100644
> --- a/fs/autofs/waitq.c
> +++ b/fs/autofs/waitq.c
> @@ -46,7 +46,7 @@ void autofs_catatonic_mode(struct autofs_sb_info *sbi)
>   static int autofs_write(struct autofs_sb_info *sbi,
>   			struct file *file, const void *addr, int bytes)
>   {
> -	unsigned long sigpipe, flags;
> +	unsigned long sigpipe;
>   	const char *data = (const char *)addr;
>   	ssize_t wr = 0;
>   
> @@ -66,10 +66,10 @@ static int autofs_write(struct autofs_sb_info *sbi,
>   	 * SIGPIPE unless it was already supposed to get one
>   	 */
>   	if (wr == -EPIPE && !sigpipe) {
> -		spin_lock_irqsave(&current->sighand->siglock, flags);
> +		spin_lock_irq(&current->sighand->siglock);
>   		sigdelset(&current->pending.signal, SIGPIPE);
>   		recalc_sigpending();
> -		spin_unlock_irqrestore(&current->sighand->siglock, flags);
> +		spin_unlock_irq(&current->sighand->siglock);
>   	}
>   
>   	/* if 'wr' returned 0 (impossible) we assume -EIO (safe) */
> 

Reviewed-by: Paul Menzel <pmenzel@...gen.mpg.de>


Kind regards,

Paul