Messages in this thread | | | Date | Fri, 17 Mar 2023 09:28:21 +0800 | Subject | Re: [PATCH linux-next v2 1/3] platform/x86/intel/tpmi: Fix double free in tpmi_create_device() | From | Dongliang Mu <> |
| |
On 2023/3/17 02:18, srinivas pandruvada wrote: > Hi Hans, > > On Thu, 2023-03-16 at 15:25 +0100, Hans de Goede wrote: >> Hi, >> >> On 3/9/23 05:01, Dongliang Mu wrote: >>> The previous commit 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix >>> double free reported by Smatch") incorrectly handle the >>> deallocation of >>> res variable. As shown in the comment, intel_vsec_add_aux handles >>> all >>> the deallocation of res and feature_vsec_dev. Therefore, kfree(res) >>> can >>> still cause double free if intel_vsec_add_aux returns error. >>> >>> Fix this by adjusting the error handling part in >>> tpmi_create_device, >>> following the function intel_vsec_add_dev. >>> >>> Fixes: 6a192c0cbf38 ("platform/x86/intel/tpmi: Fix double free >>> reported by Smatch") >>> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > >> IIRC then after this v2 was posted I still saw some comments on the >> original v1 which was not posted on the list. Without the v1 comments >> being on the list and this archived, I have lost track of what the >> status of these patches is. >> >> Srinivas, can you let me know if I should merge these, or if more >> changes are necessary ? >> >> From the off-list discussion of v1 I got the impression more changes >> are necessary, but I'm not sure. > I was looking for changes submitted by the following patch > " > [PATCH linux-next v2 3/3] drivers/platform/x86/intel: fix a memory leak > in intel_vsec_add_aux > " > > Since I was not copied on this, I was unaware. So I was requesting this > change. > > Thanks, > Srinivas
Hi Srinivas and Hans,
How about folding these three patches into one patch and resend a v3 patch?
This will get all people together and avoid the previous embarrassing sitation.
Dongliang Mu
> >> Regards, >> >> Hans >> >> >> >> >>> --- >>> drivers/platform/x86/intel/tpmi.c | 17 ++++------------- >>> 1 file changed, 4 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/platform/x86/intel/tpmi.c >>> b/drivers/platform/x86/intel/tpmi.c >>> index c999732b0f1e..882fe5e4763f 100644 >>> --- a/drivers/platform/x86/intel/tpmi.c >>> +++ b/drivers/platform/x86/intel/tpmi.c >>> @@ -215,8 +215,8 @@ static int tpmi_create_device(struct >>> intel_tpmi_info *tpmi_info, >>> >>> feature_vsec_dev = kzalloc(sizeof(*feature_vsec_dev), >>> GFP_KERNEL); >>> if (!feature_vsec_dev) { >>> - ret = -ENOMEM; >>> - goto free_res; >>> + kfree(res); >>> + return -ENOMEM; >>> } >>> >>> snprintf(feature_id_name, sizeof(feature_id_name), "tpmi- >>> %s", name); >>> @@ -242,17 +242,8 @@ static int tpmi_create_device(struct >>> intel_tpmi_info *tpmi_info, >>> * feature_vsec_dev memory is also freed as part of device >>> * delete. >>> */ >>> - ret = intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev- >>>> auxdev.dev, >>> - feature_vsec_dev, >>> feature_id_name); >>> - if (ret) >>> - goto free_res; >>> - >>> - return 0; >>> - >>> -free_res: >>> - kfree(res); >>> - >>> - return ret; >>> + return intel_vsec_add_aux(vsec_dev->pcidev, &vsec_dev- >>>> auxdev.dev, >>> + feature_vsec_dev, >>> feature_id_name); >>> } >>> >>> static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info)
| |