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

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

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

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

Отлично! Код ядра на 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, Software Security Engineer at Google, и закоммиченный Paolo Bonzini, Principal Software Engineer at Red Hat:

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 и удивляемся:

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 (вот, например).

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

	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 отличий".

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

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