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 <[email protected]> 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/[email protected]?part=1
