Subject: [PATCH 00/91] pending uml patches

My apologies for mailbomb from hell. *All* this stuff is available in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/um-header.git/ #master, but since uml folks had been stuck with mail and patch for a long time...

Anyway, most of the stuff in this pile is merging, cleaning and mutilating subarchitecture-related code in arch/um. By the end of it we have x86 bits largely merged between 32bit and 64bit variants and taken to arch/x86/um; headers seriously cleaned up and mostly free of x86-isms now (not completely - we still have page size dependencies in there).

Beginning of the series are pure build and driver fixes; those should go to Linus before 3.1-final, IMO.

As far as I know, there's no regressions introduced by that pile; testing and comments would be, of course, welcome.

Al,

Am 18.08.2011 20:58, schrieb Al Viro:

Have you touched your patches since yesterday?
I've already pulled and uploaded them to my shiny new git repo at:
git://git.kernel.org/pub/scm/linux/kernel/git/rw/linux-um.git unstable

Due to the current mirroring problems with git.kernel.org I have not sent made it public. Sorry for that, I screwed it. :-(

Okay.

There was a small build regression, I've already fixed it!

Thanks,
//richard

Reordered and added missing S-o-b on a couple, split one commit.

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Umm... One comment after looking at your tree: you probably want to rebase for-3.2 on top of fixes (and presumably feed it to sfr for inclusion into linux-next).

And for pity sake, do *not* merge from Linus every day; that's one sure way to get yourself flamed into crisp. Just trying to figure out what's in your tree is a _hard_ exercise. git cherry between Linus' tree and e.g. #fixes in yours gives a long list of commits, most of them _probably_ duplicates of the stuff in mainline. What are bnx2 patches doing in there, for example?

I've tried to figure out what's going on in there; AFAICS, your #fixes is mainline plus
Al Viro (6):
um: fix oopsable race in line_close()
um: winch_interrupt() can happen inside of free_winch()
um: fix free_winch() mess
um: PTRACE_[GS]ETFPXREGS had been wired on the wrong subarch
um: fix strrchr problems
um: clean arch_ptrace() up a bit

Ingo van Lil (1):
um: Save FPU registers between task switches

Jonathan Neusch<C3><A4>fer (3):
UserModeLinux-HOWTO.txt: fix a typo
um: drivers/xterm.c: fix a file descriptor leak UserModeLinux-HOWTO.txt: remove ^H characters

Thadeu Lima de Souza Cascardo (1):
um: disable CMPXCHG_DOUBLE as it breaks UML build

I've cherry-picked those on top of the same branchpoint; see
#cleaned-fixes in um-headers.git. AFAICS, that's the same contents as your #fixes, with clean history. Diff against your #fixes consists of

-       .irq_set_type = pmic_irq_type, <<<<<<< HEAD
-       .irq_bus_lock           = pmic_irq_buslock,
+       .irq_set_type           = pmic_irq_type,
+       .irq_bus_lock           = pmic_bus_lock,

in drivers/platform/x86/intel_pmic_gpio.c, which is an obvious mismerge (AFAICS, on May 29).

IME the sane policy is to keep for-linus, pulling into it when Linus pulls from you. At that point it's a fast-forward and all previous history is not cluttering the things up anymore. for-next I rebase and reorder at will, TBH, but generally I start it at the current tip of for-linus.

Beyond what you've got in #for-3.2 I have a couple of commits, but that can wait until the history is sorted out. As it is, I 100% guarantee that pull request on your #fixes as it is will result in pyrotechnical effects from hell (OK, from Linus, actually, but in this case there won't be any real difference).

Am 19.08.2011 06:31, schrieb Al Viro:

Please slow down a bit. :-)
All these branches are just for testing purposes.
That's why I have not announced them nor sent a pull request to Linus.

Anyway, thanks for the hints!

Thanks,
//richard

np... FWIW, there's a really ugly bug present in mainline as well as in mainline + these patches and I'd welcome any help in figuring out what's going on.

1) USER_OBJS do not see CONFIG_..., so os-Linux/main.c doesn't see CONFIG_ARCH_REUSE_HOST_VSYSCALL_AREA. As the result, uml/i386 doesn't notice that host vdso is there. That one is easy to fix:
-obj-$(CONFIG_ARCH_REUSE_HOST_VSYSCALL_AREA) += elf_aux.o +ifeq ($(CONFIG_ARCH_REUSE_HOST_VSYSCALL_AREA),y)
+obj-y += elf_aux.o
+CFLAGS_main.o += -DCONFIG_ARCH_REUSE_HOST_VSYSCALL_AREA +endif
in arch/um/os-Linux/Makefile takes care of that. Unfortunately, it also exposes a bug in fixrange_init():

2) fixrange_init() gets called with start (and end) not multiple of PMD_SIZE; moreover, end is very close to the ~0UL - closer than by PMD_SIZE. Bad things start happening to the loops in there. Again, easy to fix:

