Re: [PATCH] drm/virtio: Fix possible memory allocation failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Adding __GFP_NOFAIL to a GFP_ATOMIC allocation is invalid and triggers a kernel warning

Pre-existing issues:
- [High] NULL pointer dereference remains unfixed and slab allocation in panic handler is fundamentally unsafe
--

--- Patch [1]: [PATCH] drm/virtio: Fix possible memory allocation failure ---
commit 2b29d148bc8be5eb73791a39e29cb1b504f8f37b
Author: Triet Hoang <triet.hoang.dev@gmail.com>

drm/virtio: Fix possible memory allocation failure

This patch attempts to fix a possible NULL pointer dereference in
virtio_gpu_panic_get_vbuf() by adding the __GFP_NOFAIL flag to the
kmem_cache_zalloc() call instead of checking the return value.

> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 905b1f42cd983..be051ac4bd78c 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -93,7 +93,7 @@ virtio_gpu_panic_get_vbuf(struct virtio_gpu_device *vgdev, int size)
>  {
>  	struct virtio_gpu_vbuffer *vbuf;
>  
> -	vbuf = kmem_cache_zalloc(vgdev->vbufs, GFP_ATOMIC);
> +	vbuf = kmem_cache_zalloc(vgdev->vbufs, GFP_ATOMIC | __GFP_NOFAIL);

[Severity: High]
Does this combination of flags work as intended? The page allocator explicitly
prohibits combining __GFP_NOFAIL with non-blocking flags like GFP_ATOMIC.

The allocator's slow path checks for !can_direct_reclaim (which is true for
GFP_ATOMIC), emits a WARN_ON_ONCE_GFP, and jumps to the failure path. This
returns NULL anyway, failing to guarantee allocation success while introducing
a new warning during a system panic.

>  
>  	vbuf->buf = (void *)vbuf + sizeof(*vbuf);

[Severity: High]
This isn't a bug introduced by this patch, but since the allocator ignores
__GFP_NOFAIL for atomic allocations, can kmem_cache_zalloc() still return NULL
here?

If it does, this will dereference NULL and cause a double fault, breaking
the panic handler.

Additionally, is it safe to call kmem_cache_zalloc() during a kernel panic?
If a stopped CPU was holding a slab lock, wouldn't the panicking CPU spin
forever attempting to acquire it, resulting in a hard system hang?

>  	vbuf->size = size;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818113256.27848-1-triet.hoang.dev@gmail.com?part=1