From owner-freebsd-bugs@FreeBSD.ORG Thu Apr 4 16:50:01 2013 Return-Path: Delivered-To: freebsd-bugs@smarthost.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 8124ADBF for ; Thu, 4 Apr 2013 16:50:01 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) by mx1.freebsd.org (Postfix) with ESMTP id 717D57F for ; Thu, 4 Apr 2013 16:50:01 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.6/8.14.6) with ESMTP id r34Go1hC024087 for ; Thu, 4 Apr 2013 16:50:01 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.6/8.14.6/Submit) id r34Go1hr024086; Thu, 4 Apr 2013 16:50:01 GMT (envelope-from gnats) Date: Thu, 4 Apr 2013 16:50:01 GMT Message-Id: <201304041650.r34Go1hr024086@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org Cc: From: Brian Demsky Subject: Re: misc/177624: Swapcontext can get compiled incorrectly X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: Brian Demsky List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Apr 2013 16:50:01 -0000 The following reply was made to PR misc/177624; it has been noted by GNATS. From: Brian Demsky To: Bruce Evans Cc: freebsd-bugs@freebsd.org, freebsd-gnats-submit@freebsd.org Subject: Re: misc/177624: Swapcontext can get compiled incorrectly Date: Thu, 4 Apr 2013 09:43:06 -0700 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 : pop %rbx >>> 0x00007fff901e870c : pop %r14 >>> 0x00007fff901e870e : jmpq 0x7fff90262855 = >>> 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 : push %rbx 0x00007fff90262856 : lea 0x38(%rdi),%rbx 0x00007fff9026285a : cmp 0x30(%rdi),%rbx 0x00007fff9026285e : je 0x7fff90262864 = 0x00007fff90262860 : mov %rbx,0x30(%rdi) 0x00007fff90262864 : mov 0x4(%rdi),%edi 0x00007fff90262867 : callq 0x7fff90262998 = 0x00007fff9026286c : mov %rbx,%rdi 0x00007fff9026286f : pop %rbx 0x00007fff90262870 : 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