index 8137ccc..39ee674 100644
@@ -119,19 +119,22 @@ static void __init fixrange_init(unsigned long start, unsigned long end,
 	int i, j;
 	unsigned long vaddr;
 
-	vaddr = start;
+	vaddr = start & PMD_MASK;
 	i = pgd_index(vaddr);
 	j = pmd_index(vaddr);
 	pgd = pgd_base + i;
+	start >>= PMD_SHIFT;
+	end = (end - 1) >> PMD_SHIFT;
 
-	for ( ; (i < PTRS_PER_PGD) && (vaddr < end); pgd++, i++) {
+	for ( ; (i < PTRS_PER_PGD) && start <= end; pgd++, i++) {
 		pud = pud_offset(pgd, vaddr);
 		if (pud_none(*pud))
 			one_md_table_init(pud);
 		pmd = pmd_offset(pud, vaddr);
-		for (; (j < PTRS_PER_PMD) && (vaddr < end); pmd++, j++) {
+		for (; (j < PTRS_PER_PMD) && start <= end; pmd++, j++) {
 			one_page_table_init(pmd);
 			vaddr += PMD_SIZE;
+			start++;
 		}
 		j = 0;
 	}

That populates the page tables in the right places and fixrange_user_init() manages to call it, avoid death-by-oom from runaway allocations and then install references to all pages it wants. Alas, at that point the things become really interesting.

3) with the previous two issues dealt with, we get the following magical mistery shite when running 32bit uml kernel + userland on 64bit host:
* the system boots all the way to getty/login and sshd (i.e. gets
through the debian /etc/init.d (squeeze/i386))
* one can log into it, both on terminals and over ssh. shell and
a bunch of other stuff works. Mostly.
* /bin/bash -c "echo *" reliably segfaults. Always. So does tab
completion in bash, for that matter.
* said segfault is reproducible both from shell and under gdb.
For /bin/bash -c "echo *" under gdb it's always the 10th call of brk(3). What happens there apparently boils down to __kernel_vsyscall() getting called (and yes, sys_brk() is called, succeeds and results in expected value in %eax) and corrupting the living hell out of %ecx. Namely, on return from what presumably is __kernel_vsyscall() I'm seeing %ecx equal to (original value of) %ebp. All registers except %eax and %ecx (including %esp and %ebp) remain unchanged.
Again, that happens only on the same call of brk(3) - all previous
calls succeed as expected. I don't believe that it's a race. I also very much doubt that we are calling the wrong location - it's hard to tell with the call being call *%gs:0x10 (is there any way to find what that is equal to in gdb, BTW? Short of hot-patching movl *%gs:0x10,%eax in place of that call and single-stepping it, that is...) but it *does* end up making the system call that ought to have been made, so I suspect that it does hit __kernel_vsyscall(), after all...

The text of __kernel_vsyscall() is

	0xffffe420 <__kernel_vsyscall+0>:       push   %ebp
	0xffffe421 <__kernel_vsyscall+1>:       mov    %ecx,%ebp
	0xffffe423 <__kernel_vsyscall+3>:       syscall 
	0xffffe425 <__kernel_vsyscall+5>:       mov    $0x2b,%ecx
	0xffffe42a <__kernel_vsyscall+10>:      mov    %ecx,%ss
	0xffffe42c <__kernel_vsyscall+12>:      mov    %ebp,%ecx
	0xffffe42e <__kernel_vsyscall+14>:      pop    %ebp
	0xffffe42f <__kernel_vsyscall+15>:      ret    

so %ecx on the way out becoming equal to original %ebp is bloody curious - it would smell like entering that sucker 3 bytes too late and skipping mov %ecx, %ebp, but... we would also skip push %ebp, so we'd get trashed on the way out - wrong return address, wrong value in %ebp, changed %esp. None of that happens. And we are executing that code in userland - i.e. to get corrupt it would have to get corrupt in *HOST* 32bit VDSO. Which would have much more visible effects, starting with the next attempt to run the testcase blowing up immediately instead of waiting (as it actually does) for the same 10th call of brk()...

I'm at loss, to be honest. The sucker is nicely reproducible, but bisecting doesn't help at all - it seems to be present all the way back at least to 2.6.33. I hadn't tried to go back further and I hadn't tried to go for older host kernels, but I wouldn't put too much faith into that... The reason it hadn't been noticed much earlier is that it works fine on i386 host - aforementioned shit happens only when the entire thing (identical binary, identical fs image, identical options) is run on amd64. However, on i386 I have a different __kernel_vsyscall, which might easily be the reason it doesn't happen there. It's a K7 box with sysenter-based variant ending up as __kernel_vsyscall(). Hell knows what's going on... Behaviour is really weird and I'd appreciate any pointers re debugging that crap. Suggestions?

Am 20.08.2011 03:18, schrieb Al Viro:

Hmmm, very strange.
Sadly I cannot reproduce the issue. :(
Everything works fine within UML.
(Of course I've applied your vDSO/i386 patches)

My test setup:
Host kernel: 2.6.37 and 3.0.1
Distro: openSUSE 11.4/x86_64

UML kernel: 3.1-rc2
Distro: openSUSE 11.1/i386

Does the problem also occur with another host kernel or a different guest image?

Thanks,
//richard

Could you check what you get in __kernel_vsyscall()? On iAMD64 box where that sucker contains sysenter-based variant the bug is not present. IOW, it's sensitive to syscall vs. systenter vs. int 0x80 differences.

I can throw the trimmed-down fs image your way, BTW (66MB of bzipped ext2 ;-/) if you want to see if that gets reproduced on your box. I'll drop it on anonftp if you are interested. FWIW, the same kernel binary/same image result in
* K7 box - no breakage, SYSENTER-based vdso
* K8 box - breakage as described, SYSCALL-based vdso32
* P4 box - no breakage, SYSENTER-based vdso32
Hell knows... In theory that would seem to point towards ia32_cstar_target(), so I'm going to RTFS carefully through that animal.

The thing is, whatever happens happens when victim gets resumed inside vdso page. I'll try to dump PTRACE_SETREGS and see the values host kernel asked to set and work from there, but the interesting part is bloody hard to singlestep through - the victim is back to user mode and it is already traced by the guest kernel, so it's not as if we could attach host gdb to it and walk through that crap. And guest gdb is not going to be able to set breakpoints in there - vdso page is r/o...

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Am 20.08.2011 22:14, schrieb Al Viro:

OK, this explains why I cannot reproduce it.
My Intel Core2 box is sysenter-based.

(gdb) disass __kernel_vsyscall

0xffffe420 <__kernel_vsyscall+0>:       push   %ecx
0xffffe421 <__kernel_vsyscall+1>:       push   %edx
0xffffe422 <__kernel_vsyscall+2>:       push   %ebp
0xffffe423 <__kernel_vsyscall+3>:       mov    %esp,%ebp
0xffffe425 <__kernel_vsyscall+5>:       sysenter
0xffffe427 <__kernel_vsyscall+7>:       nop
0xffffe428 <__kernel_vsyscall+8>:       nop
0xffffe429 <__kernel_vsyscall+9>:       nop
0xffffe42a <__kernel_vsyscall+10>:      nop
0xffffe42b <__kernel_vsyscall+11>:      nop
0xffffe42c <__kernel_vsyscall+12>:      nop
0xffffe42d <__kernel_vsyscall+13>:      nop
0xffffe42e <__kernel_vsyscall+14>:      jmp 0xffffe423<__kernel_vsyscall+3>
0xffffe430 <__kernel_vsyscall+16>:      pop    %ebp
0xffffe431 <__kernel_vsyscall+17>:      pop    %edx
0xffffe432 <__kernel_vsyscall+18>:      pop    %ecx
0xffffe433 <__kernel_vsyscall+19>:      ret

Now I'm testing with a Debian fs from: http://fs.devloop.org.uk/filesystems/Debian-Squeeze/

[ CC'ing [email protected] ]
Andy, do you have an idea?
You can find Al's original report here:
http://marc.info/?l=linux-kernel&m=131380315624244&w=2

Thanks,
//richard

I'm missing a bit of the background. Is the user-on-UML app calling into a vdso entry provided by UML or into a vdso entry provided by the host?

Why does anything care whether ecx is saved? Doesn't the default calling convention allow the callee to clobber ecx?

But my guess is that the 64-bit host sysret code might be buggy (or the value in gs:whatever is wrong). Can you get gdb to breakpoint at the beginning of __kernel_vsyscall before the crash?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Am 20.08.2011 23:26, schrieb Andrew Lutomirski:

UML/i386 reuses the host's vDSO page.
IOW it does not have it's own vDSO like UML/x86_64.

Thanks,
//richard

This is suspicious:

ENTRY(ia32_cstar_target)
CFI_STARTPROC32 simple
CFI_SIGNAL_FRAME
CFI_DEF_CFA rsp,KERNEL_STACK_OFFSET
CFI_REGISTER rip,rcx
/*CFI_REGISTER rflags,r11*/
SWAPGS_UNSAFE_STACK
movl %esp,%r8d
CFI_REGISTER rsp,r8
movq PER_CPU_VAR(kernel_stack),%rsp
/*
* No need to follow this irqs on/off section: the syscall
* disabled irqs and here we enable it straight after entry:
*/
ENABLE_INTERRUPTS(CLBR_NONE)
SAVE_ARGS 8,0,0
movl %eax,%eax /* zero extension */
movq %rax,ORIG_RAX-ARGOFFSET(%rsp)
movq %rcx,RIP-ARGOFFSET(%rsp)
CFI_REL_OFFSET rip,RIP-ARGOFFSET
movq %rbp,RCX-ARGOFFSET(%rsp) /* this lies slightly to ptrace */

The entry code looks something like:

The text of __kernel_vsyscall() is

	0xffffe420 <__kernel_vsyscall+0>:       push   %ebp
	0xffffe421 <__kernel_vsyscall+1>:       mov    %ecx,%ebp
	0xffffe423 <__kernel_vsyscall+3>:       syscall
	0xffffe425 <__kernel_vsyscall+5>:       mov    $0x2b,%ecx
	0xffffe42a <__kernel_vsyscall+10>:      mov    %ecx,%ss
	0xffffe42c <__kernel_vsyscall+12>:      mov    %ebp,%ecx
	0xffffe42e <__kernel_vsyscall+14>:      pop    %ebp
	0xffffe42f <__kernel_vsyscall+15>:      ret

so the line:

movq %rbp,RCX-ARGOFFSET(%rsp) /* this lies slightly to ptrace */

will cause iret (if iret happens) to restore the original rbp in rcx (why? -- it seems okay if syscall is hit in __kernel_vsyscall but not if something else does the syscall). I don't see what saves rbp to the stack frame.

This is also suspicious:

movq %r11,EFLAGS-ARGOFFSET(%rsp)

that's inconsistent with my reading of the AMD manual.

How well is the compat syscall entry tested through both the fast and slow paths? UML is unusual in that it uses ptrace to trap all system calls, right? That means that syscalls will enter through the cstar target but return through the iret path.

--Andy

Far more interesting question is how the hell does that thing manage to work in face of syscall restarts? As the matter of fact, how does it (and sysenter-based variant) play with ptrace() *and* restarts?

Suppose we have a traced process. foo6() is called and the thing it stopped before the sys_foo6() is reached kernel-side. The sixth argument is on stack, ebp is set to user esp. SYSENTER happens, we read the 6th argument from userland stack and put it along with the rest into pt_regs. tracer examines the arguments, modifies them (including the last one) and lets the tracee run free - e.g. detaches from the tracee.

What should happen if we happen to get a signal that would restart that sucker? Granted, it's not going to happen with mmap() - it doesn't, AFAICS, do anything of that kind. However, I wouldn't bet a dime on other 6-argument syscalls not stepping on that. sendto() and recvfrom(), in particular...

OK, we return to userland. The sixth argument is placed into %ebp. Linus' "pig and proud of that" trick works and we end up slapping userland %esp into %ebp and hitting SYSENTER again. Only one problem, though - the sixth argument on user stack is completely unaffected by what tracer had done. Unlike the rest of arguments, that *are* changed.

We could deal with that in case of SYSENTER if we e.g. replaced that
jmp .Lenter_kernel
with
jmp .Lrestart
and added
.Lrestart:

	movl %ebp, (%esp)
	jmp .Lenter_kernel

but in case of SYSCALL it seems to be even messier... Comments?

... and there I thought that last year session of asm glue sniffing couldn't be topped by anything more unpleasant ;-/

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Oh, hell... Compat SYSCALL one is really buggered on syscall restarts, ptrace or no ptrace. Look: calling conventions for SYSCALL are

	arg1..5: ebx, ebp, edx, edi, esi.  arg6: stack
and after syscall restart we end up with
	arg1..5: ebx, ecx, edx, edi, esi.  arg6: ebp

so restart will instantly clobber arg2, in effect replacing it with arg6.

And yes, adding ptrace to the mix makes things even uglier. For one thing, changes to ECX via ptrace are completely lost on the fast exit. Not pretty, and might make life painful for uml, but not for the majority of programs. What's worse, combination of ptrace with restart will lose changes to arg6 (again, value on stack left as it was, changes to arg6 by tracer lost) *and* it will lose changes to arg2 (along with arg2 itself - see above).

Linus' Dirty Trick(tm) is not trivial to apply - with SYSCALL we *do* retain the address of next insn and that's where we end up going. IOW, SYSCALL not inside vdso32 currently works (for small values of "works", due to restart issues). Playing with return elsewhere might break some userland code...

Guys, that's *way* out of the area I'm comfortable with.

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

I don't see the point of all this hackery at all. sysenter/sysexit indeed screws up some registers, but we can return on the iret path in the case of restart.

So why do we lie to ptrace (and iret!) at all? Why not just fill in pt_regs with the registers as they were (at least the non-clobbered-by-sysenter ones), set the actual C parameters correctly to contain the six arguments (in rdi, rsi, etc.), do the syscall, and return back to userspace without any funny business? Is there some ABI reason that, once we've started lying to tracers, we have to keep doing so?

--Andy

Gack. Is this a holdover from the 32-bit code that shares the argument save area with the parameters passed on the C stack? If so, we could just set up the argument save area honestly and pass the real parameters in registers like 64-bit C code expects.

If the tracing and restart cases use iret to return to userspace, this should all just work. ptrace users shouldn't notice the overhead, and syscall restart is presumably slow enough anyway that it wouldn't matter. The userspace entry code would be as simple as:

sysenter
ret

or

sysexit
ret

--Andy

You are making no sense at all...

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

We *do* return on iret path in case of restart, TYVM.

We do not lie to ptrace and iret. At all. We do just what you have described. And fuck up when restart returns us to the SYSCALL / SYSENTER instruction again, which expects the different calling conventions, so the values arranged in registers in the way int 0x80 would expect do us no good.

FWIW, what really happens (for 32bit task on amd64) is this:
* both SYSCALL and SYSENTER variants of __kernel_vsyscall are
entered with the same calling conventions; eax contains syscall number, ebx/ecx/edx/esi/edi/ebp contain arg1..6 resp. Same as what int 0x80 would expect.
* they arrange slightly different calling conventions for
actual SYSCALL/SYSENTER instructions. SYSENTER one: ecx and edx saved on user stack to undo the effect of SYSEXIT clobbering them, arg6 (from ebp) pushed to stack as well (for kernel side of SYSENTER to pick it from there) and userland esp copied to ebp (SYSENTER clobbers esp). SYSCALL one: arg6 (from ebp) pushed to stack (again, for kernel to pick it from there), arg2 (from ecx) copied to ebp (SYSCALL clobbers ecx). Then we hit the kernel.
* Both codepaths start with arranging the same thing on the kernel
stack frame; one 64bit int 0x80 would create. For the good and simple
reason: they all have to be able to leave via IRET. Stack layout is the same, but we need to fill it accordingly to calling conventions we are stuck with. I.e. ->cx should be initialized with arg2 and ->bp with arg6, wherever those currently are on given codepath. _That_ is what "lying to ptrace" is about - we store there registers according to how they were when we entered __kernel_vsyscall(), not as they are at the moment of actual SYSCALL insn. Which is precisely the right thing to do, since if we *are* ptraced, the tracer expects to find the syscall argument in the same places, whichever variant of syscall tracee happens to be using.
* In both variants it means picking arg6 from userland stack; if
that pagefaults, we act as if we returned -EFAULT in normal way. Again, the value is stored in the expected place - ->bp, same as it would on int 0x80 path.
* If we are traced, we grow the things on stack to full pt_regs,
including the callee-saved registers. And call syscall_trace_enter(&regs). If tracer decides to change registers, it can do so. After that call we restore the registers from pt_regs on stack and rejoin the corresponding common codepath.
* In both cases we reshuffle registers to match amd64 C calling
conventions; the only subtle part is that SYSCALL path has arg6 in r9d (and ebp same as we had on entry, i.e. the original arg2, unaffected by whatever ptrace might have done to regs->cx, BTW) while SYSENTER path has it in ebp, same as int 0x80 one. After reshuffling arg6 ends up r9 in all cases and in all cases ptrace changes to regs->bp (aka where ptrace expects to see arg6) do affect what's in r9.
* The actual sys_whatever() is called in all cases. If there's
any work to do after it (signals, still being traced, need to be rescheduled, etc.), we go for the good old IRET path (after having cleaned r8--r12 in pt_regs - IRET path is shared with 64bit and we don't want random kernel values leaking to userland).
* If there's no non-trivial work to do, int 0x80 *still* cleans
r8--r12 in pt_regs and goes for IRET path. End of story for it.
* In the same case, SYSENTER path will restore the contents of si and
di from pt_regs (bx is unaffected by sys_whatever(), ax holds return value and cx/dx are going to be clobbered anyway; bp is not restored to the conditions it had when hitting SYSENTER, but it's redundant - it was equal to userland sp and *that* we do restore, of course). r8--r11 are cleared in actual CPU registers and off we bugger, back to vdso32. Where we pop ebp/ecx/edx and return to caller. Note that syscall restart couldn't have happened on that path - it would qualify as "work to do after syscall" (specifically, signal handling) as we'd be off to IRET path.
* In the same case, SYSCALL path will restore the contents of
si, di and dx from pt_regs (bx is unaffected by sys_whatever(), ax contains the return value and bp is actually the same as it was on entry, after all dances). r8-r11 are cleaned in registers, cx is clobbered by SYSRET and we are off to __kernel_vsyscall(), again. This time back in there we restore cx to what it used to be on entry to __kernel_vsyscall() [*NOTE*:
unaffected by ptrace manipulations; we probably don't care about that] and restore bp (from stack). We also restore %ss along the way, but that's a separate story. Again, no syscall restarts on that path.
* If there *was* a syscall restart to be done, we are guaranteed to
have left via IRET path. In all cases the syscall arguments end up in registers, in the same way int 0x80 expected them. What happens afterwards depends on how we entered, though.
+ int 0x80: all registers are restored (with ptrace
manipulations, if any, having left their effect) as they'd been the last time around. In we go and that's it.
+ SYSENTER: return address had been set *not* to the
insn right next after SYSENTER when we'd been setting the stack frame up. That's the dirty trick Linus had come up with - return ip is set to insn in vfso32 (SYSENTER loses the original ip for good, unlike SYSCALL that would store it in cx, so it has to be at fixed location anyway). Normally we just pop ecx/edx/ebp from stack and we are done. However, two bytes prior to that insn (i.e. where syscall restart would land us) we have jump to just a bit before SYSENTER. Namely, to the point where we had copied esp to ebp. That, combined with what IRET path has done, will get us the layout SYSENTER expects once we get to SYSENTER again. Except that ptrace modifications to arg6 will be lost - *ebp is where SYSENTER picks it from and it's not updated. Modified value is in ebp on return from kernel and it's overwritten (with esp) and lost. That's ptrace vs. restarts bug I've mentioned in SYSENTER case.
+ SYSCALL: buggered. On restart we end up repeating
the call, with arg2 replaced with whatever had been in ebp when we entered __kernel_vsyscall(). Simply because nobody cared to move it from ecx (where IRET path has put it) to ebp (where SYSCALL expects to find it). ebp gets what used to be in arg6 (again, IRET path doing). Oh, and ptrace modifications, if any, are lost as well - both in arg2 and in arg6.

I *think* the above is an accurate description of what happens,
but I could certainly be wrong - that's just from RTFS of unfamiliar and seriously convoluted code, so I'd very much appreciate ACK/NAK on the analysis above from the people actually familiar with that thing...

I think I believe your analysis...

This is, IMO, gross -- if the values in pt_regs matched what they were when sysenter / syscall was issued, then we'd be fine -- we could restart the syscall and everything would work. Apparently ptrace users have a problem with that, so we're stuck with the "lie" (i.e. reporting values as of __kernel_vsyscall, not as of the actual kernel entry).

Which suggests an easy-ish fix: if sysenter is used or if syscall is entered from the EIP is is supposed to be entered from, then just change ip in the argument save to point to the int 0x80 instruction. This might also require tweaking the userspace stack. That way, restart would hit int 0x80 instead of syscall/sysenter and the registers are exactly as expected.

Getting this right in the case where ptrace attaches during the syscall might be tricky, though.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Indeed. Just add an "int 0x80" instruction to the vsyscall thing, and you'd be done.

In fact, just replace the

jmp .Lenter_kernel

with

int 0x80

and you'd be pretty much all done, no?

(Ok, that's probably a huge over-simplification, but perhaps "close enough" to true that it would be workable)

Linus

In case of sysenter - almost, in case of syscall - nope...

Hm... I think a jump to something which adjusts %esp and invokes int $0x80 might just work, but only for SYSENTER.

SYSCALL is different, especially since SYSCALL is legal to execute from anywhere in userspace (and no, as we have learned already doing EIP checking is *NOT* acceptable.)

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Commit-ID:  6fe83695299270f5b813c037dc90492d13786b70
Gitweb:     http://git.kernel.org/tip/6fe83695299270f5b813c037dc90492d13786b70
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Mon, 22 Aug 2011 13:27:06 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 22 Aug 2011 13:27:06 -0700

x86-32, vdso: On system call restart after SYSENTER, use int $0x80

When we enter a 32-bit system call via SYSENTER or SYSCALL, we shuffle the arguments to match the int $0x80 calling convention. This was probably a design mistake, but it's what it is now. This causes errors if the system call as to be restarted.

For SYSENTER, we have to invoke the instruction from the vdso as the return address is hardcoded. Accordingly, we can simply replace the jump in the vdso with an int $0x80 instruction and use the slower entry point for a post-restart.

---
 arch/x86/vdso/vdso32/sysenter.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index e2800af..e354bce 100644
@@ -43,7 +43,7 @@ __kernel_vsyscall:
 	.space 7,0x90
 
 	/* 14: System call restart point is here! (SYSENTER_RETURN-2) */
-	jmp .Lenter_kernel
+	int $0x80
 	/* 16: System call normal return point is here! */
 VDSO32_SYSENTER_RETURN:	/* Symbol used by sysenter.c via vdso32-syms.h */
 	pop %ebp

Commit-ID:  7ca0758cdb7c241cb4e0490a8d95f0eb5b861daf
Gitweb:     http://git.kernel.org/tip/7ca0758cdb7c241cb4e0490a8d95f0eb5b861daf
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Mon, 22 Aug 2011 13:27:06 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Tue, 23 Aug 2011 16:20:10 -0700

x86-32, vdso: On system call restart after SYSENTER, use int $0x80

When we enter a 32-bit system call via SYSENTER or SYSCALL, we shuffle the arguments to match the int $0x80 calling convention. This was probably a design mistake, but it's what it is now. This causes errors if the system call as to be restarted.

For SYSENTER, we have to invoke the instruction from the vdso as the return address is hardcoded. Accordingly, we can simply replace the jump in the vdso with an int $0x80 instruction and use the slower entry point for a post-restart.

---
 arch/x86/vdso/vdso32/sysenter.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index e2800af..e354bce 100644
@@ -43,7 +43,7 @@ __kernel_vsyscall:
 	.space 7,0x90
 
 	/* 14: System call restart point is here! (SYSENTER_RETURN-2) */
-	jmp .Lenter_kernel
+	int $0x80
 	/* 16: System call normal return point is here! */
 VDSO32_SYSENTER_RETURN:	/* Symbol used by sysenter.c via vdso32-syms.h */
 	pop %ebp

Um, _no_. If nothing else, pt_regs is seen by sys_.... And they don't bloody know or care how the syscall had been entered.

Huh? Actions after SYSENTER differ from those after int 0x80. If nothing else, you would need to tweak saved userland stack pointer as well. It is possible, but I seriously doubt that it's a better way to deal with that mess. And in any case, SYSEXIT buggers CX/DX, so we'd need two separate post-syscall sequences in vdso. Yucky... I really don't like it.

The really ugly part for the SYSCALL variant is that right now we *can* do things like this:
read_it:

	pushl %ebp
	movl $__NR_read, %eax
	movl $0, %ebx
	movl $array, %ebp
	movl $100, %edx
	syscall
        movl $__USER32_DS, %ecx
        movl %ecx, %ss
	popl %ebp
	ret
anywhere in your userland and have it act as an equivalent of
int read_it(void)
{
	return read(0, array, 100);
}

Is that ability a part of userland ABI or are we declaring that hopelessly wrong and require to go through the function in vdso32? Linus?

As it is, I don't see any cheap ways to deal with restarts if that thing has to be preserved. For sysenter it's flatly prohibited and that allows us to play such games with adjusted return address. Here, OTOH...

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

If people are using syscall directly, we're pretty much stuck. No amount of "that's hopelessly wrong" will ever matter. We don't break existing binaries.

That said, I'd *hope* that everybody uses the vdso32, simply because user programs are not supposed to know which CPU they are running on and if that CPU even *supports* the syscall instruction. In which case it may be possible that we can play games with the vdso thing. But that really would be conditional on "nobody ever reports a failure".

But if that's possible, maybe we can increment the RIP by 2 for 'syscall', and slip an "'int 0x80" after the syscall instruction in the vdso there? Resulting in the same pseudo-solution I suggested for sysenter...

Linus

I think we found that out with the vsyscall emulation issue last cycle.
It works, so it will have been used, somewhere...

I think we have the above problem.

The problem here is that the syscall state is actually more complex than we retain: the entire state is given by (entry point, register state); with that amount of state we have all the information needed to *either* extract the syscall arguments *or* the register contents. Without those, we can only represent one of the two possible metalevels (right now we represent the higher-level metalevel, the argument vector), but we need both for different usages.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

My understanding of the problem is the following:

1. The SYSCALL 32-bit calling convention puts arg2 in ebp and arg6 on
the stack.

2. The int 0x80 convention is different: arg2 is in ecx.

3. We're worried that pt_regs-using compat syscalls might want the
regs to appear to match the actual arguments (why?)

4. ptrace expects the "registers" when SYSCALL happens to match the
int 0x80 convention. (This is, IMO, sick.)

5. Syscall restart with the SYSCALL instruction must switch to
userspace and back to the kernel for reasons I don't understand that presumably involve signal delivery.

6. Existing ABI requires that the kernel not clobber syscall
arguments (except, of course, when ptrace or syscall restart explicitly change those arguments).

So we're sort of screwed. arg2 must be in ecx to keep ptrace happy but SYSCALL clobbers ecx, so arg2 cannot be preserved.

So here are three strawman ideas:

a) Change #4. Maybe it's too late to do this, though.

b) When SYSCALL happens, change RIP to point two bytes past an int 0x80 instruction in the vdso. Make the next instruction there be a "ret" that returns to the instruction after the original syscall. Patch the stack in the kernel.

c) Disable syscall restart when SYSCALL happens from somewhere outside the vdso.

--Andy

run strace and you'll see why.

That's what ptrace is *for*. It's there to let debuggers play with the program being debugged, including taking a look at the syscall arguments and modifying them. In a predictable way, please.

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

I'm talking about the implementations of stub32_rt_sigreturn, sys32_rt_sigreturn, stub32_sigreturn, stub32_sigaltstack, stub32_execve, stub32_fork, stub32_clone, stub32_vfork, and stub32_iopl. I don't know what this has to do with strace or user ABI at all.

It may be necessary, but I still think it's sick. Especially in the case of inlined SYSCALL, where the registers reported to ptrace do not match any register values that ever actually existed in CPU registers.
Too late to fix it, though.

Which still leaves the question of how to fix it. Restarting via an int 0x80-based helper might be the only option that leaves everything fully functional.

--Andy

The issue is that we don't represent the entire state ... we represent only one metalevel of it, currently the "cooked" one. The problem is that we need the "raw" one as well, and in order to have *both* we need to know the entry mechanism.

We need that IN EITHER CASE.

This is reasonably straightforward... we can carry the entry mechanism forward inside the kernel, and fix it up in the IRET path.

The *really* big issue is what we drop as the sigcontext since this is an ABI carried out to userspace. We could just say "it's currently totally broken for SYSCALL" and just change it to drop the raw state, but which has the potential for breaking unknown programs, *or* we could add a bit of state (presumably by reclaiming one of the padding fields around cs and ss) ... which *also* has the potential for breaking programs.

Right now, SYSCALL -> signal -> restart *is broken*, however, so there is also the option of just doing nothing in this case, I guess.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Borislav,

We're tracking down an issue with the way system call arguments are handled on 32 bits. We have a solution for SYSENTER but not SYSCALL; fixing SYSCALL "properly" appears to be very difficult at best.

So the question is: how much overhead would it be to simply fall back to int $0x80 or some other legacy-style domain crossing instruction for 32-bit system calls on AMD64 processors? We don't ever use SYSCALL in legacy mode, so native i386 kernels are unaffected.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Last i measured INT80 and SYSCALL costs they were pretty close to each other on AMD CPUs - closer than on Intel.

Also, most installations are either pure 32-bit or dominantly 64-bit, the significantly mixed-mode case is dwindling.

Unifying some more in this area would definitely simplify things ...

Thanks,

Ingo

than int 0xcc, which is presumably about the same speed as int 0x80. That's because SYSCALL is blazingly fast (<30 ns for a whole system call) and int is slower.

--Andy

Just to make sure I'm grokking this correctly - we want to use int $0x80 only for the SYSCALL variant in __kernel_vsyscall, right? Not for all 32-bit syscalls on a 64-bit kernel.

Thanks.

Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Um... The problem is, syscall restart with SYSCALL insn is badly broken; we end up with arg2 (expected by SYSCALL in %ebp) overwritten with arg6 by IRET path. With obvious nasty results.

In __kernel_vsyscall() the problem is possible to deal with; there we control the code around that sucker. It's SYSCALL in 32bit binary outside of vdso32 that causes real PITA...

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

I just checked. 'syscall' (at least on x86-64) is definitely called outside of __kernel_vsyscall in all the normal cases. It's part of the fundamental ABI, after all. We don't use "int 0x80" there.

But on x86-32, I think we might be better off. There, we only have 'sysenter', and can perhaps use my suggested "just use int 0x80 instead of the jump back to the sysenter instruction" trick. Plus people *will* be using __kernel_vsyscall, since on x86-32 you aren't guaranteed to have a CPU that supports sysenter to begin with.

Or am I missing something else?

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

SYSCALL in 64-bit mode is not a problem.

SYSCALL in compatibility mode (32-on-64) *is* a problem, because ECX is clobbered. Unfortunately AMD processors only support SYSENTER in legacy mode (32-on-32) -- unlike Intel and VIA.

Your trick solves SYSENTER, which takes care of legacy mode and Intel and VIA processors in compatibility mode.

Borislav is checking into if we can just use INT 80h on AMD processors in compatibility mode. So far the indication seems to be that it is probably okay.

-hpa

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Even if it's ok, we still have to do *something* in the cstar entry point -- I don't think there's any way to turn SYSCALL in compatibility mode but leave it enabled in long mode. So if we're planning on killing off SYSCALL from outside the vdso, we could probably get away with leaving it enabled in the vdso.

Unless, of course, there are 32-bit JITs that do things that they shouldn't. We could still make it work by rewriting the arguments (including arg6 on the stack) from the syscall restart path, but that may be more trouble than it's worth.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hm, looks like you might be right; I thought leaving CSTAR at zero would take care of that, but it looks like that might not be true.

However, we could just issue a SIGILL or SIGSEGV at this point; the same way we would if we got an #UD or #GP fault; SIGILL/#UD would be consistent with Intel CPUs here.

Yes, we could do an EIP check and issue SIGILL only if it isn't in the vdso, but after the crap from earlier this month I would be happier if we could avoid this potential problem entirely. Borislav has a query in with his hardware folks, let's give them the chance to answer.

-hpa

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Considering that this is not a remotely new issue, and that it has been around for years without anybody even noticing, I'd really prefer to just fix things going forwards rather than add any code to actively break any possible unlucky legacy users.

So I think the "let's fix the vdso case for sysenter" + "let's remove the 32-bit syscall vdso" is the right solution. If somebody has hardcoded syscall instructions, or generates them dynamically with some JIT, that's their problem. We'll continue to support it as well as we ever have (read: "almost nobody will ever notice").

One thing we *could* do is to just say "we never restart a x86-32 'syscall' instruction at all", and just make such a case return EINTR. IOW, do something along the lines of the appended pseudo-patch.

Because returning -EINTR is always "almost correct".

Hmm?

Linus

---
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c index 54ddaeb221c1..bc1a0f8b2707 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -678,6 +678,16 @@ setup_rt_frame(int sig, struct k_sigaction *ka,
siginfo_t *info,
return ret;
}

+static void restart_syscall(struct pt_regs *regs, int orig) +{
+ if (regs->syscall_using_syscall_insn) {
+ regs->ax = -EINTR;
+ return;
+ }
+ regs->ax = orig;
+ regs->ip -= 2;
+}
+
static int
handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
struct pt_regs *regs)
@@ -701,8 +711,7 @@ handle_signal(unsigned long sig, siginfo_t
*info, struct k_sigaction *ka,
}
/* fallthrough */
case -ERESTARTNOINTR:
- regs->ax = regs->orig_ax;
- regs->ip -= 2;
+ restart_syscall(regs, regs->orig_ax);
break;
}
}
@@ -786,13 +795,11 @@ static void do_signal(struct pt_regs *regs)
case -ERESTARTNOHAND:
case -ERESTARTSYS:
case -ERESTARTNOINTR:
- regs->ax = regs->orig_ax;
- regs->ip -= 2;
+ restart_syscall(regs, regs->orig_ax);
break;

   		case -ERESTART_RESTARTBLOCK:
  -			regs->ax = NR_restart_syscall;
  -			regs->ip -= 2;
  +			restart_syscall(regs, NR_restart_syscall);
   			break;
   		}
   	}

I have to say it worries me from a potential security hole point of view, especially since it clearly isn't very well trod ground to begin with. An almost-never-used path with access to the full system call suite is scarier than hell in that sense.

Keep in mind support for SYSCALL32 is already (vendor-)conditional.

(The obvious solution of just putting the proper register frame back in its place would be okay except for totally breaking anything trace-on-exit as already hashed to death...)

-hpa

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Umm... Maybe, but I really wonder if it would be better to do this:
* check if %ecx is the right one for vdso32 entry. If it isn't,
act as we act now (and possibly warn). If it is, increment it by 4.
* slap 0x90, 0x90, 0xcd, 0x80 right after that syscall insn -
i.e. nop/nop/int 0x80. Followed by what we currently do there.

Those who do syscall insn in 32bit mode outside of vdso will get
what they currently get. __kernel_vsyscall() will keep working and do that without restart problems. And the price is comparison + branch not taken + addition for that path...

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

s/whine/set your "don't restart that one" flag/, even...

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Checking for SYSCALL in the vdso is cheap... the only problem is if someone thinks that they can copy it out of the vdso and run it, having special-cased SYSENTER already, but not SYSCALL. The recent experience shows that that is apparently a very real possibility.

Without a SYSCALL in the vdso, anything that examines the vdso will end up using int $0x80 and we don't have an issue.

Again, waiting for Borislav & co to tell us how much the SYSCALL instruction actually buys us.

-hpa

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

I don't understand why people keep harping on this "copy the vdso" thing.

Guys, IF THEY COPY THE VDSO, THEN THEY WOULD GET OUR IMPROVEMENTS!

So stop being silly.

As to comparing the IP address, that's just moronic. Why the hell would anybody ever do something that stupid? It's expensive and not worth it. You'd be much better off comparing the contents of the memory at 'rip', and see if that memory contains the 'int 0x80' instruction we would add. That automatically also gets the 'somebody copied the vdso text' case right.

You guys seem to positively _want_ to make this a bigger issue than it is. As far as I can tell, nobody has ever even noticed this problem before, and we already have a trivial fix ("don't do it then") for the case Al actually did notice.

The *only* case left as far as I can tell is the case where people do *not* copy the vdso, but simply generate (statically and/or dynamically with a JIT) the syscall instruction directly. And we cannot fix that, so stop even bothering to try. The absolute best we can do for that case is to just continue to do what we do now. WE SIMPLY HAVE NO CHOICE.

I really don't see this as being worth this much discussion.

Linus

I'm not sure. What I've noticed was complete weirdness in uml, apparently
dependent on SYSCALL vs. SYSENTER or int 0x80. With following round of RTFS and finding what looks like an ugly issue with restarts.

What I don't understand at all is how the hell do we manage to avoid much more obvious breakage all the time we ptrace a 32bit task. Look:
__kernel_vsyscall() is
push %ebp
movl %ecx, %ebp
syscall
movl $__USER32_DS, %ecx
movl %ecx, %ss
movl %ebp, %ecx
popl %ebp

now, what is going to happen to %ebp if we go through IRET path, for any reason? From my reading it appears that right after that IRET we'll have ebp containing arg6. I.e. what we'd pushed on stack. Now, popl %ebp will bring the same value back. Not a problem. But what about movl %ebp, %ecx? Again, I'm talking about the case when we have no restart at all - just an strace(1) tracing a process.

AFAICS, in that case we ought to have %ecx == %ebp after return from __kernel_vsyscall(). Which would blow the things up _very_ fast.

So what the hell am I missing here?

*UGH*. OK,
1) I'm an idiot; int_ret_from_sys_call does *not* usually step on
rbp (it's callee-saved). So normally ebp is left as is on the way out, which is why we don't see stuff getting buggered left, right and center.
2) Sometimes it apparently does somehow happen. I don't see where
it happens yet, but uml breakage that started all of that looks *exactly* like that. %ebp getting arg6 in it when we return into __kernel_vsyscall() from the kernel fits the observed pattern precisely.
3) modulo that the situation is nowhere near as bad as I thought.
Brown paperbag time for me - for missing that if my analysis had been correct we'd've seen breakage _much_ earlier. Mea culpa.
4) we still have a problem, apparently, but it's more narrow now -
the question is when would %rbp be shat into?

