amd64/163710: setjump in userboot.so causes stack corruption

[Kostik Belousov]

The following reply was made to PR amd64/163710; it has been noted by GNATS.

From: Kostik Belousov <kostikbel at gmail.com>
To: Russell Cattelan <cattelan at digitalelves.com>
Cc: freebsd-gnats-submit at freebsd.org
Subject: Re: amd64/163710: setjump in userboot.so  causes stack corruption
Date: Fri, 30 Dec 2011 10:57:08 +0200

 --DbeKRiHxlYa1cCF3
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 Content-Transfer-Encoding: quoted-printable
 
 On Fri, Dec 30, 2011 at 03:16:34AM +0000, Russell Cattelan wrote:
 >=20
 > >Number:         163710
 > >Category:       amd64
 > >Synopsis:       setjump in userboot.so  causes stack corruption
 > >Confidential:   no
 > >Severity:       non-critical
 > >Priority:       low
 > >Responsible:    freebsd-amd64
 > >State:          open
 > >Quarter:       =20
 > >Keywords:      =20
 > >Date-Required:
 > >Class:          sw-bug
 > >Submitter-Id:   current-users
 > >Arrival-Date:   Fri Dec 30 03:20:10 UTC 2011
 > >Closed-Date:
 > >Last-Modified:
 > >Originator:     Russell Cattelan
 > >Release:        FreeBSD 9.0
 > >Organization:
 > Digtal Elves Inc
 > >Environment:
 > FreeBSD fbsd9-it 9.0-PRERELEASE FreeBSD 9.0-PRERELEASE #183: Thu Dec 29 2=
 0:20:10 CST 2011     cattelan at fbsd9-it:/sys/amd64/compile/KLOAD-CAMDEBUG  a=
 md64
 >=20
 > >Description:
 > For some reason the forth interpreter is built and linked as 32bit even=
 =20
 > on amd64.=20
 The native amd64 boot loader is 32bit. Small trampoline is used to
 switch to long mode before the control is passed to the booted kernel.
 
 >=20
 > The specific problem is that the header files used when compiling 32 are
 > the i386 variant which define the jmp_buf as such
 >=20
 > #define	_JBLEN	11		/* Size of the jmp_buf on x86. */
 >=20
 > typedef	struct _jmp_buf { int _jb[_JBLEN + 1]; } jmp_buf[1];
 >=20
 > which results in a struct of 48 bytes
 >=20
 > unfortunately libstand has a specific version of setjmp that wants to sto=
 re
 > 12 - 8 byte values
 >=20
 > ENTRY(_setjmp)
 > 	movq	%rdi,%rax
 > 	movq	0(%rsp),%rdx		/* retval */
 > 	movq	%rdx, 0(%rax)		/* 0; retval */
 > 	movq	%rbx, 8(%rax)		/* 1; rbx */
 > 	movq	%rsp,16(%rax)		/* 2; rsp */
 > 	movq	%rbp,24(%rax)		/* 3; rbp */
 > 	movq	%r12,32(%rax)		/* 4; r12 */
 > 	movq	%r13,40(%rax)		/* 5; r13 */
 > 	movq	%r14,48(%rax)		/* 6; r14 */
 > 	movq	%r15,56(%rax)		/* 7; r15 */
 > 	fnstcw	64(%rax)		/* 8; fpu cw */
 > 	stmxcsr	68(%rax)		/*    and mxcsr */
 > 	xorq	%rax,%rax
 > 	ret
 > END(_setjmp)
 >=20
 > What is happening then in the function ficlExecC=20
 > the saveTib was getting corrupted due to the jmpbuf stack variable
 > getting over written.=20
 > This causes bogus values to be places in the saveTib structure=20
 > which eventually results in a fault.
 
 How do you build ? It seems that userboot build uses its own ficl
 build from sys/boot/userboot/ficl.
 
 >=20
 >=20
 > This fix is to compile the forth interpreter as 64 bit when running on am=
 d64
 >=20
 >=20
 >=20
 >=20
 >=20
 > >How-To-Repeat:
 >=20
 > >Fix:
 > commit ad5840c9e38ba05bdcab3c9efea1c73272bbd9b7
 > Author: Russell Cattelan <cattelan at digitalelves.com>
 > Date:   Thu Dec 29 20:50:57 2011 -0600
 >=20
 >     Change the build of forth interpreter to 64bit.
 >     The main problem is that the jmp_buf structure is
 >     half the size it needs to be 12 * 4 vs 12 * 8
 >     The setjump functions was then causing stack corruption
 >     since the buffer was not large enough.
 >=20
 > diff --git a/sys/boot/ficl/Makefile b/sys/boot/ficl/Makefile
 > index 5cf54c8..75f4349 100644
 > --- a/sys/boot/ficl/Makefile
 > +++ b/sys/boot/ficl/Makefile
 > @@ -8,7 +8,6 @@ SRCS=3D           ${BASE_SRCS} sysdep.c softcore.c
 >  CLEANFILES=3D    softcore.c testmain testmain.o
 >  CFLAGS+=3D       -ffreestanding -g3
 >  .if ${MACHINE_CPUARCH} =3D=3D "i386" || ${MACHINE_CPUARCH} =3D=3D "amd64"
 > -CFLAGS+=3D   -mpreferred-stack-boundary=3D2
 >  CFLAGS+=3D       -mno-mmx -mno-3dnow -mno-sse -mno-sse2 -mno-sse3 -msoft=
 -float
 >  .endif
 >  .if ${MACHINE_CPUARCH} =3D=3D "powerpc" || ${MACHINE_CPUARCH} =3D=3D "ar=
 m"
 > @@ -38,10 +37,6 @@ SOFTWORDS=3D   softcore.fr jhlocal.fr marker.fr freebs=
 d.fr ficllocal.fr \
 >  # Optional OO extension softwords
 >  #SOFTWORDS+=3D   oo.fr classes.fr
 > =20
 > -.if ${MACHINE_CPUARCH} =3D=3D "amd64"
 > -CFLAGS+=3D   -m32 -march=3Di386 -I.
 > -.endif
 > -
 I assume you never did even the buildworld with the change ?
 
 --DbeKRiHxlYa1cCF3
 Content-Type: application/pgp-signature
 Content-Disposition: inline
 
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v2.0.18 (FreeBSD)
 
 iEYEARECAAYFAk79fOQACgkQC3+MBN1Mb4jA1gCghqTNd5abxQBT/Y1eOGWZqgjf
 4S4An2flz+MTNX4KUfdSDBl7rYzQrwUv
 =GUlo
 -----END PGP SIGNATURE-----
 
 --DbeKRiHxlYa1cCF3--