RE: [PATCH v2] video: hyperv_fb: Fix a double free in hvfb_probe
From: Michael Kelley <hidden>
Date: 2021-03-24 13:47:25
Also in:
dri-devel, linux-hyperv, lkml
From: Lv Yunlong <redacted> 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 <redacted>
quoted hunk ↗ jump to hunk
Signed-off-by: Lv Yunlong <redacted> --- 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