[lvc-project] [PATCH] media: rc: Fix use-after-free when racing vfd_write() with disconnect

[Sergey Larshin]

24.07.2025, 10:59 пользователь "Sergey Larshin" <Sergey.Larshin at kaspersky.com <mailto: Sergey.Larshin at kaspersky.com >> написал:

> Наименования функций можно писать без кавычек. То что упоминается
> функция, понятно по наличию скобок в конце. Количество `` можно
> понизить.

Принято, исправлю стиль на более унифицированный.

> В https://syzkaller.appspot.com/bug?extid=f1a69784f6efe748c3bf UAF
> происходит к struct usb_device.
> Пока что из описания не ясно, в чём заключается проблема.

struct usb_device вложен в imon_context, и UAF возникает из-за того,
что disconnect начинает очистку ictx. Это может привести к доступу
к уже освобождённой памяти при обращении к любому полю ictx во время
выполнения vfd_write.

> Каким образом атомарность операций позволит что-то устранить при
> наблюдаемой гонке?

Атомарность операций не устраняет гонку напрямую, но позволяет потоку
записи (vfd_write) быстрее отреагировать на отключение устройства,
так как vfd_write удерживает mutex на несколько вызовов send_packet.
Можно было бы устанавливать флаг disconnected внутри lock, однако в этом
случае передача продолжит работу, даже если устройство уже отключено.

smp_store_release и smp_load_acquire позволяют выставить и
прочитать флаг вне критических секций, при этом гарантируя корректный
порядок выполнения операций. Это даёт возможность vfd_write выйти из цикла
на следующей итерации сразу после установки флага.

Если производительность не критична, флаг можно выставлять внутри
mutex — тогда smp_* не потребуется. Однако в таком случае disconnect()
будет ждать завершения всех отправок, а не только одной итерации.

> По регламентам работ требуется добавлять строчку

Спасибо, исправлю.

> Для патчей, устраняющих ошибки в ядре, желателен тег Fixes.
> https://portal.linuxtesting.ru/How-to-send-patches-to-kernel.html#Fixes

Спасибо, учту.

> Ну это совсем уж dirty hack какой-то :)

> Честно говоря, идея патча с использованием memory-ordering рутин
> smp_load_acquire/smp_store_release для устранения гонки в драйвере не
> понятна, т.к. параллельный поток, если он действительно параллельный,
> может взять и изменить значение переменной сразу после её проверки

>        /* Ensure we see device connect before proceeding */
>        if (smp_load_acquire(&ictx->disconnected))
>                return -ENODEV;

> и полезность такой проверки становится равной нулю. А если секции чтения
> и записи в ictx->disconnected обвешаны какими-нибудь локами, то зачем
> нужен дополнительный smp_load_acquire()?

В данном случае smp_load_acquire не потребовался бы, если бы чтение
и запись переменной были обёрнуты в один и тот же mutex. Но vfd_write
уже держит mutex, а disconnect выставляет флаг до его захвата,
поэтому mutex не создаёт синхронизацию доступа к ictx->disconnected.
В этом случае smp_store_release и smp_load_acquire необходимы
для корректной видимости значения флага в vfd_write.