From: Wei Liu <wei.liu@kernel.org> To: Michael Kelley <mikelley@microsoft.com> Cc: Lv Yunlong <lyl2019@mail.ustc.edu.cn>, KY Srinivasan <kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, Stephen Hemminger <sthemmin@microsoft.com>, "wei.liu@kernel.org" <wei.liu@kernel.org>, "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, "linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2] video: hyperv_fb: Fix a double free in hvfb_probe Date: Thu, 25 Mar 2021 13:30:36 +0000 [thread overview] Message-ID: <20210325133036.rbriezf3v32wofkl@liuwe-devbox-debian-v2> (raw) In-Reply-To: <MWHPR21MB1593F19EE7AD10698582FA78D7639@MWHPR21MB1593.namprd21.prod.outlook.com> On Wed, Mar 24, 2021 at 01:46:39PM +0000, Michael Kelley wrote: > From: Lv Yunlong <lyl2019@mail.ustc.edu.cn> 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, > Yes. I surely can incorporate the changes. I will also add the Fixes tag. > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
WARNING: multiple messages have this Message-ID (diff)
From: Wei Liu <wei.liu@kernel.org> To: Michael Kelley <mikelley@microsoft.com> Cc: "wei.liu@kernel.org" <wei.liu@kernel.org>, Stephen Hemminger <sthemmin@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Lv Yunlong <lyl2019@mail.ustc.edu.cn>, "linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>, KY Srinivasan <kys@microsoft.com> Subject: Re: [PATCH v2] video: hyperv_fb: Fix a double free in hvfb_probe Date: Thu, 25 Mar 2021 13:30:36 +0000 [thread overview] Message-ID: <20210325133036.rbriezf3v32wofkl@liuwe-devbox-debian-v2> (raw) In-Reply-To: <MWHPR21MB1593F19EE7AD10698582FA78D7639@MWHPR21MB1593.namprd21.prod.outlook.com> On Wed, Mar 24, 2021 at 01:46:39PM +0000, Michael Kelley wrote: > From: Lv Yunlong <lyl2019@mail.ustc.edu.cn> 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, > Yes. I surely can incorporate the changes. I will also add the Fixes tag. > Reviewed-by: Michael Kelley <mikelley@microsoft.com> _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-03-25 13:31 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-24 10:37 [PATCH v2] video: hyperv_fb: Fix a double free in hvfb_probe Lv Yunlong 2021-03-24 10:37 ` Lv Yunlong 2021-03-24 13:46 ` Michael Kelley 2021-03-24 13:46 ` Michael Kelley 2021-03-25 13:30 ` Wei Liu [this message] 2021-03-25 13:30 ` Wei Liu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210325133036.rbriezf3v32wofkl@liuwe-devbox-debian-v2 \ --to=wei.liu@kernel.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=haiyangz@microsoft.com \ --cc=kys@microsoft.com \ --cc=linux-fbdev@vger.kernel.org \ --cc=linux-hyperv@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lyl2019@mail.ustc.edu.cn \ --cc=mikelley@microsoft.com \ --cc=sthemmin@microsoft.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.