passing private flag to SYS_futex in sem_post on x86_64

[Carlos O'Donell]

On Thu, Sep 13, 2012 at 8:49 AM, Petar Jovanovic
<petar.jovanovic@rt-rk.com> wrote:
> Is this correct behaviour? If not, we could add a small change like this:
>
> diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/sem_post.S
> b/nptl/sysdeps/unix/sysv/linux/x86_64/sem_post.S
> index 65e715d..9cb7efa 100644
> --- a/nptl/sysdeps/unix/sysv/linux/x86_64/sem_post.S
> +++ b/nptl/sysdeps/unix/sysv/linux/x86_64/sem_post.S
> @@ -49,8 +49,9 @@ sem_post:
>         je      2f
>
>         movl    $SYS_futex, %eax
> -       movl    $FUTEX_WAKE, %esi
> -       orl     PRIVATE(%rdi), %esi
> +       movl    PRIVATE(%rdi), %esi
> +       andl    $FUTEX_PRIVATE_FLAG, %esi
> +       orl     $FUTEX_WAKE, %esi
>         movl    $1, %edx
>         syscall
>
> This way, similar to other architectures, we isolate private flag from the
> variable private, and then use it to 'or' with FUTEX_WAKE.
>
> This will also get us rid of strange logs in strace such as:
>
> futex(0x7fff07dddf20, 0x55555555 /* FUTEX_??? */, 1) = -1 ENOSYS (Function
> not implemented)
>
> What do you say?

This is not an acceptable change.

If the sem_t structure has been overwritten by the process with invalid
data; fixing the `private' field won't fix the rest of the data in the
structure.

Therefore the additional instructions to isolate the private flag from
the private variable is an incomplete solution that only slows down the
fast path of sem_post for the working cases.

It would seem that your patch would hide the corruption from the user
resulting in what would be a worse situation, the semaphore with a
potentially invalid value and nwaiters is acted upon by the kernel.

Does that clearly explain my position?

Cheers,
Carlos.