[BROKEN PATCH] syscalls leak data via registers?

[BROKEN PATCH] syscalls leak data via registers?

[Jeremy Fitzhardinge]

It looks to me like the syscall calling convention, on x86 at least,
leaks kernel data out via the registers.

In arch/i386/kernel/entry.S, system_call saves all the registers on the
stack and then calls the appropriate system call function.  The integer
registers are saved on the stack in the appropriate order for the C
calling convention, so ebx=arg1, ecx=arg2, etc.

On system call exit, it saves %eax (the return value) back the stack,
restores all those registers by popping them off the stack and returns
to user-mode.

The trouble is that gcc assumes it can reuse the stack space used by
arguments for spilling, so the values the return path restores are not
necessarily the ones it saved.

For example:

If you have sys_foo():

int sys_foo(int arg1, int arg2)
/* arg1 at 4(%esp) on entry */
/* arg2 at 8(%esp) on entry */
{
	/*... stuff ...*/
	arg1 = 77;
		movl	$77, %arg1
	
	/* compiler needs to spill arg1: */
		movl	%arg1, 4(%esp)

	/* ... */

	return 99;
}

When sys_foo returns, the value of %ebx has been changed to 77 on the
stack, so when it returns to user-mode, the whole world can see that
arg1 was assigned 77 at some point.

It seems to me the bug is in restoring the register values on return to
user-mode.  As I understand it, the x86 ABI says that the called
function owns the stack memory which contains the function's arguments,
so it is completely within gcc's right to reuse the memory as spill
space (or anything else) when generating code for that function. 
Therefore, the code in entry.S should not restore those values to
registers - it should just trash all the registers (except %eax, of
course) before returning.

I tried writing a patch which replaces the RESTORE_ALL with the
equivalent which simply skips %esp over the other registers, pops %eax
and then assigns it to %ebx-%ebp (it makes as good a trash value as
any), but this crashes when calibrating the delay loop.  Hm, looks like
the RESTORE_ALL on the syscall return path is also used by the interrupt
return path - that probably shouldn't trash registers.

Anyway, patch attached so you can see what I'm thinking.

	J

 arch/i386/kernel/entry.S |   47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 45 insertions(+), 2 deletions(-)

diff -puN arch/i386/kernel/entry.S~syscall-trash-regs arch/i386/kernel/entry.S
--- local-2.6/arch/i386/kernel/entry.S~syscall-trash-regs	2003-08-02 01:33:00.000000000 -0700
+++ local-2.6-jeremy/arch/i386/kernel/entry.S	2003-08-02 01:40:32.000000000 -0700
@@ -122,6 +122,33 @@ TSS_ESP0_OFFSET = (4 - 0x200)
 	popl %ebp;	\
 	popl %eax
 
+/* restore %eax, trash the rest */
+#define RESTORE_EAX		\
+	lea  6*4(%esp),%esp;	\
+	popl %eax;		\
+	movl %eax,%ebx;		\
+	movl %eax,%ecx;		\
+	movl %eax,%edx;		\
+	movl %eax,%esi;		\
+	movl %eax,%edi;		\
+	movl %eax,%ebp
+
+#define RESTORE_SOME_REGS	\
+	RESTORE_EAX;		\
+1:	popl %ds;		\
+2:	popl %es;		\
+.section .fixup,"ax";		\
+3:	movl $0,(%esp);		\
+	jmp 1b;			\
+4:	movl $0,(%esp);		\
+	jmp 2b;			\
+.previous;			\
+.section __ex_table,"a";	\
+	.align 4;		\
+	.long 1b,3b;		\
+	.long 2b,4b;		\
+.previous
+
 #define RESTORE_REGS	\
 	RESTORE_INT_REGS; \
 1:	popl %ds;	\
@@ -138,6 +165,22 @@ TSS_ESP0_OFFSET = (4 - 0x200)
 	.long 2b,4b;	\
 .previous
 
+#define RESTORE_MOST	\
+	RESTORE_SOME_REGS;	\
+	addl $4, %esp;	\
+1:	iret;		\
+.section .fixup,"ax";   \
+2:	sti;		\
+	movl $(__USER_DS), %edx; \
+	movl %edx, %ds; \
+	movl %edx, %es; \
+	pushl $11;	\
+	call do_exit;	\
+.previous;		\
+.section __ex_table,"a";\
+	.align 4;	\
+	.long 1b,2b;	\
+.previous
 
 #define RESTORE_ALL	\
 	RESTORE_REGS	\
@@ -324,8 +367,8 @@ restore_all:
         
 resume_kernelX:
 #endif
