Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210408174917.GB2546@brightrain.aerifal.cx>
Date: Thu, 8 Apr 2021 13:49:18 -0400
From: Rich Felker <dalias@...c.org>
To: Ariadne Conill <ariadne@...eferenced.org>
Cc: musl@...ts.openwall.com
Subject: Re: errno and swapcontext in a multithreaded setup

On Thu, Apr 08, 2021 at 11:40:43AM -0600, Ariadne Conill wrote:
> 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.

Note that whether the "TLS register" is swapped or preserved is partly
a function of whether it's userspace or kernelspace. For example on
powerpc where it's in a general purpose register, the ucontext
functions could be swapping it (but IMO shouldn't; this would break
even worse by changing the "identity" of the thread when a programmer
commits this kind of UB). On x86 where it's kernel state in segment
descriptors (same descriptor value refers to a different segment in
each thread context) they really can't swap it without a lot of
hackery.

Rich

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.