Al, off to apply a serious self-LART...

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Check.

And the system call restart should actually work fine too, because at syscall entry we save %ebp *both* in the slot for ebp and ecx when we enter the first time. So the second time, we'll re-load the third argument from ebp again, but that's fine - it's still going to be the right value. Yes? No?

However, I note that the cstar entrypont has a comment about not saving ebp:

* %ebp Arg2 [note: not saved in the stack frame, should not be touched]

which sounds odd. Why don't we save it? If we take a signal handler there, don't we want %ebp on the kernel stack in pt_regs, in order to do everything right?

Now I'm *really* confused.

Linus

That's exactly because it's callee-saved. amd64 doesn't build full pt_regs on stack; there's a part built always (5 words needed for iret to work + syscall number + rdi + rsi + rdx + rcx + rax + r8--r11) and the rest of registers is not saved in regular cases. Reason: as long as what we are calling follows amd64 ABI, we are guaranteed that values of rsp/rbp/rbx/r12--r15 will not change. So we don't waste cycles and stack space unless we need to. Which is to say,
* in fork/clone/vfork - there we want full pt_regs to copy it into
child's pt_regs.
* in {rt_,}sigreturn - we don't care about the current contents of
those registers, but we want to set them. Thus the full pt_regs on stack, filled by sys_{rt_,}sigreturn() and these extra registers filled with values from pt_regs.
* execve() - we want all registers reset to know state after
sys_execve(), so it fills the full pt_regs and we get the extra regs filled out of it.
* sigaltstack() - there full pt_regs is an overkill, but we do want
userland sp.
* signal delivery - we want these registers preserved across the
duration of handler and we can't depend on handler following ABI. So we fill the entire pt_regs, and copy it into sigcontext, to be eventually picked up by sigreturn and reconstruct the entire state.
* ptrace - we want to be able to read/modify *all* these guys.
So we fill the entire pt_regs, let ptrace play with it and read extra regs back. NOTE: ia32_cstar_tracesys() takes pains to prevent buggering ebp there - we read the arg6 into r9, then swap it with ebp for duration of that stuff. So ptrace will see arg6 in regs.bp, but when it's time to go into syscall the (possibly modified) value will end in r9. Which is how it's passed to C functions, so we are fine, but it'll be really lost before we reach the userland. However, on the way *OUT* we are not that nice, and SETREGS/POKEUSER hitting us there will end up modifying ebp. Which will play hell on __kernel_vsyscall()...

