svn commit: r245486 - in user/sbruno/pxestuff/sys/boot/i386: . btx/btx btx/lib libi386 pxehttp

Thu Jan 17 22:30:56 UTC 2013

On Wed, Jan 16, 2013 at 01:49:08AM +0000, Sean Bruno wrote:
> Author: sbruno
> Date: Wed Jan 16 01:49:07 2013
> New Revision: 245486
> URL: http://svnweb.freebsd.org/changeset/base/245486

> Log:
>   First day of work on pxe_http restoral, merge in some janky assembly that
>   better get some serious reviews from people whom are way smarter than I.

> Added:
>   user/sbruno/pxestuff/sys/boot/i386/pxehttp/
> [snip]
> Modified:
>   user/sbruno/pxestuff/sys/boot/i386/Makefile
>   user/sbruno/pxestuff/sys/boot/i386/btx/btx/btx.S
>   user/sbruno/pxestuff/sys/boot/i386/btx/lib/btxsys.s
>   user/sbruno/pxestuff/sys/boot/i386/btx/lib/btxv86.h
>   user/sbruno/pxestuff/sys/boot/i386/libi386/Makefile

> [snip]
> Modified: user/sbruno/pxestuff/sys/boot/i386/btx/btx/btx.S
> ==============================================================================
> --- user/sbruno/pxestuff/sys/boot/i386/btx/btx/btx.S	Wed Jan 16 01:30:46 2013	(r245485)
> +++ user/sbruno/pxestuff/sys/boot/i386/btx/btx/btx.S	Wed Jan 16 01:49:07 2013	(r245486)
> @@ -52,7 +52,8 @@
>  		.set SEL_RDATA,0x20		# Real mode data
>  		.set SEL_UCODE,0x28|3		# User code
>  		.set SEL_UDATA,0x30|3		# User data
> -		.set SEL_TSS,0x38		# TSS
> +		.set SEL_TSS,0x38				# TSS
> +		.set SEL_CALLGATE,0x40	# super2user callgate

This cannot work, see below.

>  /*
>   * Task state segment fields.
>   */
> @@ -62,8 +63,9 @@
>  /*
>   * System calls.
>   */
> -		.set SYS_EXIT,0x0		# Exit
> -		.set SYS_EXEC,0x1		# Exec
> +		.set SYS_EXIT,0x0					# Exit
> +		.set SYS_EXEC,0x1					# Exec
> +		.set SYS_ISR_INSTALL,0x2	# ISR_install
>  /*
>   * Fields in V86 interface structure.
>   */
> @@ -366,7 +368,6 @@ except.2:	pushl 0x50(%esp,1)		# Set ESP
>  		popl %es			#  data
>  		movl %esp,%ebx			# Stack frame
>  		movl $dmpfmt,%esi		# Dump format string
> -		movl $MEM_BUF,%edi		# Buffer
>  		pushl %edi			# Dump to
>  		call dump			#  buffer
>  		popl %esi			#  and

Is this deletion deliberate? It seems to cause the crash message to be
written to whatever address happens to be in %edi instead of to the
buffer reserved for that purpose.

> @@ -690,7 +691,9 @@ rret_tramp.3:	popl %es			# Restore
>  /*
>   * System Call.
>   */
> -intx30: 	cmpl $SYS_EXEC,%eax		# Exec system call?
> +intx30:     cmpl $SYS_ISR_INSTALL, %eax # is isr_install?
> +		je intx30.2         #  yes
> +		cmpl $SYS_EXEC,%eax     # Exec system call?
>  		jne intx30.1			# No
>  		pushl %ss			# Set up
>  		popl %es			#  all
> @@ -708,6 +711,74 @@ intx30: 	cmpl $SYS_EXEC,%eax		# Exec sys
>  intx30.1:	orb $0x1,%ss:btx_hdr+0x7	# Flag reboot
>  		jmp exit			# Exit
>  /*
> + *	Here we need to modify IDT in such way, that at interrupt handle
> + *  will be run isr_trump, which role is to run provided function
> + *  in user space.
> + */
> +intx30.2:
> +		cli
> +		pushl %edi
> +		pushl %ebx

> +		pushw %ds
> +		pushw %dx

It looks a bit too clever to use the 16-bit operand size here. The risk
of accidentally modifying the top 16 bits of %edx and the two additional
bytes of code are more important than the four bytes of stack space.

