KolibriOS/kolibrios
15
24
Fork 61
Code Issues 132 Pull Requests 32 Actions Projects 1 Releases Wiki Activity

VBoxGuest драйвер и VBoxCtrl управление #352

Open
lex_coder wants to merge 7 commits from lex_coder/kolibrios:VBoxGuest into main
pull from: lex_coder/kolibrios:VBoxGuest
merge into: KolibriOS:main
Conversation 11 Commits 7 Files Changed 64
+12293
lex_coder commented 2026-03-04 18:48:47 +00:00
Contributor
Copy Link
Copy Source

Реализовано:
Mouse - абсолютное позиционирование мыши
Display - синхронизация разрешения экрана
Clipboard - общий буфер обмена
Shared Folders - общие папки
Heartbeat - мониторинг состояния гостя
Guest Properties - свойства гостевой машины
Timesync - синхронизация времени (отключена)
Seamless - бесшовный режим окон (отключен)
и известное количество багов

Реализовано: Mouse - абсолютное позиционирование мыши Display - синхронизация разрешения экрана Clipboard - общий буфер обмена Shared Folders - общие папки Heartbeat - мониторинг состояния гостя Guest Properties - свойства гостевой машины Timesync - синхронизация времени (отключена) Seamless - бесшовный режим окон (отключен) и известное количество багов
lex_coder added 2 commits 2026-03-04 18:48:48 +00:00
Add vboxctrl.asm 1493edddb6
Add VBoxGuest driver code
Build system / Check kernel codestyle (pull_request) Has been cancelled
Details
Build system / Build (pull_request) Has been cancelled
Details
ab59015ded
mxlgv requested changes 2026-03-07 20:47:12 +00:00
mxlgv left a comment
Owner
Copy Link
Copy Source

@lex_coder Hi!

I see that you have done a titanic job and your solution is extremely structured. This is worthy of all praise and respect.

What problems have I noticed:

  • Integration into the TUP build system needs to be considered.
  • I would like to see comments in the code in English (include PR description), since the project is international (a lot of work has been done to get rid of comments in Russian in the kernel, including various jokes).
@lex_coder Hi! I see that you have done a titanic job and your solution is extremely structured. This is worthy of all praise and respect. What problems have I noticed: - Integration into the TUP build system needs to be considered. - I would like to see comments in the code in English (include PR description), since the project is international (a lot of work has been done to get rid of comments in Russian in the kernel, including various jokes).
Doczom requested review from Doczom 2026-03-11 09:01:09 +00:00
Doczom commented 2026-03-11 09:06:50 +00:00
Owner
Copy Link
Copy Source

vboxctrl всё же следует перенести из директории drivers в programs/system например, так как это пользовательское приложение и не обязательная часть драйвера

vboxctrl всё же следует перенести из директории drivers в programs/system например, так как это пользовательское приложение и не обязательная часть драйвера
mxlgv added the Category/ApplicationsCategory/DriversFASM
Kind
Feature
 labels 2026-03-11 09:43:17 +00:00
lex_coder added 1 commit 2026-04-04 10:21:19 +00:00
merge upstream
Build system / Check kernel codestyle (pull_request) Has been cancelled
Details
Build system / Build (pull_request) Has been cancelled
Details
1fb26934bf
lex_coder added 1 commit 2026-04-08 15:44:09 +00:00
VBoxGuest fix lang, core, tup, ctrl
Build system / Check kernel codestyle (pull_request) Has been cancelled
Details
Build system / Build (pull_request) Has been cancelled
Details
48efdad5bd
lex_coder added 1 commit 2026-04-08 21:01:57 +00:00
Translate comment to en
Build system / Check kernel codestyle (pull_request) Successful in 38s
Details
Build system / Build (pull_request) Failing after 5m49s
Details
4830c338ac
Doczom commented 2026-04-11 09:52:49 +00:00
Owner
Copy Link
Copy Source

to fix the build, add the "-m 65536" argument to the Tupfile.lua

to fix the build, add the "-m 65536" argument to the Tupfile.lua
lex_coder added 1 commit 2026-04-15 17:41:32 +00:00
Tup file add mem limit
Build system / Check kernel codestyle (pull_request) Successful in 1m44s
Details
Build system / Build (pull_request) Successful in 11m30s
Details
258dd97d16
lex_coder requested review from mxlgv 2026-04-22 09:19:34 +00:00
Doczom reviewed 2026-05-20 04:04:47 +00:00
programs/system/vboxctrl/vboxctrl.asm Outdated
@@ -0,0 +69,4 @@
.output dd ?
.out_size dd ?
.size = $
end virtual
Doczom commented 2026-05-20 04:04:47 +00:00
Owner
Copy Link
Copy Source

