xorl %eax, %eax

Linux kernel KVM CPUID IOCTL Integer Overflow

leave a comment »

While reading some mailing lists yesterday, I noticed this email of Eugene Teo to oss-security. As you can read in Eugene Teo’s email, this issue affects only 32-bit architectures on Linux kernel releases between 2.6.25-rc1 and 2.6.32-rc3. The following code is taken from 2.6.32-rc3 release of the Linux kernel.

static int kvm_dev_ioctl_get_supported_cpuid(struct kvm_cpuid2 *cpuid,
				     struct kvm_cpuid_entry2 __user *entries)
{
	struct kvm_cpuid_entry2 *cpuid_entries;
	int limit, nent = 0, r = -E2BIG;
	u32 func;

	if (cpuid->nent < 1)
		goto out;
	r = -ENOMEM;
	cpuid_entries = vmalloc(sizeof(struct kvm_cpuid_entry2) * cpuid->nent);
	if (!cpuid_entries)
		goto out;

	do_cpuid_ent(&cpuid_entries[0], 0, 0, &nent, cpuid->nent);
	limit = cpuid_entries[0].eax;
	for (func = 1; func <= limit && nent < cpuid->nent; ++func)
		do_cpuid_ent(&cpuid_entries[nent], func, 0,
			     &nent, cpuid->nent);
    ...
}

The first check that takes place is that the completely user controlled ‘cpuid->nent’ which represents the number of entries in CPUID is negative. In my opinion, this is redundant since ‘kvm_cpuid2’ structure defines this member as an unsigned 32-bit long integer as you can read from arch/x86/include/asm/kvm.h:

/* for KVM_SET_CPUID2 */
struct kvm_cpuid2 {
	__u32 nent;
	__u32 padding;
	struct kvm_cpuid_entry2 entries[0];
};

This means that it cannot contain negative numbers. So, after doing that -almost certainly- useless check, it uses vmalloc() to allocate enough space to fit the requested number of CPUID entries (which is stored in ‘cpuid->nent’). Since this value is completely user controlled and never checked against values that could result in integer overflow, this allocation can be controlled by the user. This means that you can overflow the result of:

sizeof(struct kvm_cpuid_entry2) * cpuid->nent

expression and this would lead to the allocation of a small/controlled portion of memory. However, the subsequent operations to the returned memory area pointed by ‘cpuid_entries’ use ‘cpuid->nent’ as a counter. This results in an also quite controlled memory corruption. Now, this IOCTL handler routine is reached through this function:

long kvm_arch_dev_ioctl(struct file *filp,
			unsigned int ioctl, unsigned long arg)
{
	void __user *argp = (void __user *)arg;
	long r;

	switch (ioctl) {
     ...
	case KVM_GET_SUPPORTED_CPUID: {
		struct kvm_cpuid2 __user *cpuid_arg = argp;
		struct kvm_cpuid2 cpuid;

		r = -EFAULT;
		if (copy_from_user(&cpuid, cpuid_arg, sizeof cpuid))
			goto out;
		r = kvm_dev_ioctl_get_supported_cpuid(&cpuid,
						      cpuid_arg->entries);
		if (r)
			goto out;

		r = -EFAULT;
		if (copy_to_user(cpuid_arg, &cpuid, sizeof cpuid))
			goto out;
		r = 0;
		break;
	}
     ...
}

There is no additional check on the user controlled data retrieved through copy_from_user() routine and passed to the vulnerable IOCTL handler routine. This means that this bug can be triggered simply by issuing a KVM_GET_SUPPORTED_CPUID IOCTL command to /dev/kvm file with a ‘cpuid->nent’ value large enough to result in an integer overflow during the vmalloc()’s allocation size calculation.
The fix to this bug was to add a check on the user controlled integer like this:

  if (cpuid->nent < 1)
   goto out;
+ if (cpuid->nent > KVM_MAX_CPUID_ENTRIES)
+  cpuid->nent = KVM_MAX_CPUID_ENTRIES;
  r = -ENOMEM;
  cpuid_entries = vmalloc(sizeof(struct kvm_cpuid_entry2) * cpuid->nent);

This ensures that ‘cpuid->nent’ doesn’t exceed the ‘KVM_MAX_CPUID_ENTRIES’ limit (which is 40).

Written by xorl

October 24, 2009 at 12:22

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