This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: passing private flag to SYS_futex in sem_post on x86_64


I was more bringing the issue to discussion than really advocating the
patch,
and the issue is that we have different behaviour on different platforms
(which
is again not a real issue, if nobody else except me minds that). Other
platforms
will isolate the private flag the way I have just described it - does that
mean
they should not be doing it?

Corruption from the user can pass unnoticed in the current version too, if
the
private value is not overridden and the rest of the sem_t structure is. Some
other functions, such as sem_wait, will also not fail with the corrupted
sem_t.

Regards,
Petar

-----Original Message-----
From: Carlos O'Donell [mailto:carlos@systemhalted.org] 
Sent: Thursday, September 13, 2012 3:32 PM
To: Petar Jovanovic
Cc: libc-alpha@sourceware.org
Subject: Re: passing private flag to SYS_futex in sem_post on x86_64

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]