Hell, you have done something very similar on alpha yourself... As for ebp, it doesn't make any sense to save it on stack - ia32_cstar_entry() itself takes care of not stomping on it just fine and IRET path (int_ret_from_sys_call) modifies rbp only if explicitly asked to do so... Which is most likely where it hits the fan for uml. Normally it wouldn't hurt to ask PTRACE_PUTREGS to put into ebp the value we just got from PTRACE_GETREGS. However, it *does* hurt when it happens on the second stop per syscall - i.e. when we are on the way out. I'm not 100% sure that this is what's going on (it's using PTRACE_SYSEMU, which is supposed to avoid the second stop completely), but it looks like what I'm seeing...

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

So it smells like a nasty effect of PTRACE_SETREGS/PTRACE_POKEUSER on the way *out* of syscall (the same on the way in wouldn't have such effect - modified EBP value would be simply lost after the syscall; passed to it as arg6, but that's it).

All right, now I have a nice shiny reproducer for uml folks:
main()

{
	char *s = sbrk(8192);
	*s = 0;
	brk(s);
}

will do it on the affected boxen. It gets fucked in the second call of brk(). What happens is this:
brk(3) in libc:
about to call brk(2), will have to stomp on %ebx
save ebx into ecx for the duration of call
the new brk level into ebx, syscall number into eax
hit __kernel_vsyscall()
push ebp
mov ecx, ebp
ecx is going to get stomped on, save it (i.e. original ebx) into ebp
syscall
and at that point ebp has changed - it became equal to arg6, aka what we'd put on stack, aka the value of ebp prior to all of that, aka the frame pointer of caller.
use ecx to set ss back to sanity
mov ebp, ecx
and now ecx is buggered.
pop ebp
the value of ebp has actually not been changed by that.
ret
put the value of ecx back into ebx
only it's not the value we used to have there. Not anymore. Now we are about to store the return value of brk(2) into static variable. And that's where it really hits the fan, since we are in a PIC code and ebx is not what it used to be. So instead of that variable we access hell knows what address and promptly segfault.

I have a very strong suspicion that I know what will turn out to be involved into that - the page eviction done by sys_brk(). Note that dirtying this sucker is really necessary - without *s = 0 it won't segfault at all. With it we get a segfault described above.

And page eviction on uml is nasty and convoluted as hell. It has to do munmap() on process' VM. Which is done in a rather sick way - we have a stub present in address space of all processes, with a function that does a given series of mmap/munmap/mprotect and traps itself. Guest kernel puts arguments for that sucker into a shared data page and continues the process into that function. Once it's done, we get the damn thing stopped again, nice and ready for us to continue dealing with it.

Something in that shitstorm of ptrace() calls ends up doing SETREGS when victim sits on the way out of (host) syscall. Boom...

Almost, but not quite. What happens is:
* process hits syscall insn
* it's stopped and tracer (guest kernel) does GETREGS
+ looks at the registers (mapped to the normal layout)
+ decides to call sys_brk()
+ notices pages to kick out
+ queues munmap request for stub
* tracer does SETREGS, pointing the child's eip to stub and sp to stub stack
* tracer does CONT, letting the child run
* child finishes with syscall insn, carefully preserving ebp. It returns to userland, in the beginning of the stub.
* child does munmap() and hits int 3 in the end of stub.
* the damn thing is stopped again. The tracer had been waiting for it.
* tracer finishes with sys_brk() and returns success.
* it does SETREGS, setting eax to return value, eip to original return address of syscall insn... and ebp to what it had in regs.bp. I.e. the damn arg6 value.

And we are fucked. It doesn't happen in syscall handler. It's int3(). Having no idea that this request to set ebp should be interpreted in a really different way - "put the value I asked to put into ecx here, please, and ignore this one".

Sigh... The really ugly part is that ebp can be changed by the stuff done in stub - it's not just munmap, it can do mmap as well. We can, in principle, save ebp on its stack and restore it before trapping. Then uml kernel could, in theory, replace that SETREGS with a bunch of POKEUSER, leaving ebp alone. Ho-hum... In principle, that might even be not too horrible - we need eax/eip/esp, of course, but the rest could be dealt with by the same trick - have it pushed/popped in the stub and to hell with wasting syscalls on setting them...

Anyway, bedtime for me. I'll look into that again in the morning...

Ok, stupid question: can a convoluted ptracing case like this be created in "normal" userspace, i.e. irrespective of UML and only by using gdb, for example?

I.e., from what I understand from above, you need to stop the tracee at syscall and "redirect" it to the stub after it finishes the syscall so that in another syscall it gets a debug exception... sounds complicated.

which could mean that we could get away by not replacing SYSCALL32?

Hmm.

Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

I don't know...

Basically, we need to do things that tracer can't do via ptrace() - i.e. play with mappings in the child. I.e. we need to do several syscalls in child, then return it to traced state. And all of that - before we return to execution of instructions past the syscall.

BTW, booting 32bit uml with nosysemu on such boxen blows up instantly, since there we have *all* SETREGS done on the way out of syscall (with sysemu we use PTRACE_SYSEMU, which will stop on syscall entry, let you play with registers and suppress both the sys_...() call itself and the stop on the way out; without sysemu it'll use PTRACE_SYSCALL, replace syscall number with something harmless (getpid(2)), let it execute, stop on the way out and update the registers there). Same issue, only here it really happens from within the syscall handler itself.

Hell knows... I have no idea what kind of weirdness ptrace users exhibit. FWIW, I suspect that there's another mess around signals in uml - signal frame is built by tracer and it *has* to contain ebp. And have eip pointing to insn immediately past the syscall. What should sigreturn do? It got to restore ebp - can't rely on signal handler not having buggered the register. And we are again in for it - ebp set to arg6 as we return to insn right after syscall one.

OTOH, making GETREGS/PEEKUSER return registers without arg2 -> ecx, arg6 -> ebp would instantly break both uml and far less exotic things. strace(1), for one. Anything that wants to examine the arguments of stopped syscall will be broken.

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Ok, I think that exhaustively explains that

(a) our system call restart has always worked correctly, and we're good.

(b) it's simply just UML that is buggy, and doesn't understand the
subtleties about doing GETREGS at a system call.

and I think that the correct and simple solution is to just teach UML to understand the proper logic of pt_regs during a system call (and a 'syscall' instruction in particular).

The thing is, UML emulates 'syscall' the way the *CPU* does it, not the way *we* do it. That may make sense, but it's simply not correct.

So I would vote very strongly against actually changing anything in arch/x86. This is very much an UML issue.

Suggested fixes:

- instead of blindly doing SETREGS, just write the result registers
individually like you suggested

OR (and perhaps preferably):

- teach UML that when you do 'GETREGS' after a system call trapped,
we have switched things around to match the "official system call order", and UML should just undo our swizzling, and do a "regs.ebp = regs.ecx" to make it be what the actual original registers were (or whatever the actual correct swizzle is - I didn't think that through very much).

IOW, I think the core kernel does the right thing. Our argument register swizzling is odd, yes, but it's an implementation detail that something like uml should just have to take into account. No?

Hmm?

Linus

Egads. Does this mean that doing GETREGS and then doing SETREGS later on with the *exact same values* is considered incorrect? IMO, this way lies madness.

In any case, this seems insanely overcomplicated. I'd be less afraid of something like my approach (which, I think, makes all of the SYSCALL weirdness pretty much transparent to ptrace users) or of just removing SYSCALL entirely from 32-bit code.

--Andy

No. If it does it with exactly the same values, it's perfectly ok. But it isn't. It's changing some of those values.

No, the madness is in UML.

It's EMULATING A SYSTEM CALL. That original "getregs" value is not some "user space state". It's the *system call* state that you got after the system call trapped. Setting it back is an insane operation, but it would happen to work - if you make no changes.

But UML *does* make changes. It takes that system call state, and then EMULATES THE SYSTEM CALL INCORRECTLY.

If you see it that way (which is the correct way), then it's clearly an UML problem, and it's not at all "madness" that your getregs/setregs pairing doesn't work.

See? Buggy system call emulation. It's really that simple. Of course, "simple" in this case is "really really subtle differences in how the kernel treats syscall/sysenter/int80", so the *details* are certainly not simple, but the concept is.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

It's a bit more than that (ptrace changes to syscall arguments *are* lost on syscall restart), but... as far as I'm concerned, the situation is simple now:
* SYSCALL is not terminally broken wrt restarts. My apologies for
misreading what was going on.
* SYSENTER with Linus' patch does work just fine wrt restarts + ptrace
* SYSCALL is losing ptrace-made changes to arguments when it restarts.
Might or might not be a problem for somebody.
* UML should not touch SYSCALL for 32bit. Not without serious changes
in UML and I'm not convinced that it won't be worse than what we probably ought to do there: check if __kernel_vsyscall() does SYSCALL (recognizable by interaction with POKEUSER) and don't tell about vdso to guest processes. Anything well-behaving won't step on SYSCALL and the things that do deserve the subtle bugs they get.
* asm glue is subtle, evil and doesn't have anywhere near enough
documentation ;-/

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

BTW, that one (irrelevant to UML even if we do end up coping with SYSCALL there) might be worth spelling it out:

tracer:

	ptrace(tracee, PTRACE_SYSCALL);
tracee:
	recvfrom(..., &addrlen);
tracer:
	ptrace(tracee, PTRACE_POKEUSER, EBP, &len2);
	ptrace(tracee, PTRACE_DETACH, 0, 0);

tracee:
completes recvfrom(), using &len2 instead of the &addrlen

That works just fine, regardless of the way syscall is entered; yes, including SYSCALL - there we take care to handle ptrace on the way in. However, if it's SYSCALL and (ex-)tracee takes a restart, the second time around we'll have the original value of 6th argument used. Changes made by POKEUSER are lost.

It's not a problem with int 0x80 or SYSENTER (now, with int 0x80 instead of jmp). It's probably not going to be a real issue for anyone, but I pity the poor bastard stuck with debugging that if it *does* become someone's problem.

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

I took the liberty to document some of your asm glue analysis in an attempt to make the code a bit more understandable. How about the following:

--
From: Borislav Petkov <[email protected]>
Date: Wed, 24 Aug 2011 14:30:43 +0200
Subject: [PATCH] x86, asm: Document some of the syscall asm glue

Document some of the asm glue around compat SYSCALL32 and do a whitespace cleanup while at it. See linked thread below for further reference.

---
 arch/x86/ia32/ia32entry.S  |  138 ++++++++++++++++++++++++++-----------------
 arch/x86/kernel/entry_64.S |   19 +++++-
 2 files changed, 98 insertions(+), 59 deletions(-)

index a0e866d..8254432 100644
@@ -1,16 +1,16 @@
 /*
- * Compatibility mode system call entry point for x86-64. 
- * 		
+ * Compatibility mode system call entry point for x86-64.
+ *
  * Copyright 2000-2002 Andi Kleen, SuSE Labs.
- */		 
+ */
 
 #include <asm/dwarf2.h>
 #include <asm/calling.h>
 #include <asm/asm-offsets.h>
 #include <asm/current.h>
 #include <asm/errno.h>
-#include <asm/ia32_unistd.h>	
-#include <asm/thread_info.h>	
+#include <asm/ia32_unistd.h>
+#include <asm/thread_info.h>
 #include <asm/segment.h>
 #include <asm/irqflags.h>
 #include <linux/linkage.h>

@@ -38,11 +38,11 @@
 	xchg	%ecx,%esi
 	movl	%ebx,%edi
 	movl	%edx,%edx	/* zero extension */
-	.endm 
+	.endm
 
-	/* clobbers %eax */	
+	/* clobbers %eax */
 	.macro  CLEAR_RREGS offset=0, _r9=rax
-	xorl 	%eax,%eax
+	xorl	%eax,%eax
 	movq	%rax,\offset+R11(%rsp)
 	movq	%rax,\offset+R10(%rsp)
 	movq	%\_r9,\offset+R9(%rsp)

@@ -69,7 +69,7 @@
 	movl \offset+64(%rsp),%edi
 	movl %eax,%eax			/* zero extension */
 	.endm
-	
+
 	.macro CFI_STARTPROC32 simple
 	CFI_STARTPROC	\simple
 	CFI_UNDEFINED	r8

@@ -106,14 +106,14 @@ ENDPROC(native_irq_enable_sysexit)
  * %esi Arg4
  * %edi Arg5
  * %ebp user stack
- * 0(%ebp) Arg6	
- * 	
+ * 0(%ebp) Arg6
+ *
  * Interrupts off.
- *	
+ *
  * This is purely a fast path. For anything complicated we use the int 0x80
  * path below.	Set up a complete hardware stack frame to share code
  * with the int 0x80 path.
- */ 	
+ */
 ENTRY(ia32_sysenter_target)
 	CFI_STARTPROC32	simple
 	CFI_SIGNAL_FRAME

@@ -127,7 +127,7 @@ ENTRY(ia32_sysenter_target)
 	 * disabled irqs, here we enable it straight after entry:
 	 */
 	ENABLE_INTERRUPTS(CLBR_NONE)
- 	movl	%ebp,%ebp		/* zero extension */
+	movl	%ebp,%ebp		/* zero extension */
 	pushq_cfi $__USER32_DS
 	/*CFI_REL_OFFSET ss,0*/
 	pushq_cfi %rbp

@@ -144,12 +144,12 @@ ENTRY(ia32_sysenter_target)
 	pushq_cfi %rax
 	cld
 	SAVE_ARGS 0,1,0
- 	/* no need to do an access_ok check here because rbp has been
- 	   32bit zero extended */ 
+	/* no need to do an access_ok check here because rbp has been
+	   32bit zero extended */
 1:	movl	(%rbp),%ebp
- 	.section __ex_table,"a"
- 	.quad 1b,ia32_badarg
- 	.previous	
+	.section __ex_table,"a"
+	.quad 1b,ia32_badarg
+	.previous
 	GET_THREAD_INFO(%r10)
 	orl    $TS_COMPAT,TI_status(%r10)
 	testl  $_TIF_WORK_SYSCALL_ENTRY,TI_flags(%r10)

@@ -170,7 +170,7 @@ sysenter_dispatch:
 sysexit_from_sys_call:
 	andl    $~TS_COMPAT,TI_status(%r10)
 	/* clear IF, that popfq doesn't enable interrupts early */
-	andl  $~0x200,EFLAGS-R11(%rsp) 
+	andl  $~0x200,EFLAGS-R11(%rsp)
 	movl	RIP-R11(%rsp),%edx		/* User %eip */
 	CFI_REGISTER rip,rdx
 	RESTORE_ARGS 0,24,0,0,0,0

@@ -260,20 +260,21 @@ ENDPROC(ia32_sysenter_target)
  * Arguments:
  * %eax	System call number.
  * %ebx Arg1
- * %ecx return EIP 
+ * %ecx return EIP
  * %edx Arg3
  * %esi Arg4
  * %edi Arg5
- * %ebp Arg2    [note: not saved in the stack frame, should not be touched]
- * %esp user stack 
+ * %ebp Arg2    [note: not saved in the stack frame, should not be touched
+ *		 because it is callee-saved in 64-bit calling convention]
+ * %esp user stack
  * 0(%esp) Arg6
- * 	
+ *
  * Interrupts off.
- *	
+ *
  * This is purely a fast path. For anything complicated we use the int 0x80
  * path below.	Set up a complete hardware stack frame to share code
- * with the int 0x80 path.	
- */ 	
+ * with the int 0x80 path.
+ */
 ENTRY(ia32_cstar_target)
 	CFI_STARTPROC32	simple
 	CFI_SIGNAL_FRAME

@@ -281,34 +282,57 @@ ENTRY(ia32_cstar_target)
 	CFI_REGISTER	rip,rcx
 	/*CFI_REGISTER	rflags,r11*/
 	SWAPGS_UNSAFE_STACK
+
+	/* stash away usermode stack ptr */
 	movl	%esp,%r8d
 	CFI_REGISTER	rsp,r8
 	movq	PER_CPU_VAR(kernel_stack),%rsp
+
 	/*
 	 * No need to follow this irqs on/off section: the syscall
 	 * disabled irqs and here we enable it straight after entry:
 	 */
 	ENABLE_INTERRUPTS(CLBR_NONE)
 	SAVE_ARGS 8,0,0
-	movl 	%eax,%eax	/* zero extension */
+	movl	%eax,%eax	/* zero extension */
 	movq	%rax,ORIG_RAX-ARGOFFSET(%rsp)
+
+	/* return-RIP is in %ecx when executing SYSCALL */
 	movq	%rcx,RIP-ARGOFFSET(%rsp)
 	CFI_REL_OFFSET rip,RIP-ARGOFFSET
-	movq	%rbp,RCX-ARGOFFSET(%rsp) /* this lies slightly to ptrace */
+
+	/*
+	 * Put Arg2 into %rcx pt_regs slot to match kernel syscall
+	 * calling conventions, i.e. what INT80 would expect;
+	 * this lies slightly to ptrace
+	 */
+	movq	%rbp,RCX-ARGOFFSET(%rsp)
 	movl	%ebp,%ecx
 	movq	$__USER32_CS,CS-ARGOFFSET(%rsp)
 	movq	$__USER32_DS,SS-ARGOFFSET(%rsp)
+
+	/* rFLAGS is in %r11 when executing SYSCALL */
 	movq	%r11,EFLAGS-ARGOFFSET(%rsp)
 	/*CFI_REL_OFFSET rflags,EFLAGS-ARGOFFSET*/
-	movq	%r8,RSP-ARGOFFSET(%rsp)	
+
+	/* save usermode stack ptr into pt_regs */
+	movq	%r8,RSP-ARGOFFSET(%rsp)
 	CFI_REL_OFFSET rsp,RSP-ARGOFFSET
-	/* no need to do an access_ok check here because r8 has been
-	   32bit zero extended */ 
-	/* hardware stack frame is complete now */	
+
+	/*
+	 * Get Arg6 which is on the usermode stack; no need to do an
+	 * access_ok check here because %r8 has been 32bit zero extended.
+	 * hardware stack frame is complete now.
+	 */
 1:	movl	(%r8),%r9d
+
+	/*
+	 * handle pagefaulting when accessing usermode stack by returning
+	 * -EFAULT
+	 */
 	.section __ex_table,"a"
 	.quad 1b,ia32_badarg
-	.previous	
+	.previous
 	GET_THREAD_INFO(%r10)
 	orl   $TS_COMPAT,TI_status(%r10)
 	testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags(%r10)

@@ -331,7 +355,7 @@ sysretl_from_sys_call:
 	RESTORE_ARGS 0,-ARG_SKIP,0,0,0
 	movl RIP-ARGOFFSET(%rsp),%ecx
 	CFI_REGISTER rip,rcx
-	movl EFLAGS-ARGOFFSET(%rsp),%r11d	
+	movl EFLAGS-ARGOFFSET(%rsp),%r11d
 	/*CFI_REGISTER rflags,r11*/
 	xorq	%r10,%r10
 	xorq	%r9,%r9

@@ -340,7 +364,7 @@ sysretl_from_sys_call:
 	movl RSP-ARGOFFSET(%rsp),%esp
 	CFI_RESTORE rsp
 	USERGS_SYSRET32
-	
+
 #ifdef CONFIG_AUDITSYSCALL
 cstar_auditsys:
 	CFI_RESTORE_STATE

@@ -358,6 +382,8 @@ cstar_tracesys:
 	testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags(%r10)
 	jz cstar_auditsys
 #endif
+
+	/* put Arg6 into %ebp where ptrace expects it */
 	xchgl %r9d,%ebp
 	SAVE_REST
 	CLEAR_RREGS 0, r9

@@ -366,21 +392,23 @@ cstar_tracesys:
 	call syscall_trace_enter
 	LOAD_ARGS32 ARGOFFSET, 1  /* reload args from stack in case ptrace changed it */
 	RESTORE_REST
+
+	/* sync back Arg6's possibly changed value where it is expected by C */
 	xchgl %ebp,%r9d
 	cmpq $(IA32_NR_syscalls-1),%rax
 	ja int_ret_from_sys_call /* cstar_tracesys has set RAX(%rsp) */
 	jmp cstar_do_call
 END(ia32_cstar_target)
-				
+
 ia32_badarg:
 	movq $-EFAULT,%rax
 	jmp ia32_sysret
 	CFI_ENDPROC
 
-/* 
- * Emulated IA32 system calls via int 0x80. 
+/*
+ * Emulated IA32 system calls via int 0x80.
  *
- * Arguments:	 
+ * Arguments:
  * %eax	System call number.
  * %ebx Arg1
  * %ecx Arg2

@@ -390,13 +418,13 @@ ia32_badarg:
  * %ebp Arg6    [note: not saved in the stack frame, should not be touched]
  *
  * Notes:
- * Uses the same stack frame as the x86-64 version.	
+ * Uses the same stack frame as the x86-64 version.
  * All registers except %eax must be saved (but ptrace may violate that)
  * Arguments are zero extended. For system calls that want sign extension and
  * take long arguments a wrapper is needed. Most calls can just be called
  * directly.
- * Assumes it is only called from user space and entered with interrupts off.	
- */ 				
+ * Assumes it is only called from user space and entered with interrupts off.
+ */
 
 ENTRY(ia32_syscall)
 	CFI_STARTPROC32	simple

@@ -433,9 +461,9 @@ ia32_sysret:
 	movq %rax,RAX-ARGOFFSET(%rsp)
 ia32_ret_from_sys_call:
 	CLEAR_RREGS -ARGOFFSET
-	jmp int_ret_from_sys_call 
+	jmp int_ret_from_sys_call
 
-ia32_tracesys:			 
+ia32_tracesys:
 	SAVE_REST
 	CLEAR_RREGS
 	movq $-ENOSYS,RAX(%rsp)	/* ptrace can change this for a bad syscall */

@@ -457,13 +485,13 @@ quiet_ni_syscall:
 	movq $-ENOSYS,%rax
 	ret
 	CFI_ENDPROC
-	
+
 	.macro PTREGSCALL label, func, arg
 	.globl \label
 \label:
 	leaq \func(%rip),%rax
 	leaq -ARGOFFSET+8(%rsp),\arg	/* 8 for return address */
-	jmp  ia32_ptregs_common	
+	jmp  ia32_ptregs_common
 	.endm
 
 	CFI_STARTPROC32

@@ -537,7 +565,7 @@ ia32_sys_call_table:
 	.quad quiet_ni_syscall	/* old stty syscall holder */
 	.quad quiet_ni_syscall	/* old gtty syscall holder */
 	.quad sys_access
-	.quad sys_nice	
+	.quad sys_nice
 	.quad quiet_ni_syscall	/* 35 */	/* old ftime syscall holder */
 	.quad sys_sync
 	.quad sys32_kill

@@ -616,7 +644,7 @@ ia32_sys_call_table:
 	.quad stub32_iopl		/* 110 */
 	.quad sys_vhangup
 	.quad quiet_ni_syscall	/* old "idle" system call */
-	.quad sys32_vm86_warning	/* vm86old */ 
+	.quad sys32_vm86_warning	/* vm86old */
 	.quad compat_sys_wait4
 	.quad sys_swapoff		/* 115 */
 	.quad compat_sys_sysinfo

@@ -669,7 +697,7 @@ ia32_sys_call_table:
 	.quad sys_mremap
 	.quad sys_setresuid16
 	.quad sys_getresuid16	/* 165 */
-	.quad sys32_vm86_warning	/* vm86 */ 
+	.quad sys32_vm86_warning	/* vm86 */
 	.quad quiet_ni_syscall	/* query_module */
 	.quad sys_poll
 	.quad compat_sys_nfsservctl

@@ -724,10 +752,10 @@ ia32_sys_call_table:
 	.quad sys_mincore
 	.quad sys_madvise
 	.quad compat_sys_getdents64	/* 220 getdents64 */
-	.quad compat_sys_fcntl64	
+	.quad compat_sys_fcntl64
 	.quad quiet_ni_syscall		/* tux */
-	.quad quiet_ni_syscall    	/* security */
-	.quad sys_gettid	
+	.quad quiet_ni_syscall		/* security */
+	.quad sys_gettid
 	.quad sys32_readahead	/* 225 */
 	.quad sys_setxattr
 	.quad sys_lsetxattr

@@ -742,7 +770,7 @@ ia32_sys_call_table:
 	.quad sys_lremovexattr
 	.quad sys_fremovexattr
 	.quad sys_tkill
-	.quad sys_sendfile64 
+	.quad sys_sendfile64
 	.quad compat_sys_futex		/* 240 */
 	.quad compat_sys_sched_setaffinity
 	.quad compat_sys_sched_getaffinity

@@ -754,7 +782,7 @@ ia32_sys_call_table:
 	.quad compat_sys_io_submit
 	.quad sys_io_cancel
 	.quad sys32_fadvise64		/* 250 */
-	.quad quiet_ni_syscall 	/* free_huge_pages */
+	.quad quiet_ni_syscall	/* free_huge_pages */
 	.quad sys_exit_group
 	.quad sys32_lookup_dcookie
 	.quad sys_epoll_create

index 6419bb0..9569f11 100644
@@ -607,10 +607,16 @@ tracesys:
 GLOBAL(int_ret_from_sys_call)
 	DISABLE_INTERRUPTS(CLBR_NONE)
 	TRACE_IRQS_OFF
+
+	/*
+	 * check the Requestor Privilege Level of the CS selector
+	 * previously pushed on the stack. If 0, we're returning
+	 * to kernel space.
+	 */
 	testl $3,CS-ARGOFFSET(%rsp)
 	je retint_restore_args
-	movl $_TIF_ALLWORK_MASK,%edi
 	/* edi:	mask to check */
+	movl $_TIF_ALLWORK_MASK,%edi
 GLOBAL(int_with_check)
 	LOCKDEP_SYS_EXIT_IRQ
 	GET_THREAD_INFO(%rcx)

@@ -618,11 +624,16 @@ GLOBAL(int_with_check)
 	andl %edi,%edx
 	jnz   int_careful
 	andl    $~TS_COMPAT,TI_status(%rcx)
+
+	/* no work pending, return to userspace */
 	jmp   retint_swapgs
 
-	/* Either reschedule or signal or syscall exit tracking needed. */
-	/* First do a reschedule test. */
-	/* edx:	work, edi: workmask */
+	/*
+	 * Either reschedule or signal or syscall exit tracking
+	 * needed. First do a reschedule test.
+	 *
+	 * edx:	work, edi: workmask
+	 */
 int_careful:
 	bt $TIF_NEED_RESCHED,%edx
 	jnc  int_very_careful

1.7.4


-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

I don't think that removing SYSCALL from 32-bit code just so that UML trapped syscalls work is something we'd like since SYSCALL is much cheaper than INT $0x80:

"As a result, SYSCALL and SYSRET can take fewer than one-fourth the number of internal clock cycles to complete than the legacy CALL and RET instructions."

http://support.amd.com/us/Processor_TechDocs/24593.pdf, p. 152.

I know, it is 32-bit syscall on 64-bit kernel which should be pretty rare but still...

Thanks.

Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Oh yes.

System call performance is *important*. And x86 is *important*.

UML? In comparison, not that important.

So quite frankly, if this is purely an UML issue (and unless we're missing something else, that's what it looks like, despite all the confusion we've had so far), then if we have a choice between "remove syscall instruction" and "remove UML", then ...

Linus

Agreed. Note, BTW, that UML has perfectly usable workaround for 99% of that - don't tell the binaries it has *any* vdso in such cases. And "remove UML" turns into "remove support under UML for 32bit binaries that go out of their way to do SYSCALL directly, which wouldn't work on native 32bit", which is really a no-brainer.

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Am 23.08.2011 18:53, schrieb Al Viro:

What about this hack/solution?
While booting UML can check whether the host's vDSO contains a SYSCALL instruction.
If so, UML will not make the host's vDSO available to it's processes...

Thanks,
//richard

Note that this is *only* for 32bit side of things. 64bit one works fine...

I wouldn't search for SYSCALL in vdso, BTW - not when we have a good way to trigger that crap and recognize it.

At boot time, fork a child. Have it traced with PTRACE_SYSCALL. Let it put recognizable values in registers and call __kernel_vsyscall(). Then let the parent do one more PTRACE_SYSCALL, then PTRACE_POKEUSER and set ebp to 0x69696969. PTRACE_CONT the sucker and let it report what it sees in ecx. If it's what we'd put there - fine, it looks safe. If it's 0x69696969 - we have a problem, no vdso for us.

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Am 23.08.2011 19:07, schrieb Al Viro:

I know. :)

Okay, this is a much cleaner approach.
But first I've to find a machine where I can test the issue. At home none on of my x86_64 machines is SYSCALL-based. Tomorrow I'll search at the university for one...

Thanks,
//richard

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Am 23.08.2011 19:07, schrieb Al Viro:

BTW: IMHO we can completely disable the vDSO for 32bit guests. I did some benchmarks, there is no performance gain at all within UML.

The attached program runs some syscalls for 10 seconds and prints the number of iterations.

Some results (5 runs on my Intel Core2):
vdso: 360099 362057 365982 367132 368907
none: 344195 355759 358974 366630 420027

Thanks,
//richard

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <sys/types.h>
#include <sys/stat.h>

static unsigned long long iter;

static void done(int sig)

{
	printf("%llu\n", iter);
	exit(0);
}

int main()

{
	signal(SIGALRM, done);
	alarm(10);

	for(;;){
		close(dup(0));
		getpid();
		getuid();
		umask(022);
		iter++;
	}
	return 0;
}

Right, but if you had said the difference had disappeared on current AMD silicon it would be much less of an issue. That's why I wanted to find that bit out from you.

-hpa

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Yeah, I dug this out in the APM, btw.

But no, I don't think the difference has disappeared - to the contrary, AFAICT, the intention is for SYSCALL to be the fastest way to do syscalls on x86 due to diminished number of segment checks etc. INT80 is legacy, slower, etc. I believe Andy measured a similar situation on Sandy Bridge with SYSCALL having latencies in the tens of nsecs range and INT80 being much slower. Ingo also measured a similar situation where the latency gap between the two on Intel is even bigger.

So, the only problem left now is what we're going
to do with cases similar to what Al conjured up:
http://marc.info/?l=linux-kernel&m=131412271112461&w=2

I don't know, maybe the most cowardly approach is to issue a warning and shrug with the shoulders, or do some asm magic... Also, do we care at all, how relevant is a case like that?

Hmmm.

Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Sandy Bridge doesn't have SYSCALL32 at all. It has SYSENTER and SYSCALL64.

-hpa

Yeah, I was talking about SYSCALL in general.

Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Right, but there could be an arbitrary chasm between SYSCALL32 and SYSCALL64.

-hpa

------------------------------------------------------------------------------
EMC VNX: the world's simplest storage, starting under $10K The only unified storage solution that offers unified management Up to 160% more powerful than alternatives and 25% more efficient. Guaranteed. http://p.sf.net/sfu/emc-vnx-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

By the way, Borislav;

any way you could nudge your hardware people into

a) supporting SYSENTER in compatibility mode, and
b) giving us a way to turn SYSCALL *off* in compat mode?

... for future chips?

-hpa

------------------------------------------------------------------------------
EMC VNX: the world's simplest storage, starting under $10K The only unified storage solution that offers unified management Up to 160% more powerful than alternatives and 25% more efficient. Guaranteed. http://p.sf.net/sfu/emc-vnx-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Not enough. There is also a PITA with signal handlers. There we can't avoid modifying ebp on the way out of handler (i.e. by emulated sigreturn). And it lands us straight after syscall insn, with ebp "restored" to the wrong value.

Um... How would it know which syscall variant had that been, to start with? For int 0x80 it would need to use registers as-is. For SYSENTER it also could use them as-is - ebp will differ from what we put there when entering the sucker, but not critically so; on the way out of syscall we'll overwrite it anyway immediately (either by pop or mov). For SYSCALL... we don't really care about ecx contents prior to entering the kernel (and it'll be blown out anyway), and ebp one could be found in regs.ecx. So yes, we can do it that way, but... how to tell what variant had been triggered? Examining two bytes prior to user eip? Sounds bloody brittle...

Just read the instruction, for chissake.

UML *already* does that, to see if it's "int80" or "sysenter" ('is_syscall()').

Now, I do agree that if we had designed the ptrace interface with these kinds of issues in mind, then we would have added a "state" field to the thing that could have this kind of information as part of the GETREGS interface. There is no question that that would have been a good idea - but we have what we have.

I mean, technically, we could also have always just given "raw user space register state" to ptrace, and then just said that "anybody who traces system calls needs to know the exact calling conventions for *that* kind of system call". But instead of that, we give the "cooked" pt_regs values on read-out, to make it simpler for strace and friends.

And it's actualyl simpler for UML too. If we *didn't* give that cooked register set information, then UML would *still* have to look at the actual instruction in order to emulate the system call correctly ("it's sysenter, so now I need to take some of the system call arguments from the stack"). So the fact that we do that register state swizzling actually helps not just strace, but UML too.

It would be *nice* if we did the swizzling automatically at setregs() time too, but we simply don't have enough information in the kernel to do that. Again, exactly because pt_regs doesn't have a "state" variable, when user-space does the SETREGS call, we simply don't know whether we are in "normal" code or in some system call entry or exit state. So the kernel does the swizzling at GETREGS time (by virtue of always having the registers in a "canonical" state for system call entry), but we fundamentally *cannot* to do the unswizzle, because we don't know what the SETREGS caller actually did.

So I think the current state is actually the best we could possibly do, with the caveat that *if* we had known about the "different system calls have different register layouts" originally and had thought of it, we could have added a 'state' word that the kernel could set at GETREGS time, and use at SETREGS time to decide whether swizzling is needed or not.

But not only would that have required time travel (ptrace existed before the multiple system calls did), even then it's not 100% clear that the current simpler model (with the admittedly subtle case of implicit state and its effect on register state) isn't actually the better solution. *Somebody* has to do the register swizzling, and the current "kernel canonicalizes registers at read time, you need to swizzle them if you change state" may simply be the RightThing(tm).

Linus

Again, can we steal one of the padding fields to use for that state variable? We have two 16-bit padding fields; one for cs and one for ss.

For UML, I agree, let's just not expose the vdso assuming that is possible, but for other -- possibly future -- users.

-hpa

------------------------------------------------------------------------------
EMC VNX: the world's simplest storage, starting under $10K The only unified storage solution that offers unified management Up to 160% more powerful than alternatives and 25% more efficient. Guaranteed. http://p.sf.net/sfu/emc-vnx-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

We can steal them for passing the information to the user, but no, I don't think we can use them to then take the information *from* the user.

Somebody may well be setting up a 'pt_regs' structure on his own, and simply not fill in the padding, resulting in random data in those fields.

Linus

That would be fine, I'd think... just gives the user space application enough information to know how it would have to reshuffle the registers if it needs to.

-hpa

------------------------------------------------------------------------------
EMC VNX: the world's simplest storage, starting under $10K The only unified storage solution that offers unified management Up to 160% more powerful than alternatives and 25% more efficient. Guaranteed. http://p.sf.net/sfu/emc-vnx-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

We could drop that information in a metaregister. It's not backward compatible, but at least it will be obvious when that information is available and not.

-hpa

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Well, seriously, UML already looks at the word at "ip-2" for other reasons. So it isn't like there is any point in adding more support to just give you that information in another form.

Linus

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Not for UML, but it might be useful for other things. Certainly lowers the priority, though.

-hpa

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

That is done only for task singlestepped in the guest:
/*
* This closes a way to execute a system call on the host. If
* you set a breakpoint on a system call instruction and singlestep
* from it, the tracing thread used to PTRACE_SINGLESTEP the process
* rather than PTRACE_SYSCALL it, allowing the system call to execute
* on the host. The tracing thread will check this flag and
* PTRACE_SYSCALL if necessary.
*/
if (current->ptrace & PT_DTRACE)
current->thread.singlestep_syscall =
is_syscall(PT_REGS_IP(&current->thread.regs));
with PT_DTRACE set by uml user_enable_single_step()

And it's not cheap - doing that on each syscall will be unpleasant... Frankly, I'd rather stopped telling the uml userland about vdso in such setups. And anything that plays with SYSCALL outside of vdso... <shrug> we already have a "don't run it native on 32bit", adding "don't run it on 32bit uml on amd64 host" is not too serious. At least for now...

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

I do agree that the solution might well be to just stop using the non-int80 vdsos for UML. That may just solve everything in practice.

Linus

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

SYSENTER works fine, actually... And we can easily check if we have an affected SYSCALL, simply by forking a child, tracing it into a syscall and doing POKEUSER to ebp on the second stop (i.e. on the way out). If the value ends up in ecx after __kernel_vsyscall(), we have SYSCALL-based variant on amd64 host, if it's lost completely - it's SYSENTER, if it shows up in ebp - int 0x80.

------------------------------------------------------------------------------
EMC VNX: the world's simplest storage, starting under $10K The only unified storage solution that offers unified management Up to 160% more powerful than alternatives and 25% more efficient. Guaranteed. http://p.sf.net/sfu/emc-vnx-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

apparently

I suspect that very few things care whether syscall arguments get clobbered. The only way it would matter is if gcc reuses the argument in the ecx slot after an inlined syscall later in the same function. Any code that does that is already wrong if the syscall restarts with changed ecx or if something like UML changes the syscall argument.

--Andy

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2

_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

No, the glibc wrapper for the system call Al was looking at used %ecx to hold a copy of the PIC pointer (normally %ebx)!

-hpa

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

There's a funny part, though - such binary won't work on 32bit kernel. AFAICS, we never set MSR_*STAR on 32bit kernels (and native 32bit vdso doesn't provide a SYSCALL-based variant).

So if we really consider such SYSCALL outside of vdso32 kosher, shouldn't we do something with entry_32.S as well? I don't think it's worth doing, TBH...

Again, I very much hope that binaries with such stray SYSCALL simply do not exist. In theory it's possible to write one, but...

IIRC, the reason we never had SYSCALL support in 32bit kernel was the utter lack of point - the *only* CPU where it would matter would be K6-2, IIRC, and (again, IIRC) it had some differences in SYSCALL semantics compared to K7 (which supports SYSENTER as well). Bugger if I remember what those differences might've been... Some flag not cleared?

The most likely reason for a binary to execute a stray SYSCALL is because they read it out of the vdso. Totally daft, but we certainly see a lot of stupid things as evidenced by the JIT thread earlier this month.

In that sense, a "safe" thing would be to drop use of SYSCALL for 32-bit processes... I just sent Borislav a query about the cost.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Um... What, blindly, no matter what surrounds it in there? What will happen to the same eager JIT when it steps on SYSENTER?

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

The JIT will have had to manage SYSENTER already. It's not a change, whereas SYSCALL would be. We could just try it, and see if anything breaks, of course.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Here's a possible solution that works for standalone SYSCALL and vdso SYSCALL. The idea is to preserve the exact same SYSCALL invocation sequence. Logically, the SYSCALL instruction does:

push %ebp
mov %ebp,%ecx
mov 4(%esp),%ebp
call __fake_int80

and __fake_int80 is:
int 0x80
mov 4(%esp),%ebp
ret $4

The entire system call sequence is then (effectively):

push %ebp
movl %ecx,%ebp

; "SYSCALL" starts here
push %ebp
mov %ebp,%ecx
mov 4(%esp),%ebp
call __fake_int80
; "SYSCALL ends here

movl %ebp,%ecx
popl %ebp
ret

So we rearrange ebp and ecx and then immediately rearrange them back. The landing point tweaks them again so that we preserve the old semantics of SYSCALL. But now the pt_regs values exactly match what would have happened if we entered via the int 0x80 path, so there shouldn't be any corner cases with ptrace or restart -- as far as either one is concerned, we actually entered via int 0x80. If we deliver a signal, the signal handler returns to the int 0x80 instruction.

Am I missing something? Extremely buggy, incomplete code that implements this is:

index a0e866d..6cda8ce 100644
@@ -291,24 +291,59 @@ ENTRY(ia32_cstar_target)
 	ENABLE_INTERRUPTS(CLBR_NONE)
 	SAVE_ARGS 8,0,0
 	movl 	%eax,%eax	/* zero extension */
-	movq	%rax,ORIG_RAX-ARGOFFSET(%rsp)
-	movq	%rcx,RIP-ARGOFFSET(%rsp)
-	CFI_REL_OFFSET rip,RIP-ARGOFFSET
-	movq	%rbp,RCX-ARGOFFSET(%rsp) /* this lies slightly to ptrace */
-	movl	%ebp,%ecx
+
+	/*
+	 * This does (from the user's point of view):
+	 * push %ebp
+	 * mov %ebp, %ecx
+	 * mov 4(%esp), %ebp
+	 * call <function that does int 0x80; mov 4(%esp),%ebp; ret 4>
+	 *
+	 * User address access does not need access_ok check as r8
+	 * has been zero-extended, so even with the offsets it cannot
+	 * exceed 2**32 + 8.
+	 */
+
+	/* XXX: need to check that vdso actually exists. */
+	/* XXX: ia32_badarg may do bad things to the user state. */
+
+	/* move ebp into place on the user stack */
+	1:	movl	%ebp,-4(%r8)
+	.section __ex_table,"a"
+	.quad 1b,ia32_badarg
+	.previous
+
+	/* move eip into place on the user stack */
+	1:	movl	%ecx,-8(%r8)  /* user eip is in ecx */
+	.section __ex_table,"a"
+	.quad 1b,ia32_badarg
+	.previous
+
+	/* move ebp to ecx in CPU registers and argument save area */
+	mov %ebp,%ecx
+	movq %ecx,RCX-ARGOFFSET(%rsp)
+
+	/*
+	 * move arg6 to ebp in CPU registers and argument save area
+	 * minor optimization: the actual value of ebp is irrelevent,
+	 * so stick it straight into r9d -- see the definition of
+	 * IA32_ARG_FIXUP.
+	 */
+1:	movl	(%r8),%r9d
+	.section __ex_table,"a"
+	.quad 1b,ia32_badarg
+	.previous	
+
+	/* Do the fake call */
+	movl [insert address of int 0x80; ret helper + 2 here],RIP-ARGOFFSET(%rsp)
+	subl $8,%r8 /* we pushed twice */
+
 	movq	$__USER32_CS,CS-ARGOFFSET(%rsp)
 	movq	$__USER32_DS,SS-ARGOFFSET(%rsp)
 	movq	%r11,EFLAGS-ARGOFFSET(%rsp)
 	/*CFI_REL_OFFSET rflags,EFLAGS-ARGOFFSET*/
 	movq	%r8,RSP-ARGOFFSET(%rsp)	
 	CFI_REL_OFFSET rsp,RSP-ARGOFFSET
-	/* no need to do an access_ok check here because r8 has been
-	   32bit zero extended */ 
-	/* hardware stack frame is complete now */	
-1:	movl	(%r8),%r9d
-	.section __ex_table,"a"
-	.quad 1b,ia32_badarg
-	.previous	
 	GET_THREAD_INFO(%r10)
 	orl   $TS_COMPAT,TI_status(%r10)
 	testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags(%r10)

index 5415b56..a3e48b0 100644
@@ -19,8 +19,8 @@ __kernel_vsyscall:
 .Lpush_ebp:
 	movl	%ecx, %ebp
 	syscall
-	movl	$__USER32_DS, %ecx
-	movl	%ecx, %ss
+	/* The ret in the fake int80 entry lands here */
+	/* ss is already correct AFAICS */
 	movl	%ebp, %ecx
 	popl	%ebp
 .Lpop_ebp:

@@ -28,6 +28,11 @@ __kernel_vsyscall:
 .LEND_vsyscall:
 	.size __kernel_vsyscall,.-.LSTART_vsyscall
 
+__kernel_vsyscall_fake_int80:
+	int 0x80
+	mov 4(%esp),%ebp
+	ret $4
+
 	.section .eh_frame,"a",@progbits
 .LSTARTFRAME:
 	.long .LENDCIE-.LSTARTCIE

This could be further simplified by checking if any work flags are set and bailing immediately to the right place in the int 0x80 entry.

--Andy

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel