From owner-freebsd-ppc@freebsd.org Sun Jul 2 03:33:52 2017 Return-Path: Delivered-To: freebsd-ppc@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 96B94D9B88D for ; Sun, 2 Jul 2017 03:33:52 +0000 (UTC) (envelope-from markmi@dsl-only.net) Received: from asp.reflexion.net (outbound-mail-210-15.reflexion.net [208.70.210.15]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 517C37D898 for ; Sun, 2 Jul 2017 03:33:51 +0000 (UTC) (envelope-from markmi@dsl-only.net) Received: (qmail 3071 invoked from network); 2 Jul 2017 03:33:50 -0000 Received: from unknown (HELO mail-cs-02.app.dca.reflexion.local) (10.81.19.2) by 0 (rfx-qmail) with SMTP; 2 Jul 2017 03:33:50 -0000 Received: by mail-cs-02.app.dca.reflexion.local (Reflexion email security v8.40.1) with SMTP; Sat, 01 Jul 2017 23:33:50 -0400 (EDT) Received: (qmail 2110 invoked from network); 2 Jul 2017 03:33:50 -0000 Received: from unknown (HELO iron2.pdx.net) (69.64.224.71) by 0 (rfx-qmail) with (AES256-SHA encrypted) SMTP; 2 Jul 2017 03:33:50 -0000 Received: from [192.168.1.114] (c-76-115-7-162.hsd1.or.comcast.net [76.115.7.162]) by iron2.pdx.net (Postfix) with ESMTPSA id A4495EC9366; Sat, 1 Jul 2017 20:33:49 -0700 (PDT) From: Mark Millard Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: Re: head -r320521 (e.g.): another powerpc64 problem: programs using fgets crash trying to store address over code instead of into __cleanup_info__ Date: Sat, 1 Jul 2017 20:33:49 -0700 References: To: FreeBSD PowerPC ML , FreeBSD Current , freebsd-hackers@freebsd.org In-Reply-To: Message-Id: <87FABF66-3023-47E4-8F8F-6E76B7B84046@dsl-only.net> X-Mailer: Apple Mail (2.3273) X-BeenThere: freebsd-ppc@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Porting FreeBSD to the PowerPC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Jul 2017 03:33:52 -0000 [I've now got a route to get information from the old PowerMac so-called "Quad Core" despite sshd being broken. So I add gdb output material as evidence to go with the more source code level analysis from before.] On 2017-Jul-1, at 7:42 PM, Mark Millard wrote: > [Note: this is from a amd64 -> powerpc64 cross-build based > on system clang 4 instead of gcc 4.2.1. I'm building a > gcc 4.2.1 based system currently so that I can test > a more standard configuration. But I'm one of the ones > that experiments with finding things to report for > clang targeting powerpc64 and powerpc.] >=20 > powerpc64 is having programs crash with an attempt > to store addresses over code instead of into > __cleanup_info__ when fgets is used. ntpd is an > example. As is sshd (although I've looked at > its details less). >=20 > Building up the context for this claim. . . >=20 > public declaration: >=20 > struct _pthread_cleanup_info { > __uintptr_t pthread_cleanup_pad[8]; > }; >=20 > private declaration: >=20 > struct pthread_cleanup { > struct pthread_cleanup *prev; > void (*routine)(void *); > void *routine_arg; > int onheap; > }; >=20 > ntpd and sshd die with segmentation faults in: >=20 > void > __pthread_cleanup_push_imp(void (*routine)(void *), void *arg, > struct _pthread_cleanup_info *info) > { > struct pthread *curthread =3D _get_curthread(); > struct pthread_cleanup *newbuf; >=20 > newbuf =3D (void *)info; > newbuf->routine =3D routine; > newbuf->routine_arg =3D arg; > newbuf->onheap =3D 0; > newbuf->prev =3D curthread->cleanup; > curthread->cleanup =3D newbuf; > } >=20 > at the statement: newbuf->routine =3D routine; >=20 > But it turns out that the bt is like: >=20 > __pthread_cleanup_push_imp(routine=3D0x507b1248 = <__stdio_cancel_cleanup>, arg=3D0x0, info=3D0x509eaaf4) > __pthread_cleanup_push_imp_int(p0=3D0x507b1248,p1=3D0x0) > fgets (buf=3D0x51415000 "", n=3D511, fp=3D0x507d4c40) > . . . >=20 > Note the 2 arguments to __pthread_cleanup_push_imp_int when called > from fgets but the 3 arguemnts to __pthread_cleanup_push_imp . . . >=20 > fgets uses FLOCK_CANCELSAFE(fp) : >=20 > #define FLOCKFILE_CANCELSAFE(fp) = \ > { = \ > struct _pthread_cleanup_info __cleanup_info__; = \ > if (__isthreaded) { = \ > _FLOCKFILE(fp); = \ > ___pthread_cleanup_push_imp( = \ > __stdio_cancel_cleanup, (fp), = \ > &__cleanup_info__); = \ > } else { = \ > ___pthread_cleanup_push_imp( = \ > __stdio_cancel_cleanup, NULL, = \ > &__cleanup_info__); = \ > } = \ > { > #define FUNLOCKFILE_CANCELSAFE() = \ > (void)0; = \ > } = \ > ___pthread_cleanup_pop_imp(1); = \ > } >=20 > where here the NULL case is in use. 3 arguments. >=20 > But: >=20 > STUB_FUNC2(__pthread_cleanup_push_imp, PJT_CLEANUP_PUSH_IMP, void, = void*, void *); >=20 > which is use of: >=20 > #define STUB_FUNC2(name, idx, ret, p0_type, p1_type) \ > static ret FUNC_EXP(name)(p0_type, p1_type) __used; \ > static ret FUNC_INT(name)(p0_type, p1_type) __used; \ > WEAK_REF(FUNC_EXP(name), name); \ > WEAK_REF(FUNC_INT(name), __CONCAT(_, name)); \ > typedef ret (*FUNC_TYPE(name))(p0_type, p1_type); \ > static ret FUNC_EXP(name)(p0_type p0, p1_type p1) \ > { \ > FUNC_TYPE(name) func; \ > func =3D (FUNC_TYPE(name))__thr_jtable[idx][0]; \ > return (func(p0, p1)); \ > } \ > static ret FUNC_INT(name)(p0_type p0, p1_type p1) \ > { \ > FUNC_TYPE(name) func; \ > func =3D (FUNC_TYPE(name))__thr_jtable[idx][1]; \ > return (func(p0, p1)); \ > } >=20 > so only 2 arguments to func (i.e., __pthread_cleanup_push_imp > here). >=20 > Compared to: >=20 > ___pthread_cleanup_push_imp( = \ > __stdio_cancel_cleanup, NULL, = \ > &__cleanup_info__); = \ >=20 > As a result junk is used instead of &__cleanup_info__. > On powerpc64 it happens to be the address of > ___pthread_cleanup_push_imp that happens to be > in r5 (normally the third argument) at the time. >=20 > So: >=20 > newbuf->routine =3D routine; >=20 > tries to replace the first instruction of > ___pthread_cleanup_push_imp . >=20 > As far as I can tell what should be used is: >=20 > #define STUB_FUNC3(name, idx, ret, p0_type, p1_type, p2_type) \ > static ret FUNC_EXP(name)(p0_type, p1_type, p2_type) __used; \ > static ret FUNC_INT(name)(p0_type, p1_type, p2_type) __used; \ > WEAK_REF(FUNC_EXP(name), name); \ > WEAK_REF(FUNC_INT(name), __CONCAT(_, name)); \ > typedef ret (*FUNC_TYPE(name))(p0_type, p1_type, p2_type); \ > static ret FUNC_EXP(name)(p0_type p0, p1_type p1, p2_type p2) \ > { \ > FUNC_TYPE(name) func; \ > func =3D (FUNC_TYPE(name))__thr_jtable[idx][0]; \ > return (func(p0, p1, p2)); \ > } \ > static ret FUNC_INT(name)(p0_type p0, p1_type p1, p2_type p2) \ > { \ > FUNC_TYPE(name) func; \ > func =3D (FUNC_TYPE(name))__thr_jtable[idx][1]; \ > return (func(p0, p1, p2)); \ > } >=20 > with the p2_type being: struct _pthread_cleanup_info * >=20 > but I'm not expert in this code. The backtrace for ntpd : Loaded symbols for /libexec/ld-elf.so.1 #0 __pthread_cleanup_push_imp (routine=3D0x50649248 = <__stdio_cancel_cleanup>, arg=3D0x0, info=3D0x50403af4) at = /usr/src/lib/libthr/thread/thr_clean.c:57 57 newbuf->routine =3D routine; (gdb) bt #0 __pthread_cleanup_push_imp (routine=3D0x50649248 = <__stdio_cancel_cleanup>, arg=3D0x0, info=3D0x50403af4) at = /usr/src/lib/libthr/thread/thr_clean.c:57 #1 0x0000000050603cbc in __pthread_cleanup_push_imp_int (p0=3D0x50649248,= p1=3D0x0) at /usr/src/lib/libc/gen/_pthread_stubs.c:283 #2 0x00000000505850a0 in vfprintf_l (fp=3D0x5066cc40, locale=3D, fmt0=3D0x50618523 "<%d>", ap=3D0xffffffffffffc150 "") at = printfcommon.h:75 #3 0x000000005058b034 in fprintf (fp=3D0x50649248, fmt=3D0x50403af4 = "`") at /usr/src/lib/libc/stdio/fprintf.c:55 #4 0x00000000505d22e0 in vsyslog (pri=3D, = fmt=3D0x50618523 "<%d>", ap=3D0xffffffffffffc150 "") at = /usr/src/lib/libc/gen/syslog.c:171 #5 0x00000000505d2138 in syslog (pri=3D1348768328, fmt=3D0x0) at = /usr/src/lib/libc/gen/syslog.c:128 #6 0x0000000010095eb8 in 00000016.plt_call.OBJ_sn2nid () #7 0x00000000100965e0 in 00000016.plt_call.OBJ_sn2nid () #8 0x000000001004624c in ntpdmain (argc=3D0, argv=3D) at /usr/src/contrib/ntp/ntpd/ntpd.c:602 #9 0x0000000010046020 in main (argc=3D1348768328, argv=3D0x0) at = /usr/src/contrib/ntp/ntpd/ntpd.c:394 #10 0x0000000010008b5c in 00000016.plt_call.OBJ_sn2nid () #11 0x00000000500e2b70 in .text () at = /usr/src/libexec/rtld-elf/powerpc64/rtld_start.S:104 Note the expected use of r5 in the below. 0x50403af4 <__pthread_cleanup_push_imp>: nop 0x50403af8 <__pthread_cleanup_push_imp+4>: ld r6,18288(r2) 0x50403afc <__pthread_cleanup_push_imp+8>: cmpldi r6,0 0x50403b00 <__pthread_cleanup_push_imp+12>: beq- 0x50403b10 = <__pthread_cleanup_push_imp+28> 0x50403b04 <__pthread_cleanup_push_imp+16>: mr r6,r13 0x50403b08 <__pthread_cleanup_push_imp+20>: ld r6,-28680(r6) 0x50403b0c <__pthread_cleanup_push_imp+24>: b 0x50403b14 = <__pthread_cleanup_push_imp+32> 0x50403b10 <__pthread_cleanup_push_imp+28>: li r6,0 0x50403b14 <__pthread_cleanup_push_imp+32>: std r3,8(r5) 0x50403b18 <__pthread_cleanup_push_imp+36>: li r3,0 0x50403b1c <__pthread_cleanup_push_imp+40>: std r4,16(r5) 0x50403b20 <__pthread_cleanup_push_imp+44>: stw r3,24(r5) 0x50403b24 <__pthread_cleanup_push_imp+48>: ld r3,552(r6) 0x50403b28 <__pthread_cleanup_push_imp+52>: std r3,0(r5) 0x50403b2c <__pthread_cleanup_push_imp+56>: std r5,552(r6) 0x50403b30 <__pthread_cleanup_push_imp+60>: blr 0x50403b34 <__pthread_cleanup_push_imp+64>: .long 0x0 0x50403b38 <__pthread_cleanup_push_imp+68>: .long 0x0 0x50403b3c <__pthread_cleanup_push_imp+72>: .long 0x0 But note the use of r5 in the below: r5=3D=3D0x50403af4 results, replacing any potential r5 from the call to __pthread_cleanup_push_imp_int . Also r5's value is replaced without accessing or recording the value it the __pthread_cleanup_push_imp_int's start. r5 0x50403af4 1346386676 0x50603c80 <__pthread_cleanup_push_imp_int>: mflr r0 0x50603c84 <__pthread_cleanup_push_imp_int+4>: std r31,-8(r1) 0x50603c88 <__pthread_cleanup_push_imp_int+8>: std r0,16(r1) 0x50603c8c <__pthread_cleanup_push_imp_int+12>: stdu r1,-128(r1) 0x50603c90 <__pthread_cleanup_push_imp_int+16>: mr r31,r1 0x50603c94 <__pthread_cleanup_push_imp_int+20>: nop 0x50603c98 <__pthread_cleanup_push_imp_int+24>: std r2,40(r1) 0x50603c9c <__pthread_cleanup_push_imp_int+28>: ld r5,-22408(r2) 0x50603ca0 <__pthread_cleanup_push_imp_int+32>: ld r5,968(r5) 0x50603ca4 <__pthread_cleanup_push_imp_int+36>: ld r6,8(r5) 0x50603ca8 <__pthread_cleanup_push_imp_int+40>: ld r11,16(r5) 0x50603cac <__pthread_cleanup_push_imp_int+44>: ld r5,0(r5) 0x50603cb0 <__pthread_cleanup_push_imp_int+48>: mr r2,r6 0x50603cb4 <__pthread_cleanup_push_imp_int+52>: mtctr r5 0x50603cb8 <__pthread_cleanup_push_imp_int+56>: bctrl 0x50603cbc <__pthread_cleanup_push_imp_int+60>: ld r2,40(r1) 0x50603cc0 <__pthread_cleanup_push_imp_int+64>: addi r1,r1,128 0x50603cc4 <__pthread_cleanup_push_imp_int+68>: ld r0,16(r1) 0x50603cc8 <__pthread_cleanup_push_imp_int+72>: ld r31,-8(r1) 0x50603ccc <__pthread_cleanup_push_imp_int+76>: mtlr r0 0x50603cd0 <__pthread_cleanup_push_imp_int+80>: blr 0x50603cd4 <__pthread_cleanup_push_imp_int+84>: .long 0x0 0x50603cd8 <__pthread_cleanup_push_imp_int+88>: .long 0x0 0x50603cdc <__pthread_cleanup_push_imp_int+92>: .long 0x0 As the above code is set up for passing 2 arguments to __pthread_cleanup_push_imp (in r3 and r4) the above code uses r5 as a scratch register to hold the computed address of __pthread_cleanup_push_imp. This is not what __pthread_cleanup_push_imp is designed for in its use of r5. But in the below r5 was expected to be passed in to __pthread_cleanup_push_imp_int even though __pthread_cleanup_push_imp_int ignores the r5 value that is supplied. 0x505da750 : mflr r0 0x505da754 : std r31,-8(r1) 0x505da758 : std r0,16(r1) 0x505da75c : stdu r1,-256(r1) 0x505da760 : mr r31,r1 . . . 0x505da7b8 : bl 0x5048a9a0 = <00000017.plt_call._flockfile> 0x505da7bc : ld r2,40(r1) 0x505da7c0 : nop 0x505da7c4 : addi r5,r31,112 0x505da7c8 : mr r4,r29 0x505da7cc : b 0x505da7dc 0x505da7d0 : nop 0x505da7d4 : addi r5,r31,112 0x505da7d8 : li r4,0 0x505da7dc : ld r3,-23664(r2) 0x505da7e0 : bl 0x50603c80 = <__pthread_cleanup_push_imp_int> 0x505da7e4 : nop . . . Conclusion: __pthread_cleanup_push_imp_int should expect and handle 3 arguments and propagate all 3 to __pthread_cleanup_push_imp, not using r5 for the address of __pthread_cleanup_push_imp . I'll only give the backtrace for sshd. Also I've removed the garbage text shown for appname=3D in #8 and just used ". . .". Note that #0 through #2 are very similar to the above for ntpd: same problem for sshd. #0 __pthread_cleanup_push_imp (routine=3D0x507b1248 = <__stdio_cancel_cleanup>, arg=3D0x0, info=3D0x509eaaf4) at = /usr/src/lib/libthr/thread/thr_clean.c:57 #1 0x000000005076bcbc in __pthread_cleanup_push_imp_int (p0=3D0x507b1248,= p1=3D0x0) at /usr/src/lib/libc/gen/_pthread_stubs.c:283 #2 0x00000000507427e4 in fgets (buf=3D0x51415000 "", n=3D511, = fp=3D0x507d4c40) from /lib/libc.so.7 #3 0x00000000504e5268 in file_gets (bp=3D, = buf=3D0x507d4c40 "", size=3D0) at = /usr/src/crypto/openssl/crypto/bio/bss_file.c:461 #4 0x00000000504f0678 in BIO_gets (b=3D, = in=3D0x51415000 "", inl=3D) at = /usr/src/crypto/openssl/crypto/bio/bio_lib.c:303 #5 0x000000005047eaa8 in def_load_bio (conf=3D0x51415000, = in=3D0x5140e000, line=3D0x507d4c40) at = /usr/src/crypto/openssl/crypto/conf/conf_def.c:260 #6 0x000000005047f5b4 in def_load (conf=3D, = name=3D, line=3D0x51415000) at = /usr/src/crypto/openssl/crypto/conf/conf_def.c:207 #7 0x000000005047e518 in NCONF_load (conf=3D0x507b1248, file=3D0x0, = eline=3D0x509eaaf4) at = /usr/src/crypto/openssl/crypto/conf/conf_lib.c:265 #8 0x000000005041e97c in CONF_modules_load_file (filename=3D, appname=3D0x507c3da0 ". . .",=20 flags=3D1363206144) at = /usr/src/crypto/openssl/crypto/conf/conf_mod.c:179 #9 0x00000000503ce39c in OPENSSL_config (config_name=3D0x51415000 "") = at /usr/src/crypto/openssl/crypto/conf/conf_sap.c:90 #10 0x00000000500e6200 in Fssh_ssh_OpenSSL_add_all_algorithms () at = /usr/src/crypto/openssh/openbsd-compat/openssl-compat.c:78 #11 0x000000001000e0e0 in main (ac=3D0, av=3D) at = /usr/src/crypto/openssh/sshd.c:1553 #12 0x000000001000d4bc in 00000016.plt_call.Fssh_scan_scaled () #13 0x0000000050057b70 in .text () at = /usr/src/libexec/rtld-elf/powerpc64/rtld_start.S:104 =3D=3D=3D Mark Millard markmi at dsl-only.net