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

[Fedor Pchelkin]

On Thu, 24. Jul 16:15, Sergey Larshin wrote:
> > Жизненный цикл ictx контролируется с помощью счётчика ссылок &ictx->users.
> > Если счётчик в imon_disconnect() становится равным нулю после декремента,
> > то объект освобождается.
> 
> Cпасибо за материалы, изучив логику ictx->users, пришёл к выводу, что проблема не во
> всём ctx — он защищён корректно. Однако два его поля — usbdev_intf0 и
> usbdev_intf1 — не защищены. При disconnect() они уменьшается счётчик
> ссылок на устройство в зависимости от номера интерфейса.
> 
> В imon_probe в зависимости от номера интерфейса вызываются функции
> imon_init_intf0 или imon_init_intf1, каждая из которых увеличивает
> счётчик ссылок на usb_device на один.
> 
> В imon_disconnect счётчик уменьшается на один, независимо от
> ictx->users, что приводит к освобождению usbdev_intf0 или
> usbdev_intf1 через вызов usb_put_dev(). В то время как vfd_write
> продолжает работу.
> 
> Решений может быть несколько: 
> - Делать usb_put_dev, когда отчищаем icxt (падение не воспроизвелось 
>   в течении 40 минут)
>   if (refcount_dec_and_test(&ictx->users)) {
>       if (ictx->usbdev_intf0)
>           usb_put_dev(ictx->usbdev_intf0);
>       if (ictx->usbdev_intf1)
>          usb_put_dev(ictx->usbdev_intf1);
>        free_imon_context(ictx);
>   }
> - Сделать lock в disconnect()

IMHO взятие лока при выставлении флага в disconnect и занесение проверки
этого флага под тот же мьютекс в lcd_write()/vfd_write() решает проблему
достаточно понятным образом, ведь эти функции берут мьютекс на всё время
своего выполнения.

(и я правда не понимаю возни с smp_load_acquire и прочими усложнениями,
если всё равно предложенный патч сейчас делает странный хак с взятием и
моментальным снятием мьютекса в disconnect)

Нужно только подумать  о том, что происходит в display_open().