The macros.inc file includes the connection of the struct.inc file, which allows you to declare this structure through the "struct" macro.

The macros.inc file includes the connection of the struct.inc file, which allows you to declare this structure through the "struct" macro.
lex_coder commented 2026-05-29 17:19:53 +00:00
Author
Contributor
Copy Link
Copy Source

Изменил

Изменил
Doczom reviewed 2026-05-20 15:21:43 +00:00
Doczom left a comment
Owner
Copy Link
Copy Source

@dunkaist please check the file system driver code

@dunkaist please check the file system driver code
drivers/vboxguest/core/irq.inc Outdated
@@ -0,0 +8,4 @@
vbox_irq_count dd 0
; vbox_irq_handler - Top-half IRQ handler
proc vbox_irq_handler stdcall
Doczom commented 2026-05-20 05:12:53 +00:00
Owner
Copy Link
Copy Source

incorrect function declaration

int32_t __cdecl irq_handler(uint32_t userdata);

incorrect function declaration `int32_t __cdecl irq_handler(uint32_t userdata);`
lex_coder commented 2026-05-29 17:20:34 +00:00
Author
Contributor
Copy Link
Copy Source

Внеси правку

Внеси правку
drivers/vboxguest/core/pci.inc
@@ -0,0 +67,4 @@
; Validate IRQ (must be 0-15)
cmp eax, 16
jae .invalid_irq
Doczom commented 2026-05-20 04:59:53 +00:00
Owner
Copy Link
Copy Source

Is this check really necessary?

Is this check really necessary?
lex_coder commented 2026-05-29 17:19:39 +00:00
Author
Contributor
Copy Link
Copy Source

Убрал)

Убрал)
drivers/vboxguest/core/ports.inc
@@ -0,0 +17,4 @@
out dx, eax
; Spin-wait (~1ms): gives the host time to process the request
mov ecx, 1000000
Doczom commented 2026-05-20 05:03:43 +00:00
Owner
Copy Link
Copy Source

немного похоже на костыль

немного похоже на костыль
lex_coder commented 2026-05-29 17:19:32 +00:00
Author
Contributor
Copy Link
Copy Source

Да, убрал

Да, убрал
drivers/vboxguest/services/clipboard/clipboard.inc Outdated
@@ -0,0 +56,4 @@
cmp dword [clip_state.listen_pkt_virt], 0
jne .pkt_ready
invoke KernelAlloc, 4096
Doczom commented 2026-05-20 14:48:18 +00:00
Owner
Copy Link
Copy Source

instead of this magic number, it is better to use the PAGE_SIZE constant.

instead of this magic number, it is better to use the PAGE_SIZE constant.
lex_coder commented 2026-05-29 17:19:26 +00:00
Author
Contributor
Copy Link
Copy Source

Заменил везде на PAGE_SIZE

Заменил везде на PAGE_SIZE
drivers/vboxguest/services/guest_props/guest_props.inc Outdated
@@ -0,0 +58,4 @@
je .already_connected
; --- Allocate buffers ---
invoke KernelAlloc, 4096 ; gp_enum_buf (page-aligned for HGCM)
Doczom commented 2026-05-20 14:54:59 +00:00
Owner
Copy Link
Copy Source

instead of this magic number, it is better to use the PAGE_SIZE constant.

instead of this magic number, it is better to use the PAGE_SIZE constant.
drivers/vboxguest/services/shared_folders/shared_folders.inc Outdated
@@ -0,0 +42,4 @@
jae .error
; Allocate memory for SF_FOLDER
invoke KernelAlloc, 4096
Doczom commented 2026-05-20 15:07:33 +00:00
Owner
Copy Link
Copy Source

instead of this magic number, it is better to use the PAGE_SIZE constant.

instead of this magic number, it is better to use the PAGE_SIZE constant.
drivers/vboxguest/services/shared_folders/shared_folders.inc Outdated
@@ -0,0 +198,4 @@
je .already_connected
; --- Allocate init buffers ---
invoke KernelAlloc, 4096 ; sf_packet
Doczom commented 2026-05-20 15:12:22 +00:00
Owner
Copy Link
Copy Source

instead of this magic number, it is better to use the PAGE_SIZE constant.

instead of this magic number, it is better to use the PAGE_SIZE constant.
drivers/vboxguest/vmmdev/packets.inc Outdated
@@ -0,0 +22,4 @@
DEBUGF 2, "[VBoxGuest] [VMMDev] Init packets (dynamic)\n"
; --- Allocate page ---
invoke KernelAlloc, 4096
Doczom commented 2026-05-20 14:41:55 +00:00
Owner
Copy Link
Copy Source

