Как-бы патч как-бы уязвимости

Сегодня на SecurityLab в «TOP Новостях» появилась ссылка на «Множественные уязвимости KVM» ^[1].

Интересно посмотреть, думаю, я же копаюсь в нём сейчас. Тем более — open source: можно сразу и на патч посмотреть.
Всем, кому интересно, как инженер по безопасности ПО из Google и главный инженер-программист из Red Hat закоммитили в kernel/git/torvalds/linux.git ^[2]ненужный патч несуществующей уязвимости, добро пожаловать под кат.

Чтобы не отвлекаться на ссылки, приведу цитаты:

1. Уязвимость существует из-за ошибки проверки границ данных в функции kvm_vm_ioctl_create_vcpu() в файле virt/kvm/kvm_main.c. Локальный пользователь может вызвать повреждение памяти и выполнить произвольный код на целевой системе с повышенными привилегиями.

В описании к патчу — чуть более подробно:

KVM: Improve create VCPU parameter (CVE-2013-4587)
In multiple functions the vcpu_id is used as an offset into a bitfield. Ag malicious user could specify a vcpu_id greater than 255 in order to set or clear bits in kernel memory. This could be used to elevate priveges in the kernel. This patch verifies that the vcpu_id provided is less than 255.
The api documentation already specifies that the vcpu_id must be less than max_vcpus, but this is currently not checked.

Отличный пример для повышения квалификации! «Боевая» уязвимость, в исходниках, да ещё и патчем!

Сразу приведены ссылки на патчи.
Вот первая:
git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=338c7dbadd2671189cec7faf64c84d01071b3f96 ^[2]

Отлично! Код ядра на Git уже пропатчен. Но под рукой исходники ядра от RHEL 6 (linux-2.6.32-358.11.1.el6.x86_64), ищу и нахожу:

/*
 * Creates some virtual cpus.  Good luck creating more than one.
 */

static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
{
	int r;
	struct kvm_vcpu *vcpu, *v;

	vcpu = kvm_arch_vcpu_create(kvm, id);

	if (IS_ERR(vcpu))
		return PTR_ERR(vcpu);

	preempt_notifier_init(&vcpu->preempt_notifier, &kvm_preempt_ops);

	r = kvm_arch_vcpu_setup(vcpu);
	if (r)
		return r;

	mutex_lock(&kvm->lock);
	if (!kvm_vcpu_compatible(vcpu)) {
		r = -EINVAL;
		goto vcpu_destroy;
	}
	if (atomic_read(&kvm->online_vcpus) == KVM_MAX_VCPUS) {
		r = -EINVAL;
		goto vcpu_destroy;
	}

	kvm_for_each_vcpu(r, v, kvm)
		if (v->vcpu_id == id) {
			r = -EEXIST;
			goto vcpu_destroy;
		}

	BUG_ON(kvm->vcpus[atomic_read(&kvm->online_vcpus)]);

	/* Now it's all set up, let userspace reach it */
	kvm_get_kvm(kvm);
	r = create_vcpu_fd(vcpu);
	if (r < 0) {
		kvm_put_kvm(kvm);
		goto vcpu_destroy;
	}

	kvm->vcpus[atomic_read(&kvm->online_vcpus)] = vcpu;
	smp_wmb();
	atomic_inc(&kvm->online_vcpus);

#ifdef CONFIG_KVM_APIC_ARCHITECTURE
	if (kvm->bsp_vcpu_id == id)
		kvm->bsp_vcpu = vcpu;
#endif
	mutex_unlock(&kvm->lock);
	return r;

vcpu_destroy:
	mutex_unlock(&kvm->lock);
	kvm_arch_vcpu_destroy(vcpu);
	return r;
}

Вот патч, предложенный Andy Honig ^[3], Software Security Engineer at Google ^[4], и закоммиченный Paolo Bonzini ^[5], Principal Software Engineer at Red Hat ^[6]:

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index a0aa84b..4f588bc 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1898,6 +1898,9 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 	int r;
 	struct kvm_vcpu *vcpu, *v;
 
+	if (id >= KVM_MAX_VCPUS)
+		return -EINVAL;
+
 	vcpu = kvm_arch_vcpu_create(kvm, id);
 	if (IS_ERR(vcpu))
 		return PTR_ERR(vcpu);

А дай-ка, думаю, посмотрю содержимое функции kvm_arch_vcpu_create, которая вызывается сразу после добавленных патчем строк и, как раз, получает проверяемый параметр id при своём вызове. Смотрим файл kvm-ia64.c ^[7] и удивляемся:

struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
		unsigned int id)
{

	struct kvm_vcpu *vcpu;
	unsigned long vm_base = kvm->arch.vm_base;
	int r;
	int cpu;

	BUG_ON(sizeof(struct kvm_vcpu) > VCPU_STRUCT_SIZE/2);

	r = -EINVAL;
	if (id >= KVM_MAX_VCPUS) {
		printk(KERN_ERR"kvm: Can't configure vcpus > %ld",
				KVM_MAX_VCPUS);
		goto fail;
	}

        r = -ENOMEM;
	if (!vm_base) {
		printk(KERN_ERR"kvm: Create vcpu[%d] error!n", id);
		goto fail;
	}
	vcpu = (struct kvm_vcpu *)(vm_base + offsetof(struct kvm_vm_data,
					vcpu_data[id].vcpu_struct));
	vcpu->kvm = kvm;

	cpu = get_cpu();
	r = vti_vcpu_setup(vcpu, id);
	put_cpu();

	if (r) {
		printk(KERN_DEBUG"kvm: vcpu_setup error!!n");
		goto fail;
	}

	return vcpu;

fail:
	return ERR_PTR(r);
}

Да ведь этот код практически эквивалентен тому, что предлагается в патче!

И других способов обойти эту проверку — нет. Причём проверка существует не только в самой свежей версии ядра, но и в версии 2.6.32 (вот, например ^[8]).

Сравните:
Было:

	r = -EINVAL;
	if (id >= KVM_MAX_VCPUS) {
                ...
		goto fail;
	}
fail:	return ERR_PTR(r);

Предлагается:

	if (id >= KVM_MAX_VCPUS)
		return -EINVAL;

и далее «по тексту» — выполняется ещё и проверка в файле kvm-ia64.c.

Как говорится: "Найдите 10 отличий".

Для меня осталась загадкой необходимость таких «патчей».
Ещё более я удивился, погуглив информацию по автору патча ^[3] и коммитеру ^[5].

«Доверяй, но проверяй» (с)?

Автор: Noospheratu

Источник ^[9]