Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] fbdev: hyperv_fb: Use-After-Free in dma_free_coherent via deferred 
fb_destroy
--

commit 5cd0184cceb2674876a9c7e3d72e937d61e3e2d8
Author: Saurabh Sengar <[email protected]>

fbdev: hyperv_fb: Simplify hvfb_putmem

This commit simplifies hvfb_putmem() by removing its hdev parameter. It
derives the device from fb_info and passes it to hvfb_release_phymem()
instead.

Link: 
https://lore.kernel.org/r/[email protected]

> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index 80e8ec36b7db2..80e6d96821795 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -973,7 +973,7 @@ static phys_addr_t hvfb_get_phymem(struct hv_device *hdev,
>  }
>  
>  /* Release contiguous physical memory */
> -static void hvfb_release_phymem(struct hv_device *hdev,
> +static void hvfb_release_phymem(struct device *device,
>                               phys_addr_t paddr, unsigned int size)
>  {
>       unsigned int order = get_order(size);
> @@ -981,7 +981,7 @@ static void hvfb_release_phymem(struct hv_device *hdev,
>       if (order <= MAX_ORDER)
>               __free_pages(pfn_to_page(paddr >> PAGE_SHIFT), order);
>       else
> -             dma_free_coherent(&hdev->device,
> +             dma_free_coherent(device,
>                                 round_up(size, PAGE_SIZE),
>                                 phys_to_virt(paddr),
>                                 paddr);

[ ... ]

> @@ -1100,7 +1100,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct 
> fb_info *info)
>  }
>  
>  /* Release the framebuffer */
> -static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info)
> +static void hvfb_putmem(struct fb_info *info)
>  {
>       struct hvfb_par *par = info->par;
>  
> @@ -1109,7 +1109,7 @@ static void hvfb_putmem(struct hv_device *hdev, struct 
> fb_info *info)
>               iounmap(par->mmio_vp);
>               vmbus_free_mmio(par->mem->start, screen_fb_size);
>       } else {
> -             hvfb_release_phymem(hdev, info->fix.smem_start,
> +             hvfb_release_phymem(info->device, info->fix.smem_start,
>                                   screen_fb_size);
>       }

[Severity: High]
Can this change lead to dma_free_coherent() in drivers/video/fbdev/hyperv_fb.c
dereferencing a freed device pointer when executed from a deferred fb_destroy()
callback?

Moving framebuffer memory teardown, via hvfb_putmem(), to the asynchronous
fb_destroy() callback causes it to execute when the last userspace reference
is dropped.

If /dev/fb0 is held open by a user process when the Hyper-V device is removed
or unbound, the driver's remove() callback unregisters the framebuffer and
returns. At this point, the driver core drops the final reference to the
parent hv_device and frees it.

Later, when the user process closes the file descriptor, fb_destroy() invokes
hvfb_putmem(), which calls dma_free_coherent() using info->device. This points
to the now-freed &hdev->device.

Because dma_free_coherent() dereferences this device pointer to access dma_ops,
does this result in a concrete use-after-free and likely a kernel panic?

Checking the final state of the series reveals no get_device(&hdev->device)
was added to ensure the device outlives the fb_info.

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to