Messages in this thread | | | Subject | Re: [PATCH] tools/bootconfig: fix resource leak in apply_xbc() | From | Yunfeng Ye <> | Date | Fri, 8 May 2020 14:41:31 +0800 |
| |
On 2020/5/8 4:30, Markus Elfring wrote: >> The @data and @fd is leak in the error path of apply_xbc(), so this >> patch fix it. > > I suggest to improve this change description. > > * Please use an imperative wording. > > * Would you like to add the tag “Fixes”? > > >> +++ b/tools/bootconfig/main.c >> @@ -314,6 +314,7 @@ int apply_xbc(const char *path, const char *xbc_path) >> ret = delete_xbc(path); >> if (ret < 0) { >> pr_err("Failed to delete previous boot config: %d\n", ret); >> + free(data); >> return ret; >> } > > I propose to adjust the exception handling. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=6e7f2eacf09811d092c1b41263108ac7fe0d089d#n450 > > - return ret; > + goto free_data; > > >> @@ -321,24 +322,26 @@ int apply_xbc(const char *path, const char *xbc_path) >> fd = open(path, O_RDWR | O_APPEND); >> if (fd < 0) { >> pr_err("Failed to open %s: %d\n", path, fd); >> + free(data); >> return fd; > > - return fd; > + ret = fd; > + goto free_data; > > >> } >> /* TODO: Ensure the @path is initramfs/initrd image */ >> ret = write(fd, data, size + 8); >> if (ret < 0) { >> pr_err("Failed to apply a boot config: %d\n", ret); >> - return ret; >> + goto out; > > + goto free_data; > > >> } >> /* Write a magic word of the bootconfig */ >> ret = write(fd, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN); >> if (ret < 0) { > > - if (ret < 0) { > + if (ret < 0) > > >> pr_err("Failed to apply a boot config magic: %d\n", ret); >> - return ret; >> + goto out; > > I suggest to avoid an extra jump at such a place. > > >> } > > - } > + > > >> +out: > > +close_fd: >> close(fd); > > +free_data: >> free(data); > > > How do you think about to complete the error handling also at other > source code places? > ok, I will modify and send the patch v2, thanks.
> Regards, > Markus > >
| |