Re: [PATCH v3] drm/virtio: share one vbuf cache across all devices

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 10/10/26 18:04, Nguyen Ngoc Thang wrote:
> virtio_gpu_alloc_vbufs() creates a kmem_cache with the fixed name
> "virtio-gpu-vbufs" for every virtio-gpu device and destroys it from
> virtio_gpu_release(), which only runs once the last drm_device
> reference is dropped.
> 
> Two live virtio-gpu devices are enough to hit this: the second
> kmem_cache_create() call finds the name already taken and
> kmem_cache_sanity_check() WARNs. This reproduces at plain boot with
> two "-device virtio-gpu-pci" on the QEMU command line, no sysfs
> remove/rescan or held-open fd needed.
> 
>   kmem_cache of name 'virtio-gpu-vbufs' already exists
>   WARNING: mm/slab_common.c:111 at __kmem_cache_create_args
>   Call Trace:
>    virtio_gpu_alloc_vbufs
>    virtio_gpu_init
>    virtio_gpu_probe
>    ...
> 
> syzbot found the same WARNing through a different path: open an
> fbdev node, then remove and rescan its PCI device over sysfs. The
> drm_device reference the open fd holds delays virtio_gpu_release(),
> so the old cache is still around when the rescan probes the device
> again and calls virtio_gpu_alloc_vbufs() a second time.
> 
> Either way kmem_cache_create() still returns a valid (merged) cache,
> so the device keeps working; the WARN is not a probe failure. But it
> is a real defect: as the comment next to the check says, a duplicate
> name "confuses slabtop, et al", and it is exactly what syzbot and
> any panic_on_warn setup treat as a crash.
> 
> The vbuf size is the same for every device, so create the cache once
> at module init and destroy it at module exit instead of per device.
> 
> Reported-by: syzbot+1b129b44597a126d2d79@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1b129b44597a126d2d79
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
> No Fixes: tag. I could not pin down which commit introduced the
> dedicated per-device cache (dc5698e80cf7, the original driver commit,
> still used a plain kzalloc() pool); happy to add the right one if
> someone can point to it.
> 
> v3: back to one cache shared at module scope (Dmitry), not kzalloc()/
>     kfree() (v2) -- kmem_cache keeps the allocation off the general
>     kmalloc slabs for this latency-sensitive path. Commit message
>     corrected: the duplicate-name WARN does not fail the probe (the
>     cache still gets created, merged with the existing one), and the
>     trigger is any two live virtio-gpu devices, not just the syzbot

I booted QEMU with two virtio-gpu devs, each created own kmem_cache and
there is no warning.

-- 
Best regards,
Dmitry