instead of this magic number, it is better to use the PAGE_SIZE constant

instead of this magic number, it is better to use the PAGE_SIZE constant
👀 1
Doczom requested changes 2026-05-20 15:23:27 +00:00
Dismissed
Doczom left a comment
Owner
Copy Link
Copy Source

The reason is described in the comments above.

The reason is described in the comments above.
Doczom requested review from dunkaist 2026-05-20 15:24:01 +00:00
dunkaist approved these changes 2026-05-21 07:17:36 +00:00
hidnplayr commented 2026-05-28 18:01:47 +00:00
Member
Copy Link
Copy Source

Sounds amazing, I'll test it soon.

Any particular reason virtio network was skipped? Is it part of a separate logical module?

Sounds amazing, I'll test it soon. Any particular reason virtio network was skipped? Is it part of a separate logical module?
lex_coder commented 2026-05-29 17:00:12 +00:00
Author
Contributor
Copy Link
Copy Source

Sounds amazing, I'll test it soon.

Any particular reason virtio network was skipped? Is it part of a separate logical module?

VirtIO isn't included in the VirtualBox Guest Additions because the Guest Additions aren't related to VirtIO. The Guest Additions are VBoxClipBoard, VBoxSF, VBoxMouse and other

> Sounds amazing, I'll test it soon. > > Any particular reason virtio network was skipped? Is it part of a separate logical module? VirtIO isn't included in the VirtualBox Guest Additions because the Guest Additions aren't related to VirtIO. The Guest Additions are VBoxClipBoard, VBoxSF, VBoxMouse and other
👍 1
lex_coder added 1 commit 2026-05-29 17:18:46 +00:00
VBoxGuest minor fix constant, struct, version
Build system / Check kernel codestyle (pull_request) Successful in 26s
Details
Build system / Build (pull_request) Successful in 10m29s
Details
57d5026702
Doczom approved these changes 2026-05-29 17:37:54 +00:00
Burer commented 2026-06-03 14:22:31 +00:00
Owner
Copy Link
Copy Source

Я ревьюшнул нейронкой этот ПР, и нашёл несколько проблем.
Автор подтвердил их актуальность и сказал, что займётся исправлением в ближайшее время.
Поэтому на всякий случай оставляю этот ревью здесь.

🔴 Критические - память и безопасность

1. IOCTL: указатели input/output от пользователя не валидируются → произвольная запись/чтение в ring0
drivers/vboxguest/sys/ioctl.inc:51-54 и все обработчики clipboard.

Ядро (srv_handlerEx) проверяет только сам IOCTL-структуру (< OS_BASE), но не указатели input/output внутри неё — они приходят от недоверенного приложения. Драйвер разыменовывает их без проверки диапазона:

mov eax, [ebx + IOCTL.output]   ; указатель из user space
mov dword [eax], API_VERSION    ; запись по произвольному адресу ядра

То же в svc_enable (чтение [input]), clip_ioctl_read/write/status. Проверяются только размеры, не принадлежность памяти вызывающему. Это классический примитив записи/чтения в ядре для любого процесса, открывшего сервис.

Исправление: проверять каждый input/output вместе с inp_size/out_size на принадлежность user space (без переполнения) до разыменования.

2. Длина имени файла от хоста копируется без ограничения → переполнение буфера
drivers/vboxguest/services/shared_folders/vboxsf.inc:732-735 + drivers/vboxguest/common/utils.inc:7-133

movzx ecx, word [esi + SHFLDIRINFO_OFS_NAME_LENGTH]  ; длина от хоста, до 65535
lea   eax, [edi + BDFE_OFS_NAME]                     ; поле имени BDFE = 264 байта
stdcall sf_utf8_to_cp866, esi, eax, ecx

sf_utf8_to_cp866 не принимает размер приёмника и не проверяет границу dst — пишет по байту на символ плюс терминатор. Имя длиннее поля BDFE переполняет буфер вызывающего VFS. Антипаттерн «данные хоста как длина копирования».

Исправление: добавить параметр dst_len в sf_utf8_to_cp866 и обрезать ecx (например, ≤255) до вызова. Тот же отсутствующий клэмп u16Length от хоста — в shared_folders.inc (null-запись по индексу от хоста).

3. HGCM page-list буфер физически не непрерывен, но отправляется хосту как единый >4 КБ запрос
drivers/vboxguest/hgcm/hgcm.inc:60-66, drivers/vboxguest/hgcm/call.inc:282-303

