From: Lv Yunlong <lyl2019@xxxxxxxxxxxxxxxx> Sent: Wednesday, March 24, 2021 3:37 AM
>
> In function hvfb_probe in hyperv_fb.c, it calls hvfb_getmem(hdev, info)
> and return err when info->apertures is freed.
>
> In the error1 label of hvfb_probe, info->apertures will be freed for the
> second time in framebuffer_release(info).
>
> My patch removes all kfree(info->apertures) instead of set info->apertures
> to NULL. It is because that let framebuffer_release() handle freeing the
> memory flows the fbdev pattern, and less code overall.
Let me suggest some clarifications in the commit message. It's probably
better not to reference the initial approach of setting info->apertures to
NULL, since there won't be any record of that approach in the commit
history. Here's what I would suggest:
Function hvfb_probe() calls hvfb_getmem(), expecting upon return that
info->apertures is either NULL or points to memory that should be freed
by framebuffer_release(). But hvfb_getmem() is freeing the memory and
leaving the pointer non-NULL, resulting in a double free if an error
occurs or later if hvfb_remove() is called.
Fix this by removing all kfree(info->apertures) calls in hvfb_getmem().
This will allow framebuffer_release() to free the memory, which follows
the pattern of other fbdev drivers.
Modulo this revision to the commit message, which Wei Liu can
probably incorporate,
Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx>
>
> Signed-off-by: Lv Yunlong <lyl2019@xxxxxxxxxxxxxxxx>
> ---
> drivers/video/fbdev/hyperv_fb.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index c8b0ae676809..4dc9077dd2ac 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -1031,7 +1031,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> if (!pdev) {
> pr_err("Unable to find PCI Hyper-V video\n");
> - kfree(info->apertures);
> return -ENODEV;
> }
>
> @@ -1129,7 +1128,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> } else {
> pci_dev_put(pdev);
> }
> - kfree(info->apertures);
>
> return 0;
>
> @@ -1141,7 +1139,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info
> *info)
> err1:
> if (!gen2vm)
> pci_dev_put(pdev);
> - kfree(info->apertures);
>
> return -ENOMEM;
> }
> --
> 2.25.1
>
