| Message ID | 12e34ddf-6e60-ab5f-020d-bd004fbbef06@huawei.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Jes Sorensen |
| Headers | show |
| Series | mdadm: fix memory leak and double free | expand |
On Tue, 31 May 2022 14:50:44 +0800 Wu Guanghao <wuguanghao3@huawei.com> wrote: > If disk_path = diskfd_to_devpath(), we need free(disk_path) before > return, otherwise there will be a memory leak > > Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com> > --- > super-intel.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/super-intel.c b/super-intel.c > index ef21ffba..98fb63d5 100644 > --- a/super-intel.c > +++ b/super-intel.c > @@ -700,8 +700,11 @@ static struct sys_dev* find_disk_attached_hba(int fd, > const char *devname) return 0; > > for (elem = list; elem; elem = elem->next) > - if (path_attached_to_hba(disk_path, elem->path)) > + if (path_attached_to_hba(disk_path, elem->path)) { > + if (disk_path != devname) > + free(disk_path); > return elem; > + } > > if (disk_path != devname) > free(disk_path); Patch is obviously correct but we can avoid code duplication: for (elem = list; elem; elem = elem->next) if (path_attached_to_hba(disk_path, elem->path)) break; if (disk_path != devname) free(disk_path); return elem; Last list element will be NULL anyway. Could you adopt it? Thanks, Mariusz
在 2022/5/31 16:04, Mariusz Tkaczyk 写道: > On Tue, 31 May 2022 14:50:44 +0800 > Wu Guanghao <wuguanghao3@huawei.com> wrote: > >> If disk_path = diskfd_to_devpath(), we need free(disk_path) before >> return, otherwise there will be a memory leak >> >> Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com> >> --- >> super-intel.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/super-intel.c b/super-intel.c >> index ef21ffba..98fb63d5 100644 >> --- a/super-intel.c >> +++ b/super-intel.c >> @@ -700,8 +700,11 @@ static struct sys_dev* find_disk_attached_hba(int fd, >> const char *devname) return 0; >> >> for (elem = list; elem; elem = elem->next) >> - if (path_attached_to_hba(disk_path, elem->path)) >> + if (path_attached_to_hba(disk_path, elem->path)) { >> + if (disk_path != devname) >> + free(disk_path); >> return elem; >> + } >> >> if (disk_path != devname) >> free(disk_path); > > Patch is obviously correct but we can avoid code duplication: > > for (elem = list; elem; elem = elem->next) > if (path_attached_to_hba(disk_path, elem->path)) > break; > > if (disk_path != devname) > free(disk_path); > > return elem; > > Last list element will be NULL anyway. Could you adopt it? > Sure, I follow your advices and keep you updated about patch V2. > Thanks, > Mariusz > > . >
diff --git a/super-intel.c b/super-intel.c index ef21ffba..98fb63d5 100644 --- a/super-intel.c +++ b/super-intel.c @@ -700,8 +700,11 @@ static struct sys_dev* find_disk_attached_hba(int fd, const char *devname) return 0; for (elem = list; elem; elem = elem->next) - if (path_attached_to_hba(disk_path, elem->path)) + if (path_attached_to_hba(disk_path, elem->path)) { + if (disk_path != devname) + free(disk_path); return elem; + } if (disk_path != devname) free(disk_path);
If disk_path = diskfd_to_devpath(), we need free(disk_path) before return, otherwise there will be a memory leak Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com> --- super-intel.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)