HGCM_PL_BUF_SIZE (~8620 байт → 3 страницы) выделяется через KernelAlloc. По исходнику ядра kernel/trunk/core/heap.inc:493-505: для 3 страниц (pages_count>>3 == 0) все страницы выделяются поодиночке через alloc_page — виртуально непрерывны, но физически разбросаны. Хосту передаётся только физ. адрес начала (hgcm.inc:131-139), и он читает весь запрос (header.size байт) как непрерывный блок. Проверка .buffer_overflow разрешает data_tail до ~8620 байт, т.е. запрос может выходить за первую страницу. Тогда хост читает страницу 2 по неверному физ. адресу → массив aPages[] указывает на чужие фреймы → повреждение памяти.

Латентно: проявляется на крупных передачах (page-list, занимающий >4096 байт, ≈ буфер >1.8 МБ за один HGCM-вызов).

Исправление: выделять PL-буфер физически непрерывно (через alloc_pages/CreateRingBuffer-эквивалент) либо гарантировать, что запрос не пересекает первую физ. страницу.

4. Константы full-state-protocol мыши коллизируют → ломаются кнопки и колесо
drivers/vboxguest/data/mouse/constants.inc:18-25

VMMDEV_MOUSE_NOTIFY_GUEST                       equ 0x80
VMMDEV_MOUSE_HOST_WANTS_ABSOLUTE                equ 0x100
VMMDEV_MOUSE_GUEST_USES_FULL_STATE_PROTOCOL     equ 0x80    ; коллизия
VMMDEV_MOUSE_HOST_SUPPORTS_FULL_STATE_PROTOCOL  equ 0x100   ; коллизия

В реальном VBox это разные биты (0x400/0x800). mouse.inc:69 проверяет & 0x100 — но это HOST_WANTS_ABSOLUTE, который при включённой интеграции мыши выставлен почти всегда → драйвер всегда уходит в extended-режим и отключает PS/2 IRQ12 (mouse.inc:83). На хостах без full-state-протокола кнопки и колесо молча перестают работать.

Исправление: задать корректные значения битов VBox и сделать все четыре equ различными.

5. IRQ-обработчик не отсоединяется при выгрузке → use-after-free
drivers/vboxguest/sys/shutdown.inc:23-24

call vbox_irq_detach закомментирован, а самой процедуры/DetachIntHandler в дереве нет. На DRV_EXIT освобождаются HGCM/VMMDev-пакеты (shutdown.inc:37-41), пока ISR ещё подключён. Собственный комментарий кода (стр. 21-22) требует detach до освобождения буферов — но код выключен. Прерывание после освобождения = UAF в ядре. (Срабатывает только при реальной выгрузке драйвера.)

Исправление: реализовать detach (DetachIntHandler + маскирование IRQ устройства) и вызвать его до освобождения пакетов.

6. vboxctrl: rhc_bufsize не обновляется после реаллокации → зависание при буфере обмена >64 КБ
programs/system/vboxctrl/vboxctrl.asm:508-570

При переполнении утилита аллоцирует больший буфер (rhc_target_size) и прыгает на .rhc_retry, но out_size пересчитывается из rhc_bufsize, которое осталось MAX_CLIP_BUF (64 КБ). Драйвер снова возвращает overflow → реаллокация → повтор… бесконечный цикл. Поскольку это вызывается в обработчике опроса буфера обмена, утилита (а с ней и интеграция clipboard) зависает на любом содержимом >64 КБ.

Исправление: в .rhc_alloc_ok добавить mov eax,[rhc_target_size]; mov [rhc_bufsize],eax перед jmp .rhc_retry.

🟠 Средние

7. Защита от реентерабельности HGCM отключена + общие одиночные буферы → гонка
drivers/vboxguest/hgcm/call.inc:284,377,382,386

Все lock bts [hgcm_pl_busy] и сбросы закомментированы, .busy недостижим. hgcm_call32_pagelist использует один общий hgcm_call_pl_virt; connect/disconnect — тоже общая страница. Clipboard работает в отдельном потоке-слушателе, shared_folders/guest_props — в контексте VFS; одновременные HGCM-вызовы реальны и затрут пакеты друг друга (окно велико из-за долгого ожидания, см. п. 11).

Исправление: вернуть флаг занятости (со сбросом на всех выходах, включая .bad/.buffer_overflow/.send_failed) либо использовать буферы на вызов / блокировку.

8. Несогласованные границы массива shared folders → латентное переполнение
drivers/vboxguest/services/shared_folders/shared_folders.inc:41,62-63, drivers/vboxguest/data/shared_folders/structs.inc:82

