[lvc-project] [PATCH] Drivers: hv: vmbus: Fix potential NULL pointer dereference in vmbus_acpi_add()
Fedor Pchelkin
pchelkin at ispras.ru
Sat Mar 14 23:18:34 MSK 2026
On Fri, 13. Mar 06:22, Ваторопин Андрей wrote:
> По срабатыванию:
>
> https://lvc-svacer.ispras.ru/r/d3f22ahr8jq9ghitd0m0
>
> В результате исследования удалось доказать, что переменная acpi_disabled
> может иметь значение 0, но воспроизвести баг не удалось.
Что это за исследование? Его детали нигде не отражены и нам неизвестны.
>
> После создания и отправки патча в сообщество был получен ответ:
>
> I understand the case we are trying to make here, but I tried
> reproducing this at my end, where we are probing VMBus using Devicetree,
> and CONFIG_ACPI is enabled and there is no "acpi=off" in kernel cmdline.
> But still, when the control reaches this particular function -
> vmbus_platform_driver_probe(), acpi_disabled still shows up as 1 for me.
> Can you please share your configuration how you are able to reproduce
> this issue.
>
> Мейнтейнер также не смог воспроизвести падение.
И? Удалось ли вам продвинуться в попытках анализа и воспроизведения
проблемы? Мэйнтейнер попросил об этом. Он не утверждал, что проблемы
стопроцентно нет в силу таких-то причин. Он лишь сообщил, что она не
воспроизводится с наскока (в некотором его окружении).
Мы ничего не знаем о вашем исследовании, т.к. к сожалению не делитесь
деталями. Куда копали? что пробовали? в каких конфигурациях? что
получилось и что не получилось? где возникли проблемы или сомнения? Мы бы
могли помочь тем что в наших силах, но нам нужно от чего-то отталкиваться,
инициаторами активности по-хорошему должны быть не мы.
>
> Кроме того, согласно документации:
>
> There can be two kernel build combinations. One build where
> ACPI is not selected in Kconfig and another one with the ACPI Kconfig.
>
> In the first case, vfio_platform_acpi_probe will return since
> acpi_disabled is 1. DT user will not see any kind of messages from
> ACPI.
>
> In the second case, both DT and ACPI is compiled in but the system is
> booting with any of these combinations.
>
> If the firmware is DT type, then acpi_disabled is 1. The ACPI probe routine
> terminates immediately without any messages.
>
> Документация относится не к нашему драйверу, но переменная acpi_disabled глобальная.
>
> Исходя из этого считаем необходимым перевести задачу в статус confirmed minor
Прошу в чётком виде на русском языке сформулировать, почему предлагаете
признать проблему минорной. Есть подозрение, что данный комментарий в
коде (а комментарии менее формальны чем документация) в общем случае
некорректен, либо же корректен в контексте конкретного драйвера в силу
других неявных причин.
Более того, при сворачивании в Minor скорее всего эту задачу закинете на
второй план, тогда как её можно сейчас довести до финального фикса в
апстриме + есть фидбэк от сообщества.
Здесь стоит в первую очередь подчеркнуть — в данном случае нас интересуют
ARM64 системы с поддержкой загрузки посредством ACPI/DT.
Приложу отсылки к более общей формальной документации ядра.
Drivers should determine their probe() type by checking for a null value
for ACPI_HANDLE, or checking .of_node, or other information in the
device structure. This is detailed further in the “Driver
Recommendations” section.
In non-driver code, if the presence of ACPI needs to be detected at run
time, then check the value of acpi_disabled. If CONFIG_ACPI is not set,
acpi_disabled will always be 1.
https://www.kernel.org/doc/html/latest/arch/arm64/arm-acpi.html#acpi-detection
В коде драйверов не рекомендуется использовать проверки с acpi_disabled().
Жаль правда, что явно не прописано, почему. Но косвенно это обосновывается
дальше.
ACPI support in drivers and subsystems for Arm should never be mutually
exclusive with DT support at compile time.
At boot time the kernel will only use one description method depending
on parameters passed from the boot loader (including kernel bootargs).
Regardless of whether DT or ACPI is used, the kernel must always be
capable of booting with either scheme (in kernels with both schemes
enabled at compile time).
https://www.kernel.org/doc/html/latest/arch/arm64/arm-acpi.html#relationship-with-device-tree
Так, т.е. во время загрузки ядра основной метод перечисления оборудования
явно задан в зависимости от того, что передаётся фёрмварью. Хорошо, вроде
никаких проблем тогда быть не должно. Однако дальше:
When an Arm system boots, it can either have DT information, ACPI
tables, or in some very unusual cases, both. If no command line
parameters are used, the kernel will try to use DT for device
enumeration; if there is no DT present, the kernel will try to use ACPI
tables, but only if they are present. If neither is available, the
kernel will not boot. If acpi=force is used on the command line, the
kernel will attempt to use ACPI tables first, but fall back to DT if
there are no ACPI tables present. The basic idea is that the kernel
will not fail to boot unless it absolutely has no other choice.
Processing of ACPI tables may be disabled by passing acpi=off on the
kernel command line; this is the default behavior.
https://www.kernel.org/doc/html/latest/arch/arm64/arm-acpi.html#booting-using-acpi-tables
Пишут, что передача в ядро и Device tree блобов, и ACPI таблиц
одновременно вполне возможна, если прошивка такое позволяет себе и
делает для каких-то целей.
На ARM64 поддержка ACPI возможна только при наличии UEFI-совместимой
прошивки, но использование вариантов
- только Device Tree
- только ACPI
- Device Tree + ACPI
зависит от конкретной прошивки.
Также см. коммит 14bce187d160 ("of/fdt: Restore possibility to use both
ACPI and FDT from bootloader")
of/fdt: Restore possibility to use both ACPI and FDT from bootloader
There are cases when the bootloader provides information to the kernel
in both ACPI and DTB, not interchangeably. One such use case is virtual
machines in Android. When running on x86, the Android Virtualization
Framework (AVF) boots VMs with ACPI like it is usually done on x86 (i.e.
the virtual LAPIC, IOAPIC, HPET, PCI MMCONFIG etc are described in ACPI)
but also passes various AVF-specific boot parameters in DTB. This allows
reusing the same implementations of various AVF components on both
arm64 and x86.
Commit 7b937cc243e5 ("of: Create of_root if no dtb provided by firmware")
removed the possibility to do that, since among other things
it introduced forcing emptying the bootloader-provided DTB if ACPI is
enabled (probably assuming that if ACPI is available, a DTB can only be
useful for applying overlays to it afterwards, for testing purposes).
So restore this possibility. Instead of completely preventing using ACPI
and DT together, rely on arch-specific setup code to prevent using both
to set up the same things (see various acpi_disabled checks under arch/).
Просьба подготовить новую версию патча на основе замечаний мэйнтейнера.
Он предложил устранить проблему другим образом. Рекомендации по тому,
как корректно реализовать проверки устройства, смотрите в
https://www.kernel.org/doc/html/latest/arch/arm64/arm-acpi.html#driver-recommendations
Также стоит дополнить описание новыми нюансами относительно условий
возникновения ошибки. Ждём новую версию в lvc-patches. Огромная просьба
внимательно продумать и сформулировать описание, прочитать его от начала
и до конца перед отправкой патча.
More information about the lvc-project
mailing list