From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/12307 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: a third bug in musl clone() Date: Tue, 2 Jan 2018 14:58:25 -0500 Message-ID: <20180102195825.GN1627@brightrain.aerifal.cx> References: <20171223094545.rmx6xtmucyz5xzap@voyager> <72c68934-4445-c83d-7bbc-004953b2f9e9@bitwagon.com> <20171231154926.GG1627@brightrain.aerifal.cx> <20180101195224.tpkl5g5w66rzwzz3@voyager> <5caf910a-dd98-6836-c70f-6a98cf8a9d22@bitwagon.com> <20180102014915.GJ1627@brightrain.aerifal.cx> <3d795d15-4808-6120-e497-d6c367c14952@bitwagon.com> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1514923013 30563 195.159.176.226 (2 Jan 2018 19:56:53 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 2 Jan 2018 19:56:53 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-12323-gllmg-musl=m.gmane.org@lists.openwall.com Tue Jan 02 20:56:49 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1eWSfs-0006y6-L0 for gllmg-musl@m.gmane.org; Tue, 02 Jan 2018 20:56:36 +0100 Original-Received: (qmail 2027 invoked by uid 550); 2 Jan 2018 19:58:37 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 2006 invoked from network); 2 Jan 2018 19:58:37 -0000 Content-Disposition: inline In-Reply-To: <3d795d15-4808-6120-e497-d6c367c14952@bitwagon.com> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:12307 Archived-At: On Tue, Jan 02, 2018 at 10:24:42AM -0800, John Reiser wrote: > In addition to the bugs in __clone for i386 with %gs and stack alignment > in the new thread, there is a third bug in musl's implementation of clone(). > clone() takes optional arguments that need not be present, > yet musl/src/linux/clone.c fetches them anyway. This can cause SIGSEGV. > > ===== musl/src/linux/clone.c excerpts > int clone(int (*func)(void *), void *stack, int flags, void *arg, ...) > { > [[snip]] > va_start(ap, arg); > ptid = va_arg(ap, pid_t *); > tls = va_arg(ap, void *); > ctid = va_arg(ap, pid_t *); > va_end(ap); > > return __syscall_ret(__clone(func, stack, flags, arg, ptid, tls, ctid)); > } > ===== > The presence of ptid, tls, and ctid is indicated by bits in 'flags': > CLONE_PARENT_SETTID, CLONE_SETTLS, CLONE_CHILD_SETTID/CLONE_CHILD_CLEARTID. > If none of those bits are set, then it could be that none of the variable > arguments are present; therefore none of them should be fetched, and 0 (NULL) > should be passed to __clone() for each of ptid, tls, ctid. > [The meaning is unclear if any omitted argument is followed by an argument > that is flagged as present. Should the implementation call the corresponding > va_arg(), or skip over it?] > > How SIGSEGV can be generated: It is valid for &arg to be the address of > the last word on a hardware page: 0x...ffc on a 32-bit CPU with 4KiB pages, > with the following page unmapped. &func would be 16-byte aligned at 0x...ff0. > Any one of the va_arg() calls would attempt to fetch from the next > page at address 0x...000 or greater, which will generate SIGSEGV. This is undefined behavior and should be corrected, but I disagree with your claimed mechanism of failure. clone() is necessarily called by some other function, which has its own call frame, yielding a worst case of: 0. clone's return address (aligned -4 mod 16) 4. func 8. stack 12. flags 16. arg 20. ?? 24. ?? 28. ?? 32. caller's return address (aligned -4 mod 16) In any case it should be fixed by checking flags. As an aside, I don't think there are any valid (supportable) ways to use the variadic args anyway (at least the tls slot is not usable; maybe the tids are?), so it's possible that we should just remove support for them instead. Code that wants to do really wacky things here needs to be written in 100% asm anyway since calls to C from a pseudo-thread created by manual calls to clone will violate the ABI (invalid or duplicate thread pointer) and if you're writing asm for something special you should just use SYS_clone yourself... Rich