Массив folders rd SHFL_MAX_FOLDERS (=10 в config.inc), но vboxsf_add_folder проверяет лимит против литерала 32 и пишет [folders + ecx*4] для ecx до 31 → запись за пределы массива (на count, disk_handle и дальше), если в add_folder дойдёт >10 папок. Безопасность сейчас держится только на клэмпе на стороне вызова — defense-in-depth нарушен.

Исправление: заменить 32 на SHFL_MAX_FOLDERS и согласовать с размером массива.

9. sys_init: нет отката при частичной инициализации; утечка MMIO
drivers/vboxguest/sys/init.inc:68-69, drivers/vboxguest/core/mmio.inc:30-32

Все ветки .fail делают голый ret без unmap/KernelFree/остановки таймера. mmio_map_vmmdev.version_check_failed возвращается, не размапив mmio_virt. Если инициализация упала после vmmdev_irq_install, IRQ остаётся подключённым (усугубляет п. 5).

Исправление: добавить раскрутку в обратном порядке на каждой метке отказа; сделать shutdown идемпотентным.

10. clip_ioctl_write/clip_ioctl_read: данные хоста/пользователя без верхнего предела
drivers/vboxguest/services/clipboard/clipboard_listener.inc:407-427, clipboard_listener.inc:456-489

data_size (из inp_size) и actual_size (от хоста) принимаются без разумного максимума и идут в KernelAlloc. Само по себе переполнение при округлении вверх безопасно (даёт KernelAlloc(0), который корректно падает — см. ниже), но отсутствие верхней границы и проверки источника [input] (см. п. 1) стоит закрыть.

11. HGCM async-ожидание — busy-poll, игнорирующий IRQ-событие
drivers/vboxguest/hgcm/async.inc:19-39

Ожидание крутит pause/loop по флагу VBOX_HGCM_REQ_DONE, не уступая планировщику и не дожидаясь события RaiseEvent, которое поднимает IRQ-обработчик (irq.inc:50-81) — механизм пробуждения по факту не используется. KolibriOS вытесняющая, так что это не зависание, но напрасный расход CPU и риск ложного таймаута под нагрузкой.

Исправление: ждать на hgcm_wakeup_event с перепроверкой флага (паттерн condition variable), убрать busy-loop.

🟡 Мелочи/Стиль

  • Релизная сборка с включённой отладкой: __DEBUG__ = 1 в config.inc:31 и в vboxctrl — лог в debug-board каждую секунду. Для релиза 0.
  • Коллизия кодов ошибок: VERR_TIMEOUT и VERR_NOT_FOUND оба = -78 (errors.inc:27,40) — различить нельзя. Сверить с upstream err.h.
  • seamless: jnz вместо js (seamless.inc) — положительные предупреждения VINF_* трактуются как ошибки.
  • heartbeat/timesync: двойной GetTimerTicks (TOCTOU) и неинициализированный last_send_time — первый тик срабатывает не от момента init.
  • hgcm_init (hgcm.inc:113-121) пропагирует ошибку только потому, что промежуточный DEBUGF отключён (уровень 0). Хрупко: при включении __DEBUG_HGCM__ eax будет затёрт. Стоит явно test eax,eax / jnz.
  • service_proc без uses ebx esi edi (ioctl.inc:28) — для пути syscall 68.17 безопасно (ядро восстанавливает кадр), но расходится с конвенцией.
  • PCI BAR-парсинг не проверяет тип BAR / 64-битные BAR (core/pci.inc) — для фиксированного VMMDev работает, но хрупко.

🔵 Требует подтверждения автором

  • IRQ fast-ACK (core/irq.inc:14-24): обработчик читает порт +VMMDEV_PORT_OFF_REQUEST_FAST для получения и подтверждения событий, но VMMDEV_HF_FAST_IRQ_ACK нигде с хостом не согласуется. Если хост не активировал fast-ACK, прерывание может не сниматься (риск IRQ-шторма в ring0). Дополнительно вызывает сомнение out dx, eax (стр. 24), записывающий маску событий в порт запроса. Нужно сверить с поведением VMMDev конкретных версий VBox.