> +
> +		pushl %ss			# Set up
> +		popl %ds			#  registers
> +		
> +
> + 		movl $MEM_USR,%ebx		# User base address
> +		addl 0x18(%esp,1),%ebx		# getting user stack head
> +		addl $0x04, %ebx		#  first parameter
> +
> +		movl (%ebx), %eax

> +		movw 0x2(%ebx), %dx
> +		xchgw %dx, %bx

I do not understand these two instructions. They appear to set %dx to
the high 16 bits of the function pointer in a roundabout way, but
nothing is done with that subsequently.

> +
> +/*
> + * updating call gate
> + */
> +		movl $callgate, %edi

You can load the address in a register but it does not appear to save
any code bytes here.

> +		movw %ax, (%edi)		# +0: store offset 00..15
> +		shr $0x10 ,%eax			# getting high word
> +		movw %ax, 0x06(%edi)		# +6: handler offset 16..31
> +/*
> + * installing handler
> + */
> +/*
> + *  NOTE: it seems nothing else must be done
> + */
> +		popw %dx
> +		popw %ds
> +		popl %ebx		
> +		popl %edi
> +		sti	
> +		iret				# return from syscall
> +
> +user_isr_call:
> +/*
> + * NOTE: isr must use lret to return and restore SS, ESP, CS, EIP.

This is in fact iret, not lret.

> +*/
> +		pushl %ds			# saving ds
> +		pushl %edi
> +		movl $SEL_SDATA, %eax		#

You're overwriting %eax without saving and restoring it.

> +		movl %eax, %ds			
> +						
> +		movl $callgate, %edi
> +		movw (%edi), %ax

You can move from callgate directly; there is no need to load the
address into a register first.

> +		popl %edi		
> +
> +		cmpw $0x0000, %ax

testw %ax, %ax (or cleverly testl %ax, %ax to save another byte)

> +		je isr_ret
> +		
> +		lcall $SEL_CALLGATE,$0x00000000 # far call via callgate selector
> +			# offset is ignored

You cannot call to a less privileged code segment, not even via a call
gate. This is because the return would have to allow the less privileged
code segment to "return" to any code segment and offset.

It may work to execute the user code in ring 0. This requires a new code
segment descriptor with MEM_USR base. The call gate is not necessary
because a regular far call can be used. Make sure to adjust the
exception handler so it checks the privilege level of the crashing code
properly instead of assuming that only SEL_SCODE is ring 0.

Alternatively, jump to the user code using iret and have it call another
system call to resume.

> +isr_ret:
> +		
> +
> +		popl %ds
> +
> +		iret				# return from interrupt handler
> +/*
>   * Dump structure [EBX] to [EDI], using format string [ESI].
>   */
>  dump.0: 	stosb				# Save char
> @@ -1000,6 +1071,7 @@ gdt:		.word 0x0,0x0,0x0,0x0		# Null entr
>  		.word 0xffff,MEM_USR,0xfa00,0xcf# SEL_UCODE
>  		.word 0xffff,MEM_USR,0xf200,0xcf# SEL_UDATA
>  tss_desc:	.word _TSSLM,MEM_TSS,0x8900,0x0 # SEL_TSS
> +callgate:	.word 0x0, SEL_UCODE,0xec00,0x0 # SEL_CALLGATE
>  gdt.1:
>  /*
>   * Pseudo-descriptors.
> 
> Modified: user/sbruno/pxestuff/sys/boot/i386/btx/lib/btxsys.s
> ==============================================================================
> --- user/sbruno/pxestuff/sys/boot/i386/btx/lib/btxsys.s	Wed Jan 16 01:30:46 2013	(r245485)
> +++ user/sbruno/pxestuff/sys/boot/i386/btx/lib/btxsys.s	Wed Jan 16 01:49:07 2013	(r245486)
> @@ -24,6 +24,7 @@
>  #
>  		.global __exit
>  		.global __exec
> +		.global __isr_install
>  #
>  # Constants.
>  #
> @@ -38,3 +39,8 @@ __exit: 	xorl %eax,%eax			# BTX system
>  #
>  __exec: 	movl $0x1,%eax			# BTX system
>  		int $INT_SYS			#  call 0x1
> +#
> +# System call: isr_install
> +#
> +__isr_install: 	movl $0x2,%eax			# BTX system
> +		int $INT_SYS			#  call 0x2

A ret instruction is probably required here because the new system call
returns (different from the existing two).

-- 
Jilles Tjoelker