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

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

Интересно посмотреть, думаю, я же копаюсь в нём сейчас. Тем более — 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);

Дальше код нам не нужен, так как патч накладывается на строку между объявлениями локальных переменных и вызовом функции kvm_arch_vcpu_create

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

Для меня осталась загадкой необходимость таких «патчей».

Ещё более я удивился, погуглив информацию по автору патча и коммитеру.

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

This entry passed through the Full-Text RSS service — if this is your content and you're reading it on someone else's site, please read the FAQ at fivefilters.org/content-only/faq.php#publishers.