xorl %eax, %eax

CVE-2013-1798: Linux kernel KVM IOAPIC_REG_SELECT Invalid Memory Access

leave a comment »

This was very nice vulnerability reported by Andrew Honig of Google. The bug is triggered when a user specifies an invalid IOAPIC_REG_SELECT value which is reachable via read KVM I/O device operation as you can see below.

static int ioapic_mmio_read(struct kvm_io_device *this, gpa_t addr, int len,
			    void *val)
{
	struct kvm_ioapic *ioapic = to_ioapic(this);
	u32 result;
   ...
	switch (addr) {
	case IOAPIC_REG_SELECT:
		result = ioapic->ioregsel;
		break;

	case IOAPIC_REG_WINDOW:
		result = ioapic_read_indirect(ioapic, addr, len);
		break;
   ...
	return 0;
}
   ...
static const struct kvm_io_device_ops ioapic_mmio_ops = {
	.read     = ioapic_mmio_read,
	.write    = ioapic_mmio_write,
};

Additionally, if a user makes a read by invoking IOAPIC_REG_WINDOW it will result in calling ioapic_read_indirect(). Here is what this function does.

static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
					  unsigned long addr,
					  unsigned long length)
{
	unsigned long result = 0;

	switch (ioapic->ioregsel) {
  ...
	default:
		{
			u32 redir_index = (ioapic->ioregsel - 0x10) >> 1;
			u64 redir_content;

			ASSERT(redir_index < IOAPIC_NUM_PINS);

			redir_content = ioapic->redirtbl[redir_index].bits;
			result = (ioapic->ioregsel & 0x1) ?
			    (redir_content >> 32) & 0xffffffff :
			    redir_content & 0xffffffff;
			break;
		}
	}

	return result;
}

It calculates and initializes the value of ‘redir_index’ from the user controlled ‘ioapic->ioregsel’ variable and then uses it as an index to ‘ioapic->redirtbl[]’ array. If this value is larger than IOAPIC_NUM_PINS it will result in invalid memory access. Here is how IOAPIC_NUM_PINS is defined in virt/kvm/ioapic.h header file.

#define IOAPIC_NUM_PINS  KVM_IOAPIC_NUM_PINS

And this is because it is architecture specific. For IA64 is defined in include/uapi/asm/kvm.h as 48 and for x86 in arch/x86/include/uapi/asm/kvm.h as 24. As you might have noticed there is an ASSERT() call to make this check but of course, this will only take effect in the debug builds.
The fix was to replace that ASSERT() call with a range check like this.

 			u32 redir_index = (ioapic->ioregsel - 0x10) >> 1;
 			u64 redir_content;
-			ASSERT(redir_index < IOAPIC_NUM_PINS);
+			if (redir_index < IOAPIC_NUM_PINS)
+				redir_content =
+					ioapic->redirtbl[redir_index].bits;
+			else
+				redir_content = ~0ULL;
-			redir_content = ioapic->redirtbl[redir_index].bits;
 			result = (ioapic->ioregsel & 0x1) ?
 			    (redir_content >> 32) & 0xffffffff :

Written by xorl

May 23, 2013 at 22:44

Posted in bugs, linux

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s