| Message ID | 0f780b18-0b1c-e2ff-31b1-1d697becd142@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [cifs-utils,v2] mount.cifs.c: fix memory leaks in main func | expand |
пн, 5 авг. 2019 г. в 19:36, Zhiqiang Liu <liuzhiqiang26@huawei.com>: > > From: Jiawen Liu <liujiawen10@huawei.com> > > In mount.cifs module, orgoptions and mountpoint in the main func > point to the memory allocated by func realpath and strndup respectively. > However, they are not freed before the main func returns so that the > memory leaks occurred. > > The memory leak problem is reported by LeakSanitizer tool. > LeakSanitizer url: "https://github.com/google/sanitizers" > > Here I free the pointers orgoptions and mountpoint before main > func returns. > > Fixes：7549ad5e7126 ("memory leaks: caused by func realpath and strndup") > Signed-off-by: Jiawen Liu <liujiawen10@huawei.com> > Reported-by: Jin Du <dujin1@huawei.com> > Reviewed-by: Saisai Zhang <zhangsaisai@huawei.com> > Reviewed-by: Aurélien Aptel <aaptel@suse.com> > --- > v1->v2: > - free orgoptions in main func as suggested by Aurélien Aptel > - free mountpoint in acquire_mountpoint func as suggested by Aurélien Aptel > > mount.cifs.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/mount.cifs.c b/mount.cifs.c > index ae7a899..656d353 100644 > --- a/mount.cifs.c > +++ b/mount.cifs.c > @@ -1891,7 +1891,10 @@ restore_privs: > uid_t __attribute__((unused)) uignore = setfsuid(oldfsuid); > gid_t __attribute__((unused)) gignore = setfsgid(oldfsgid); > } > - > + > + if (rc) { > + free(*mountpointp); > + } > return rc; > } > > @@ -1994,8 +1997,10 @@ int main(int argc, char **argv) > > /* chdir into mountpoint as soon as possible */ > rc = acquire_mountpoint(&mountpoint); > - if (rc) > + if (rc) { > + free(orgoptions); > return rc; > + } > > /* > * mount.cifs does privilege separation. Most of the code to handle > @@ -2014,6 +2019,8 @@ int main(int argc, char **argv) > /* child */ > rc = assemble_mountinfo(parsed_info, thisprogram, mountpoint, > orig_dev, orgoptions); > + free(orgoptions); > + free(mountpoint); > return rc; > } else { > /* parent */ > @@ -2149,5 +2156,6 @@ mount_exit: > } > free(options); > free(orgoptions); > + free(mountpoint); > return rc; > } > -- > 2.7.4 > Thanks for the patch! I will apply it to my github tree shortly. -- Best regards, Pavel Shilovsky
Merged into "next" with one minor change - removed a trailing white space. Thanks. -- Best regards, Pavel Shilovsky вт, 6 авг. 2019 г. в 09:49, Pavel Shilovsky <piastryyy@gmail.com>: > > пн, 5 авг. 2019 г. в 19:36, Zhiqiang Liu <liuzhiqiang26@huawei.com>: > > > > From: Jiawen Liu <liujiawen10@huawei.com> > > > > In mount.cifs module, orgoptions and mountpoint in the main func > > point to the memory allocated by func realpath and strndup respectively. > > However, they are not freed before the main func returns so that the > > memory leaks occurred. > > > > The memory leak problem is reported by LeakSanitizer tool. > > LeakSanitizer url: "https://github.com/google/sanitizers" > > > > Here I free the pointers orgoptions and mountpoint before main > > func returns. > > > > Fixes：7549ad5e7126 ("memory leaks: caused by func realpath and strndup") > > Signed-off-by: Jiawen Liu <liujiawen10@huawei.com> > > Reported-by: Jin Du <dujin1@huawei.com> > > Reviewed-by: Saisai Zhang <zhangsaisai@huawei.com> > > Reviewed-by: Aurélien Aptel <aaptel@suse.com> > > --- > > v1->v2: > > - free orgoptions in main func as suggested by Aurélien Aptel > > - free mountpoint in acquire_mountpoint func as suggested by Aurélien Aptel > > > > mount.cifs.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/mount.cifs.c b/mount.cifs.c > > index ae7a899..656d353 100644 > > --- a/mount.cifs.c > > +++ b/mount.cifs.c > > @@ -1891,7 +1891,10 @@ restore_privs: > > uid_t __attribute__((unused)) uignore = setfsuid(oldfsuid); > > gid_t __attribute__((unused)) gignore = setfsgid(oldfsgid); > > } > > - > > + > > + if (rc) { > > + free(*mountpointp); > > + } > > return rc; > > } > > > > @@ -1994,8 +1997,10 @@ int main(int argc, char **argv) > > > > /* chdir into mountpoint as soon as possible */ > > rc = acquire_mountpoint(&mountpoint); > > - if (rc) > > + if (rc) { > > + free(orgoptions); > > return rc; > > + } > > > > /* > > * mount.cifs does privilege separation. Most of the code to handle > > @@ -2014,6 +2019,8 @@ int main(int argc, char **argv) > > /* child */ > > rc = assemble_mountinfo(parsed_info, thisprogram, mountpoint, > > orig_dev, orgoptions); > > + free(orgoptions); > > + free(mountpoint); > > return rc; > > } else { > > /* parent */ > > @@ -2149,5 +2156,6 @@ mount_exit: > > } > > free(options); > > free(orgoptions); > > + free(mountpoint); > > return rc; > > } > > -- > > 2.7.4 > > > > Thanks for the patch! I will apply it to my github tree shortly. > > -- > Best regards, > Pavel Shilovsky
On 2019/8/8 5:44, Pavel Shilovsky wrote: > Merged into "next" with one minor change - removed a trailing white > space. Thanks. > That is ok. Thank you very much. > -- > Best regards, > Pavel Shilovsky > > вт, 6 авг. 2019 г. в 09:49, Pavel Shilovsky <piastryyy@gmail.com>: > >> >> пн, 5 авг. 2019 г. в 19:36, Zhiqiang Liu <liuzhiqiang26@huawei.com>: >>> >>> From: Jiawen Liu <liujiawen10@huawei.com> >>> >>> In mount.cifs module, orgoptions and mountpoint in the main func >>> point to the memory allocated by func realpath and strndup respectively. >>> However, they are not freed before the main func returns so that the >>> memory leaks occurred. >>> >>> The memory leak problem is reported by LeakSanitizer tool. >>> LeakSanitizer url: "https://github.com/google/sanitizers" >>> >>> Here I free the pointers orgoptions and mountpoint before main >>> func returns. >>> >>> Fixes：7549ad5e7126 ("memory leaks: caused by func realpath and strndup") >>> Signed-off-by: Jiawen Liu <liujiawen10@huawei.com> >>> Reported-by: Jin Du <dujin1@huawei.com> >>> Reviewed-by: Saisai Zhang <zhangsaisai@huawei.com> >>> Reviewed-by: Aurélien Aptel <aaptel@suse.com> >>> --- >>> v1->v2: >>> - free orgoptions in main func as suggested by Aurélien Aptel >>> - free mountpoint in acquire_mountpoint func as suggested by Aurélien Aptel >>> >>> mount.cifs.c | 12 ++++++++++-- >>> 1 file changed, 10 insertions(+), 2 deletions(-) >>> >>> diff --git a/mount.cifs.c b/mount.cifs.c >>> index ae7a899..656d353 100644 >>> --- a/mount.cifs.c >>> +++ b/mount.cifs.c >>> @@ -1891,7 +1891,10 @@ restore_privs: >>> uid_t __attribute__((unused)) uignore = setfsuid(oldfsuid); >>> gid_t __attribute__((unused)) gignore = setfsgid(oldfsgid); >>> } >>> - >>> + >>> + if (rc) { >>> + free(*mountpointp); >>> + } >>> return rc; >>> } >>> >>> @@ -1994,8 +1997,10 @@ int main(int argc, char **argv) >>> >>> /* chdir into mountpoint as soon as possible */ >>> rc = acquire_mountpoint(&mountpoint); >>> - if (rc) >>> + if (rc) { >>> + free(orgoptions); >>> return rc; >>> + } >>> >>> /* >>> * mount.cifs does privilege separation. Most of the code to handle >>> @@ -2014,6 +2019,8 @@ int main(int argc, char **argv) >>> /* child */ >>> rc = assemble_mountinfo(parsed_info, thisprogram, mountpoint, >>> orig_dev, orgoptions); >>> + free(orgoptions); >>> + free(mountpoint); >>> return rc; >>> } else { >>> /* parent */ >>> @@ -2149,5 +2156,6 @@ mount_exit: >>> } >>> free(options); >>> free(orgoptions); >>> + free(mountpoint); >>> return rc; >>> } >>> -- >>> 2.7.4 >>> >> >> Thanks for the patch! I will apply it to my github tree shortly. >> >> -- >> Best regards, >> Pavel Shilovsky > > . >
diff --git a/mount.cifs.c b/mount.cifs.c index ae7a899..656d353 100644 --- a/mount.cifs.c +++ b/mount.cifs.c @@ -1891,7 +1891,10 @@ restore_privs: uid_t __attribute__((unused)) uignore = setfsuid(oldfsuid); gid_t __attribute__((unused)) gignore = setfsgid(oldfsgid); } - + + if (rc) { + free(*mountpointp); + } return rc; } @@ -1994,8 +1997,10 @@ int main(int argc, char **argv) /* chdir into mountpoint as soon as possible */ rc = acquire_mountpoint(&mountpoint); - if (rc) + if (rc) { + free(orgoptions); return rc; + } /* * mount.cifs does privilege separation. Most of the code to handle @@ -2014,6 +2019,8 @@ int main(int argc, char **argv) /* child */ rc = assemble_mountinfo(parsed_info, thisprogram, mountpoint, orig_dev, orgoptions); + free(orgoptions); + free(mountpoint); return rc; } else { /* parent */ @@ -2149,5 +2156,6 @@ mount_exit: } free(options); free(orgoptions); + free(mountpoint); return rc; }