Я ревьюшнул нейронкой этот ПР, и нашёл несколько проблем. Автор подтвердил их актуальность и сказал, что займётся исправлением в ближайшее время. Поэтому на всякий случай оставляю этот ревью здесь. **🔴 Критические - память и безопасность** **1.** IOCTL: указатели `input`/`output` от пользователя не валидируются → произвольная запись/чтение в ring0 `drivers/vboxguest/sys/ioctl.inc:51-54` и все обработчики clipboard. Ядро (`srv_handlerEx`) проверяет только сам IOCTL-структуру (`< OS_BASE`), но **не** указатели `input`/`output` внутри неё — они приходят от недоверенного приложения. Драйвер разыменовывает их без проверки диапазона: ```asm mov eax, [ebx + IOCTL.output] ; указатель из user space mov dword [eax], API_VERSION ; запись по произвольному адресу ядра ``` То же в `svc_enable` (чтение `[input]`), `clip_ioctl_read`/`write`/`status`. Проверяются только *размеры*, не принадлежность памяти вызывающему. Это классический примитив записи/чтения в ядре для любого процесса, открывшего сервис. **Исправление:** проверять каждый `input`/`output` вместе с `inp_size`/`out_size` на принадлежность user space (без переполнения) до разыменования. **2.** Длина имени файла от хоста копируется без ограничения → переполнение буфера `drivers/vboxguest/services/shared_folders/vboxsf.inc:732-735` + `drivers/vboxguest/common/utils.inc:7-133` ```asm movzx ecx, word [esi + SHFLDIRINFO_OFS_NAME_LENGTH] ; длина от хоста, до 65535 lea eax, [edi + BDFE_OFS_NAME] ; поле имени BDFE = 264 байта stdcall sf_utf8_to_cp866, esi, eax, ecx ``` `sf_utf8_to_cp866` **не принимает размер приёмника** и не проверяет границу `dst` — пишет по байту на символ плюс терминатор. Имя длиннее поля BDFE переполняет буфер вызывающего VFS. Антипаттерн «данные хоста как длина копирования». **Исправление:** добавить параметр `dst_len` в `sf_utf8_to_cp866` и обрезать `ecx` (например, ≤255) до вызова. Тот же отсутствующий клэмп `u16Length` от хоста — в `shared_folders.inc` (null-запись по индексу от хоста). **3.** HGCM page-list буфер физически не непрерывен, но отправляется хосту как единый >4 КБ запрос `drivers/vboxguest/hgcm/hgcm.inc:60-66`, `drivers/vboxguest/hgcm/call.inc:282-303` `HGCM_PL_BUF_SIZE` (~8620 байт → 3 страницы) выделяется через `KernelAlloc`. По исходнику ядра `kernel/trunk/core/heap.inc:493-505`: для 3 страниц (`pages_count>>3 == 0`) все страницы выделяются **поодиночке** через `alloc_page` — виртуально непрерывны, но **физически разбросаны**. Хосту передаётся только физ. адрес начала (`hgcm.inc:131-139`), и он читает весь запрос (`header.size` байт) как непрерывный блок. Проверка `.buffer_overflow` разрешает `data_tail` до ~8620 байт, т.е. запрос **может** выходить за первую страницу. Тогда хост читает страницу 2 по неверному физ. адресу → массив `aPages[]` указывает на чужие фреймы → повреждение памяти. Латентно: проявляется на крупных передачах (page-list, занимающий >4096 байт, ≈ буфер >1.8 МБ за один HGCM-вызов). **Исправление:** выделять PL-буфер физически непрерывно (через `alloc_pages`/`CreateRingBuffer`-эквивалент) либо гарантировать, что запрос не пересекает первую физ. страницу. **4.** Константы full-state-protocol мыши коллизируют → ломаются кнопки и колесо `drivers/vboxguest/data/mouse/constants.inc:18-25` ``` VMMDEV_MOUSE_NOTIFY_GUEST equ 0x80 VMMDEV_MOUSE_HOST_WANTS_ABSOLUTE equ 0x100 VMMDEV_MOUSE_GUEST_USES_FULL_STATE_PROTOCOL equ 0x80 ; коллизия VMMDEV_MOUSE_HOST_SUPPORTS_FULL_STATE_PROTOCOL equ 0x100 ; коллизия ``` В реальном VBox это разные биты (`0x400`/`0x800`). `mouse.inc:69` проверяет `& 0x100` — но это `HOST_WANTS_ABSOLUTE`, который при включённой интеграции мыши выставлен почти всегда → драйвер всегда уходит в extended-режим и **отключает PS/2 IRQ12** (`mouse.inc:83`). На хостах без full-state-протокола кнопки и колесо молча перестают работать. **Исправление:** задать корректные значения битов VBox и сделать все четыре `equ` различными. **5.** IRQ-обработчик не отсоединяется при выгрузке → use-after-free `drivers/vboxguest/sys/shutdown.inc:23-24` `call vbox_irq_detach` закомментирован, а самой процедуры/`DetachIntHandler` в дереве нет. На `DRV_EXIT` освобождаются HGCM/VMMDev-пакеты (`shutdown.inc:37-41`), пока ISR ещё подключён. Собственный комментарий кода (стр. 21-22) требует detach до освобождения буферов — но код выключен. Прерывание после освобождения = UAF в ядре. *(Срабатывает только при реальной выгрузке драйвера.)* **Исправление:** реализовать detach (`DetachIntHandler` + маскирование IRQ устройства) и вызвать его до освобождения пакетов. **6.** `vboxctrl`: `rhc_bufsize` не обновляется после реаллокации → зависание при буфере обмена >64 КБ `programs/system/vboxctrl/vboxctrl.asm:508-570` При переполнении утилита аллоцирует больший буфер (`rhc_target_size`) и прыгает на `.rhc_retry`, но `out_size` пересчитывается из `rhc_bufsize`, которое осталось `MAX_CLIP_BUF` (64 КБ). Драйвер снова возвращает overflow → реаллокация → повтор… **бесконечный цикл**. Поскольку это вызывается в обработчике опроса буфера обмена, утилита (а с ней и интеграция clipboard) зависает на любом содержимом >64 КБ. **Исправление:** в `.rhc_alloc_ok` добавить `mov eax,[rhc_target_size]; mov [rhc_bufsize],eax` перед `jmp .rhc_retry`. **🟠 Средние** **7.** Защита от реентерабельности HGCM отключена + общие одиночные буферы → гонка `drivers/vboxguest/hgcm/call.inc:284,377,382,386` Все `lock bts [hgcm_pl_busy]` и сбросы закомментированы, `.busy` недостижим. `hgcm_call32_pagelist` использует один общий `hgcm_call_pl_virt`; connect/disconnect — тоже общая страница. Clipboard работает в отдельном потоке-слушателе, shared_folders/guest_props — в контексте VFS; одновременные HGCM-вызовы реальны и затрут пакеты друг друга (окно велико из-за долгого ожидания, см. п. 11). **Исправление:** вернуть флаг занятости (со сбросом на **всех** выходах, включая `.bad`/`.buffer_overflow`/`.send_failed`) либо использовать буферы на вызов / блокировку. **8.** Несогласованные границы массива shared folders → латентное переполнение `drivers/vboxguest/services/shared_folders/shared_folders.inc:41,62-63`, `drivers/vboxguest/data/shared_folders/structs.inc:82` Массив `folders rd SHFL_MAX_FOLDERS` (=10 в `config.inc`), но `vboxsf_add_folder` проверяет лимит против литерала `32` и пишет `[folders + ecx*4]` для `ecx` до 31 → запись за пределы массива (на `count`, `disk_handle` и дальше), если в `add_folder` дойдёт >10 папок. Безопасность сейчас держится только на клэмпе на стороне вызова — defense-in-depth нарушен. **Исправление:** заменить `32` на `SHFL_MAX_FOLDERS` и согласовать с размером массива. **9.** `sys_init`: нет отката при частичной инициализации; утечка MMIO `drivers/vboxguest/sys/init.inc:68-69`, `drivers/vboxguest/core/mmio.inc:30-32` Все ветки `.fail` делают голый `ret` без `unmap`/`KernelFree`/остановки таймера. `mmio_map_vmmdev.version_check_failed` возвращается, не размапив `mmio_virt`. Если инициализация упала после `vmmdev_irq_install`, IRQ остаётся подключённым (усугубляет п. 5). **Исправление:** добавить раскрутку в обратном порядке на каждой метке отказа; сделать shutdown идемпотентным. **10.** `clip_ioctl_write`/`clip_ioctl_read`: данные хоста/пользователя без верхнего предела `drivers/vboxguest/services/clipboard/clipboard_listener.inc:407-427`, `clipboard_listener.inc:456-489` `data_size` (из `inp_size`) и `actual_size` (от хоста) принимаются без разумного максимума и идут в `KernelAlloc`. Само по себе переполнение при округлении вверх безопасно (даёт `KernelAlloc(0)`, который корректно падает — см. ниже), но отсутствие верхней границы и проверки источника `[input]` (см. п. 1) стоит закрыть. **11.** HGCM async-ожидание — busy-poll, игнорирующий IRQ-событие `drivers/vboxguest/hgcm/async.inc:19-39` Ожидание крутит `pause`/`loop` по флагу `VBOX_HGCM_REQ_DONE`, не уступая планировщику и **не дожидаясь** события `RaiseEvent`, которое поднимает IRQ-обработчик (`irq.inc:50-81`) — механизм пробуждения по факту не используется. KolibriOS вытесняющая, так что это не зависание, но напрасный расход CPU и риск ложного таймаута под нагрузкой. **Исправление:** ждать на `hgcm_wakeup_event` с перепроверкой флага (паттерн condition variable), убрать busy-loop. **🟡 Мелочи/Стиль** - **Релизная сборка с включённой отладкой:** `__DEBUG__ = 1` в `config.inc:31` и в `vboxctrl` — лог в debug-board каждую секунду. Для релиза `0`. - **Коллизия кодов ошибок:** `VERR_TIMEOUT` и `VERR_NOT_FOUND` оба `= -78` (`errors.inc:27,40`) — различить нельзя. Сверить с upstream `err.h`. - **seamless:** `jnz` вместо `js` (`seamless.inc`) — положительные предупреждения `VINF_*` трактуются как ошибки. - **heartbeat/timesync:** двойной `GetTimerTicks` (TOCTOU) и неинициализированный `last_send_time` — первый тик срабатывает не от момента init. - **`hgcm_init`** (`hgcm.inc:113-121`) пропагирует ошибку только потому, что промежуточный `DEBUGF` отключён (уровень 0). Хрупко: при включении `__DEBUG_HGCM__` `eax` будет затёрт. Стоит явно `test eax,eax / jnz`. - **`service_proc`** без `uses ebx esi edi` (`ioctl.inc:28`) — для пути syscall 68.17 безопасно (ядро восстанавливает кадр), но расходится с конвенцией. - **PCI BAR-парсинг** не проверяет тип BAR / 64-битные BAR (`core/pci.inc`) — для фиксированного VMMDev работает, но хрупко. **🔵 Требует подтверждения автором** - **IRQ fast-ACK** (`core/irq.inc:14-24`): обработчик читает порт `+VMMDEV_PORT_OFF_REQUEST_FAST` для получения и подтверждения событий, но `VMMDEV_HF_FAST_IRQ_ACK` нигде с хостом не согласуется. Если хост не активировал fast-ACK, прерывание может не сниматься (риск IRQ-шторма в ring0). Дополнительно вызывает сомнение `out dx, eax` (стр. 24), записывающий маску событий в порт запроса. Нужно сверить с поведением VMMDev конкретных версий VBox.
All checks were successful
Build system / Check kernel codestyle (pull_request) Successful in 26s
Required
Details
Build system / Build (pull_request) Successful in 10m29s
Required
Details
Checking for merge conflicts…
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u http://git.kolibrios.org/lex_coder/kolibrios VBoxGuest:lex_coder-VBoxGuest
git checkout lex_coder-VBoxGuest
Sign in to join this conversation.
Reviewers
No Reviewers
mxlgv
dunkaist
Doczom
Labels
Clear labels
C
Category/Applications
Category/Drivers
Category/General
Category/Kernel
Category/Libraries
Eolite
FASM
FS
GSoC
This issue in GSoC program
 HardwareTested
