[PATCH] extboot: don't use interrupt vectors $0x2b and $0x2c

[PATCH] extboot: don't use interrupt vectors $0x2b and $0x2c

[H. Peter Anvin]

extboot's use of interrupt vectors $0x2b and $0x2c is unsafe, as these
interrupt vectors fall in the OS-use range (0x20-0x3f).  Furthermore,
it's unnecessary: we can keep a local pointer instead of hooking
another interrupt as long as we can write to our own segment.

Make the extboot segment writable, and use local variables to hold the
old link pointers.

If this turns out to cause problems, we should probably switch to
using vectors in the 0xc0-0xef range, and/or other BIOS-reserved
memory.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 extboot/extboot.S |   69 +++++++++++++++++++++-------------------------------
 qemu/hw/pc.c      |   16 +++++++----
 2 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/extboot/extboot.S b/extboot/extboot.S
index 9eb9333..2630abb 100644
--- a/extboot/extboot.S
+++ b/extboot/extboot.S
@@ -25,45 +25,36 @@
 _start:
 	.short 0xaa55
 	.byte (_end - _start) / 512
-	push %ax
-	push %bx
-	push %cx
-	push %dx
+	push %eax
 	push %ds
 
 	/* setup ds so we can access the IVT */
 	xor %ax, %ax
 	mov %ax, %ds
 
-	/* save old int 19 at int 2b */
-	mov $(0x19 * 4), %bx
-	mov 0(%bx), %ax
-	mov 2(%bx), %cx
-
-	mov $(0x2b * 4), %bx
-	mov %ax, 0(%bx)
-	mov %cx, 2(%bx)
+	/* save old int 19 */
+	mov (0x19*4), %eax
+	mov %eax, %cs:old_int19
 
 	/* install out int 19 handler */
-	mov $(0x19 * 4), %bx
-	mov $int19_handler, %ax
-	mov %ax, 0(%bx)
-	mov %cs, 2(%bx)
+	movw $int19_handler, (0x19*4)
+	mov %cs, (0x19*4+2)
 
 	pop %ds
-	pop %dx
-	pop %cx
-	pop %bx
-	pop %ax
+	pop %eax
 	lret
 
 int19_handler:
-	push %ax
+	push %eax
 	push %bx
 	push %cx
 	push %dx
 	push %ds
 
+	/* setup ds to access IVT */
+	xor %ax, %ax
+	mov %ax, %ds
+
 	movw $0x404, %dx
 	inb %dx, %al
 	cmp $1, %al
@@ -73,25 +64,13 @@ int19_handler:
 	jmp 3f
 
 1: /* hook int13: intb(0x404) == 1 */
-	/* setup ds to access IVT */
-	xor %ax, %ax
-	mov %ax, %ds
-
 	/* save old int 13 to int 2c */
-	mov $(0x13 * 4), %bx
-	mov 0(%bx), %ax
-	mov 2(%bx), %cx
-
-	mov $(0x2c * 4), %bx
-	mov %ax, 0(%bx)
-	mov %cx, 2(%bx)
+	mov (0x13*4), %eax
+	mov %eax, %cs:old_int13
 
 	/* install our int 13 handler */
-	mov $(0x13 * 4), %bx
-	mov $int13_handler, %ax
-
-	mov %ax, 0(%bx)
-	mov %cs, 2(%bx)
+	movw $int13_handler, (0x13*4)
+	mov %cs, (0x13*4+2)
 	jmp 3f
 
 2: /* linux boot: intb(0x404) == 2 */
@@ -107,12 +86,16 @@ int19_handler:
 	ljmp $0x9000 + 0x20, $0
 
 3: /* fall through: inb(0x404) == 0 */
+	/* restore previous int $0x19 handler */
+	mov %cs:old_int19,%eax
+	mov %eax,(0x19*4)
+	
 	pop %ds
 	pop %dx
 	pop %cx
 	pop %bx
-	pop %ax
-	int $0x2b
+	pop %eax
+	ljmpw *%cs:old_int19
 
 #define FLAGS_CF	0x01
 
@@ -640,8 +623,7 @@ terminate_disk_emulation:
 int13_handler:
 	cmp $0x80, %dl
 	je 1f
-	int $0x2c
-	iret
+	ljmpw *%cs:old_int13
 1:
 	cmp $0x0, %ah
 	jne 1f
@@ -701,5 +683,10 @@ int13_handler:
 	int $0x18  /* boot failed */
 	iret
 
+/* Variables */
+.align 4, 0
+old_int13:	.long 0
+old_int19:	.long 0
+	
 .align 512, 0
 _end:
diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
index d578e68..5072e72 100644
--- a/qemu/hw/pc.c
+++ b/qemu/hw/pc.c
@@ -718,7 +718,7 @@ static void pc_init_ne2k_isa(NICInfo *nd, qemu_irq *pic)
     nb_ne2k++;
 }
 
-static int load_option_rom(const char *filename, int offset)
+static int load_option_rom(const char *filename, int offset, int type)
 {
     ram_addr_t option_rom_offset;
     int size, ret;
@@ -739,11 +739,10 @@ static int load_option_rom(const char *filename, int offset)
     }
     size = (size + 4095) & ~4095;
     cpu_register_physical_memory(0xd0000 + offset,
-				 size, option_rom_offset | IO_MEM_ROM);
+				 size, option_rom_offset | type);
     if (kvm_enabled())
 	    kvm_cpu_register_physical_memory(0xd0000 + offset,
-                                             size, option_rom_offset |
-                                             IO_MEM_ROM);
+                                             size, option_rom_offset | type);
     return size;
 }
 
@@ -918,13 +917,18 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
                                          isa_bios_size,
                                          (bios_offset + bios_size - isa_bios_size) | IO_MEM_ROM);
 
+    /* XXX: for DDIM support, "ROM space" should be writable during
+       initialization, and (optionally) marked readonly by the BIOS
+       before INT 19h.  See the PNPBIOS specification, appendix B.
+       DDIM support is mandatory for proper PCI expansion ROM support. */
     opt_rom_offset = 0;
     for (i = 0; i < nb_option_roms; i++)
-	opt_rom_offset += load_option_rom(option_rom[i], opt_rom_offset);
+	opt_rom_offset += load_option_rom(option_rom[i], opt_rom_offset,
+					  IO_MEM_ROM);
 
     if (extboot_drive != -1) {
 	snprintf(buf, sizeof(buf), "%s/%s", bios_dir, EXTBOOT_FILENAME);
-	opt_rom_offset += load_option_rom(buf, opt_rom_offset);
+	opt_rom_offset += load_option_rom(buf, opt_rom_offset, IO_MEM_RAM);
     }
 
     /* map all the bios at the top of memory */
-- 
1.5.6.3

Re: [PATCH] extboot: don't use interrupt vectors $0x2b and $0x2c

[Avi Kivity]

H. Peter Anvin wrote:
> extboot's use of interrupt vectors $0x2b and $0x2c is unsafe, as these
> interrupt vectors fall in the OS-use range (0x20-0x3f).  Furthermore,
> it's unnecessary: we can keep a local pointer instead of hooking
> another interrupt as long as we can write to our own segment.
>
> Make the extboot segment writable, and use local variables to hold the
> old link pointers.
>
> If this turns out to cause problems, we should probably switch to
> using vectors in the 0xc0-0xef range, and/or other BIOS-reserved
> memory.
>
>   

Applied, thanks (as two patches).


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.