mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Cc: Theodore Dubois <tblodt@icloud.com>
Subject: Re: [musl] i386 __set_thread_area will crash if the syscall fails
Date: Sun, 30 Aug 2020 22:51:38 -0400	[thread overview]
Message-ID: <20200831025138.GK3265@brightrain.aerifal.cx> (raw)
In-Reply-To: <20200831014145.GI3265@brightrain.aerifal.cx>

[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]

On Sun, Aug 30, 2020 at 09:41:45PM -0400, Rich Felker wrote:
> On Sun, Aug 30, 2020 at 09:07:10PM -0400, Rich Felker wrote:
> > On Sun, Aug 30, 2020 at 05:34:09PM -0700, Theodore Dubois wrote:
> > > Found a (small) bug in this file:
> > > https://git.musl-libc.org/cgit/musl/tree/src/thread/i386/__set_thread_area..s
> > > 
> > > If the syscall fails, the branch on line 20 is taken and %eax will
> > > be a small negative number. Then "mov $123,%al" will make syscall
> > > 0xffffff7b instead of 0x7b, since overwriting %al only overwrites
> > > the low byte of %eax. So the modify_ldt fallback has apparently
> > > never worked.
> > 
> > Thanks! Indeed, systems where the first syscall fails are outside the
> > actually-supported version range (before 2.6) so it's likely that this
> > was never actually tested. Nowadays SECCOMP_FILTER makes it easy to
> > test, so we should actually try to test some of these things. Have you
> > checked if it works adding xor %eax,%eax before the byte mov or
> > changing it to a 32-bit mov?
> 
> OK, I just confirmed it works after this change. I'll push a fix soon
> along with a bunch of other pending commits. Thanks again for the
> report.

The attached should be the equivalent in C, but it's somewhat larger.
Somewhat indifferent on replacing it.

Rich

[-- Attachment #2: __set_thread_area.c --]
[-- Type: text/plain, Size: 657 bytes --]

#define SYSCALL_NO_TLS 1
#include <stdint.h>
#include "syscall.h"

struct user_desc {
	uint32_t entry_number;
	uint32_t base_addr;
	uint32_t limit;
	uint32_t flags;
};

static int entry_number = -1;

int __set_thread_area_2(void *p)
{
	struct user_desc desc = {
		entry_number, (uintptr_t)p, 0xfffff, 0x51
	};
	int r = __syscall(SYS_set_thread_area, &desc);
	if (!r) {
		entry_number = desc.entry_number;
		__asm__ __volatile__ ("mov %0,%%gs" :
			: "r"(3+8*desc.entry_number));
		return 0;
	}
	desc.entry_number = 0;
	r = __syscall(SYS_modify_ldt, 1, &desc, 16);
	if (!r) {
		__asm__ __volatile__ ("mov %0,%%gs" :
			: "r"(7));
		return 1;
	}
	return r;
}

  reply	other threads:[~2020-08-31  2:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-31  0:34 Theodore Dubois
2020-08-31  1:07 ` Rich Felker
2020-08-31  1:41   ` Rich Felker
2020-08-31  2:51     ` Rich Felker [this message]
2020-09-01 18:20       ` Markus Wichmann
2020-08-31  6:55   ` Alexander Monakov
2020-08-31 12:36     ` Rich Felker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200831025138.GK3265@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=musl@lists.openwall.com \
    --cc=tblodt@icloud.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).