HLL
Influence/Settings
Influence/Text/TYPO
IRCC
Kernel
Kind
Breaking
Breaking change that won't be backward compatible
Kind
Bug
Something is not working
Kind
Build
Kind
Documentation
Documentation changes
Kind
Enhancement
Improve existing functionality
Kind
Feature
New functionality
Kind
Security
This is security issue
Kind
Testing
Issue or pull request related to testing
Pay for the code
Paid task
PR
Conflicts
PR conflicts with main
PR
Dependent
This PR is dependent on another PR
Priority
Critical
The priority is critical
Priority
High
The priority is high
Priority
Low
The priority is low
Priority
Medium
The priority is medium
PR
Ready to merge
Pull request is ready for merge
PR
Request changes
Changes requested in pull request
PR
Review required
Reviewed
Confirmed
Issue has been confirmed
Reviewed
Duplicate
This issue or pull request already exists
Reviewed
Invalid
Invalid issue
Reviewed
Won't Fix
This issue won't be fixed
Status
Abandoned
Somebody has started to work on this but abandoned work
Status
Blocked
Something is blocking this issue or pull request
Status
Need More Info
Feedback is required to reproduce issue or to continue work
No Label Category/Applications Category/Drivers FASM
Kind
Feature
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ace-dent (Andrew) Burer Doczom (Mikhail Frolov) dunkaist (Ivan B) Egor00f (Егор) hidnplayr IgorA Leency (Kiril Lipatov) mxlgv (Max Logaev) ramenu rgimad sdongles (Serhii Sakhno) Sweetbread (Gleb Zaharov)
No Assignees
6 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: KolibriOS/kolibrios#352
Delete Branch "lex_coder/kolibrios:VBoxGuest"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?