Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f71219a-5b24-1e6b-c9e7-1d4428df33b9@dereferenced.org>
Date: Thu, 8 Apr 2021 11:40:43 -0600 (MDT)
From: Ariadne Conill <ariadne@...eferenced.org>
To: musl@...ts.openwall.com
Subject: Re: errno and swapcontext in a multithreaded setup

Hello,

On Thu, 8 Apr 2021, Andrey Bugaevskiy wrote:

> Hi,
>
> I'm using makecontext/swapcontext to migrate contexts between threads
> and this sometimes leads to getting incorrect errno values.

This is not supported by libucontext.

> Investigating further I've noticed that __errno_location
> is marked __attribute__((const)).
> This causes optimizers to assume that errno address never changes
> in the scope of the function which is not the case in my scenario.
>
> Namely, this code:
>
> int test(ucontext_t* old_ctx, const ucontext_t* new_ctx) {
>    int err_before = errno;
>    swapcontext(old_ctx, new_ctx);
>    int err_after = errno;
>    return err_before | err_after; // do not optimize out
> }
>
> translates with -O1 to something like this:
>
> 0000000000001109 <test>:
> 1109: endbr64
> 110d: push %r13
> 110f: push %r12
> 1111: push %rbp
> 1112: push %rbx
> 1113: sub $0x8,%rsp
> 1117: mov %rdi,%r12
> 111a: mov %rsi,%r13
> 111d: callq 1030 <__errno_location@plt>
> 1122: mov %rax,%rbx
> 1125: mov (%rax),%ebp
> 1127: mov %r13,%rsi
> 112a: mov %r12,%rdi
> 112d: callq 1020 <swapcontext@plt>
> 1132: mov %ebp,%eax
> 1134: or (%rbx),%eax
> 1136: add $0x8,%rsp
> 113a: pop %rbx
> 113b: pop %rbp
> 113c: pop %r12
> 113e: pop %r13
> 1140: retq
>
> errno location is being stored to a register and then reused.
> However a call to __errno_location after swapcontext is expected to
> return a different address if the context have been swapped back
> into another thread.
> There are a couple of similarly affected functions (pthread_self,
> __h_errno_location).
>
> Removing __attribute__((const)) or changing it
> to __attribute__((pure)) resolves the problem in newly compiled code.

I believe these values use TLS (but not 100% sure, at least in uClibc 
they do).  libucontext is not aware of TLS, and so clobbers the TLS 
register.  You could modify libucontext to add awareness of the TLS 
register, but I don't think we would accept a patch for that.

> Can this change be considered for the future versions of musl?

No, musl is working as designed here.

Ariadne

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.