Re: [PATCH 6.6.y 1/2] fbdev: hyperv_fb: Simplify hvfb_putmem

[sashiko-bot@kernel.org]

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 <ssengar@linux.microsoft.com>

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/1740845791-19977-2-git-send-email-ssengar@linux.microsoft.com

> 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/20261008191309.98263-1-artem@trailofbits.com?part=1