-	RESTORE_ALL
-
+	RESTORE_MOST
+	
 	# perform work that needs to be done immediately before resumption
 	ALIGN
 work_pending:

_

Re: [BROKEN PATCH] syscalls leak data via registers?

[Pavel Machek]

Hi!

> It looks to me like the syscall calling convention, on x86 at least,
> leaks kernel data out via the registers.
[scary stuff]
> When sys_foo returns, the value of %ebx has been changed to 77 on the
> stack, so when it returns to user-mode, the whole world can see that
> arg1 was assigned 77 at some point.
> 
> It seems to me the bug is in restoring the register values on return to
> user-mode.  As I understand it, the x86 ABI says that the called
> function owns the stack memory which contains the function's arguments,
> so it is completely within gcc's right to reuse the memory as spill
> space (or anything else) when generating code for that function. 
> Therefore, the code in entry.S should not restore those values to
> registers - it should just trash all the registers (except %eax, of
> course) before returning.
> 
> I tried writing a patch which replaces the RESTORE_ALL with the
> equivalent which simply skips %esp over the other registers, pops %eax
> and then assigns it to %ebx-%ebp (it makes as good a trash value as
> any), but this crashes when calibrating the delay loop.  Hm, looks like
> the RESTORE_ALL on the syscall return path is also used by the interrupt
> return path - that probably shouldn't trash registers.

I believe userspace depends on registers to be preserved over system
call, except for eax. So what you found is not only security problem,
but also crasher bug.
								Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [BROKEN PATCH] syscalls leak data via registers?

[Jeremy Fitzhardinge]

On Thu, 2003-08-07 at 03:30, Pavel Machek wrote:
> I believe userspace depends on registers to be preserved over system
> call, except for eax.

That's what I was wondering.  Does it?  Is that a documented part of the
syscall interface?

>  So what you found is not only security problem,
> but also crasher bug.

In these sense that it crashes userspace?

	J

Re: [BROKEN PATCH] syscalls leak data via registers?

[Richard B. Johnson]

On Thu, 7 Aug 2003, Jeremy Fitzhardinge wrote:

> On Thu, 2003-08-07 at 03:30, Pavel Machek wrote:
> > I believe userspace depends on registers to be preserved over system
> > call, except for eax.
>
> That's what I was wondering.  Does it?  Is that a documented part of the
> syscall interface?
>
> >  So what you found is not only security problem,
> > but also crasher bug.
>
> In these sense that it crashes userspace?
>
> 	J
>

The kernel interface preserves the registers that GCC needs
preserved, i.e., index registers such as EBX, ESI, and EDI.
It may not preserve registers that convey information for
the call unless they are index registers. For instance,
a system call that takes two parameters has those parameters
put into EBX and ECX. Register EAX always contains the
function number. In the case described, only EBX is
guaranteed to be preserved, this because it's an index
register.

Typically parameters are passed as:

	No parameters		EAX
	1 parameter		EAX EBX
	2 parameters		EAX EBX ECX
	3 parameters		EAX EBX ECX EDX
	4 parameters		EAX EBX ECX EDX ESI
	5 parameters		EAX EBX ECX EDX ESI EDI
	6 parameters		EAX EBX ECX EDX ESI EDI EBP

Upon return only the index registers plus segments and stack
will be preserved. The other registers can contain anything.
However, this is hardly an 'information' leak. Even if the
address of something in the kernel was returned, it can't
be accessed, and the 'information' is all about the methods
used to perform a function upon behalf of the caller, not
some other task. In other words, if you are reading from
a file, and some register contains 42, you only know that
that value was used somewhere while helping you get the
file data. It is never some data from somebody else's file.
To get to somebody else's data requires a context switch and
all registers and restored are saved across a context switch.
In Unix, the kernel performs functions on behalf of the caller,
within the context of the caller, so register information
is not an information leak.

But, there is the possibility of having data left in memory
by another task, accessed by the current task. These possibilities
are constantly reviewed and fixed when found.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.

Re: [BROKEN PATCH] syscalls leak data via registers?

[Pavel Machek]

Hi!

> > I believe userspace depends on registers to be preserved over system
> > call, except for eax.
> 
> That's what I was wondering.  Does it?  Is that a documented part of the
> syscall interface?
> 
> >  So what you found is not only security problem,
> > but also crasher bug.
> 
> In these sense that it crashes userspace?

Yes. But I was probably wrong. gcc has to preserve callee saved
registers, and that should be enough.

Information leak looks very much real, through.
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]