linux-kernel - Re: [PATCH] futex: send SIGBUS if argument is not aligned on a four-byte boundary

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]

Date:   Fri, 15 May 2020 18:27:07 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
Cc:     linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Darren Hart <dvhart@...radead.org>,
        Maxim Samoylov <max7255@...dex-team.ru>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-api@...r.kernel.org
Subject: Re: [PATCH] futex: send SIGBUS if argument is not aligned on a
 four-byte boundary

On Fri, May 15, 2020 at 06:36:47PM +0300, Konstantin Khlebnikov wrote:
> Userspace implementations of mutexes (including glibc) in some cases
> retries operation without checking error code from syscall futex.
> This is good for performance because most errors are impossible when
> locking code trusts itself.
> 
> Some errors which could came from outer code are handled automatically,
> for example invalid address triggers SIGSEGV on atomic fast path.
> 
> But one case turns into nasty busy-loop: when address is unaligned.
> futex(FUTEX_WAIT) returns EINVAL immediately and loop goes to retry.
> 
> Example which loops inside second call rather than hung peacefully:
> 
> #include <stdlib.h>
> #include <pthread.h>
> 
> int main(int argc, char **argv)
> {
> 	char buf[sizeof(pthread_mutex_t) + 1];
> 	pthread_mutex_t *mutex = (pthread_mutex_t *)(buf + 1);
> 
> 	pthread_mutex_init(mutex, NULL);
> 	pthread_mutex_lock(mutex);
> 	pthread_mutex_lock(mutex);
> }
> 
> It seems there is no practical usage for calling syscall futex for
> unaligned address. This may be only bug in user space. Let's help
> and handle this gracefully without adding extra code on fast path.
> 
> This patch sends SIGBUS signal to slay task and break busy-loop.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
> Reported-by: Maxim Samoylov <max7255@...dex-team.ru>

Seems like a sensible idea to me.

Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>

> ---
>  kernel/futex.c |   13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/futex.c b/kernel/futex.c
> index b59532862bc0..8a6d35fa56bc 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -508,10 +508,21 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, enum futex_a
>  
>  	/*
>  	 * The futex address must be "naturally" aligned.
> +	 * Also send signal to break busy-loop if user-space ignore error.
> +	 * EFAULT case should trigger SIGSEGV at access from user-space.
>  	 */
>  	key->both.offset = address % PAGE_SIZE;
> -	if (unlikely((address % sizeof(u32)) != 0))
> +	if (unlikely((address % sizeof(u32)) != 0)) {
> +		struct kernel_siginfo info;
> +
> +		clear_siginfo(&info);
> +		info.si_signo = SIGBUS;
> +		info.si_code  = BUS_ADRALN;
> +		info.si_addr  = uaddr;
> +		force_sig_info(&info);
> +
>  		return -EINVAL;
> +	}
>  	address -= key->both.offset;
>  
>  	if (unlikely(!access_ok(uaddr, sizeof(u32))))
>