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

[Thomas Gleixner]

Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx> writes:

> 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.

This argument is blantantly wrong. It's the justification for bad
programming. Code ignoring error returns is simply buggy.

> 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.

Why is that something the kernel has to care about? The kernel returns
EINVAl as documented and when user space decides to ignore then it goes
to retry for a full timeslice for nothing.

You have to come up with a better argument why we want to send a signal
here.

Along with an argument why SIGBUS is the right thing when a user space
fast path violation results in a SIGSEGV as you stated above.

Plus a patch which documents this change in the futex man page.

> 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);
> }

And this broken code is a kernel problem because?

> 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.

How does that help?

Are you going to stick SIGBUS in _ALL_ syscalls which might be retried
in a loop just because user space fails to evaluate the error code
properly?

You have to come up with a better argument to justify this change.

> This patch sends SIGBUS signal to slay task and break busy-loop.

I'm pretty sure that I asked you to read and follow documentation
before. If I did not:

 git grep 'This patch' Documentation/process/

Thanks,

        tglx