Date: Thu, 4 Apr 2013 09:43:06 -0700 From: Brian Demsky <bdemsky@uci.edu> To: Bruce Evans <brde@optusnet.com.au> Cc: freebsd-bugs@freebsd.org, freebsd-gnats-submit@freebsd.org Subject: Re: misc/177624: Swapcontext can get compiled incorrectly Message-ID: <CF93B187-2005-46EA-B581-2FB10EFDD55E@uci.edu> In-Reply-To: <20130405011027.Y1350@besplex.bde.org> References: <201304040232.r342WFTC020054@red.freebsd.org> <20130404232206.S1025@besplex.bde.org> <20130405011027.Y1350@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Apr 4, 2013, at 8:16 AM, Bruce Evans wrote:
> On Fri, 5 Apr 2013, Bruce Evans wrote:
>=20
>> On Thu, 4 Apr 2013, Brian Demsky wrote:
>>=20
>>>> Description:
>>> Here is the code for swap context:
>>> int
>>> swapcontext(ucontext_t *oucp, const ucontext_t *ucp)
>>> {
>>> int ret;
>>> if ((oucp =3D=3D NULL) || (ucp =3D=3D NULL)) {
>>> errno =3D EINVAL;
>>> return (-1);
>>> }
>>> oucp->uc_flags &=3D ~UCF_SWAPPED;
>>> ret =3D getcontext(oucp);
>>> if ((ret =3D=3D 0) && !(oucp->uc_flags & UCF_SWAPPED)) {
>>> oucp->uc_flags |=3D UCF_SWAPPED;
>>> ret =3D setcontext(ucp);
>>> }
>>> return (ret);
>>> }
>>=20
>>> On the OS X port of libc in Mac OSX 10.7.5, this gets compiled as:
>>=20
>>> ...
>>> 0x00007fff901e870b <swapcontext+89>: pop %rbx
>>> 0x00007fff901e870c <swapcontext+90>: pop %r14
>>> 0x00007fff901e870e <swapcontext+92>: jmpq 0x7fff90262855 =
<setcontext>
>>> The problem is that rbx is callee saved by compiled version of =
swapcontext and then reused before getcontext is called. Getcontext =
then stores the wrong value for rbx and setcontext later restores the =
wrong value for rbx. If the caller had any value in rbx, it has been =
trashed at this point.
>>=20
>> Later you wrote:
>>=20
>>> The analysis is a little wrong about the problem. Ultimately, the =
tail call to set context trashes the copies of bx and r14 on the stack=85.=
>>=20
>> The bug seems to be in setcontext(). It must preserve the =
callee-saved
>> registers, not restore them. This would happen automatically if more
>> were written in C. But setcontext() can't be written entirely in C,
>> since it must save all callee-saved registers including ones not used
>> and therefore not normally saved by any C function that it might be =
in,
>> and possibly also including callee-saved registers for nonstandard or
>> non-C ABIs. In FreeBSD, it is apparently always a syscall.
>=20
> This is more than a little wrong. When setcontext() succeeds, it
> doesn't return here. Then it acts like longjmp() and must restore all
> the callee-saved to whatever they were when getcontext() was called.
> Otherwise, it must not clobber any callee-saved registers (then it
> differs from longjmp(). longjmp() just can't fail).
>=20
> Now I don't see any bug here. If the saved state is returned to, then
> it is as if getcontext() returned, and the intermediately-saved %rbx
> is correct (we will restore the orginal %rbx if we return). If
> setcontext() fails, then it should preserve all callee-saved =
registers.
> In the tail-call case, we have already restored the orginal %rbx and
> the failing setcontext() should preserve that.
>=20
> Bruce
Take at setcontext:=20
(gdb) disassemble setcontext
Dump of assembler code for function setcontext:
0x00007fff90262855 <setcontext+0>: push %rbx
0x00007fff90262856 <setcontext+1>: lea 0x38(%rdi),%rbx
0x00007fff9026285a <setcontext+5>: cmp 0x30(%rdi),%rbx
0x00007fff9026285e <setcontext+9>: je 0x7fff90262864 =
<setcontext+15>
0x00007fff90262860 <setcontext+11>: mov %rbx,0x30(%rdi)
0x00007fff90262864 <setcontext+15>: mov 0x4(%rdi),%edi
0x00007fff90262867 <setcontext+18>: callq 0x7fff90262998 =
<sigsetmask>
0x00007fff9026286c <setcontext+23>: mov %rbx,%rdi
0x00007fff9026286f <setcontext+26>: pop %rbx
0x00007fff90262870 <setcontext+27>: jmpq 0x7fff90262875 =
<_setcontext>
End of assembler dump.
The stack from swapcontext had rbx and r14 popped after getcontext =
stored everything. Now we push rbx and then later call sigsetmask. =
Those two actions guarantee that the memory locations where rbx and r14 =
were on the stack have been overwritten. When we later return to the =
save context, it will start up swapcontext and pop the wrong values off =
of the stack for rbx and r14.
Brian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CF93B187-2005-46EA-B581-2FB10EFDD55E>