| Message ID | 20250301053547.1560-1-xjdeng@buaa.edu.cn |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Fix Potential Memory Leak | expand |
Hi Deng, On 01.03.25 06:35, Deng XingJing wrote: > Here are two memory leak issues I discovered: > > The first issue is in handlers/remote_handler#static int install_remote_image(struct img_type *img, void __attribute__ ((__unused__)) *data). I focused on line 170, where the variable connect_string = malloc(len); is allocated. This variable is used for ZeroMQ communication. However, regardless of whether the connection is successful or not, it is never freed. To address this, I added a free operation at the cleanup location. Agree, this fixes memory leak. > > The second issue is in corelib/mtd-interface#static void scan_ubi_volumes(struct mtd_ubi_info *info). I focused on line 283, where the variable ubi_part = (struct ubi_part *)calloc(1, sizeof(struct ubi_part)); is allocated. The problem arises in the if (err == -1) branch. If this branch is entered, whether the loop is skipped or a return occurs, this variable is not freed. > Fine as well. However, I will strongly suggest you split this patch into two different patches, because the issues are unrelated. Best regards, Stefano Babic > Special thanks to Stefano Babic for providing developer guidance. > > Signed-off-by: Deng xingjing <xjdeng@buaa.edu.cn> > > --- > corelib/mtd-interface.c | 1 + > handlers/remote_handler.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/corelib/mtd-interface.c b/corelib/mtd-interface.c > index c8979687..1364d209 100644 > --- a/corelib/mtd-interface.c > +++ b/corelib/mtd-interface.c > @@ -289,6 +289,7 @@ static void scan_ubi_volumes(struct mtd_ubi_info *info) > err = ubi_get_vol_info1(libubi, info->dev_info.dev_num, > i, &ubi_part->vol_info); > if (err == -1) { > + free(ubi_part); > if (errno == ENOENT || errno == ENODEV) > continue; > > diff --git a/handlers/remote_handler.c b/handlers/remote_handler.c > index a2ee1775..224c513d 100644 > --- a/handlers/remote_handler.c > +++ b/handlers/remote_handler.c > @@ -197,6 +197,7 @@ static int install_remote_image(struct img_type *img, > ret = copyimage(request, img, forward_data); > > cleanup: > + free(connect_string); > zmq_close(request); > zmq_ctx_destroy(context); >
Oh, this time I did neglect again, and next time if I find a problem, I will definitely separate them. Thanks. 在2025年3月3日星期一 UTC+8 04:15:03<Stefano Babic> 写道： > Hi Deng, > > On 01.03.25 06:35, Deng XingJing wrote: > > Here are two memory leak issues I discovered: > > > > The first issue is in handlers/remote_handler#static int > install_remote_image(struct img_type *img, void __attribute__ > ((__unused__)) *data). I focused on line 170, where the variable > connect_string = malloc(len); is allocated. This variable is used for > ZeroMQ communication. However, regardless of whether the connection is > successful or not, it is never freed. To address this, I added a free > operation at the cleanup location. > > Agree, this fixes memory leak. > > > > > The second issue is in corelib/mtd-interface#static void > scan_ubi_volumes(struct mtd_ubi_info *info). I focused on line 283, where > the variable ubi_part = (struct ubi_part *)calloc(1, sizeof(struct > ubi_part)); is allocated. The problem arises in the if (err == -1) branch. > If this branch is entered, whether the loop is skipped or a return occurs, > this variable is not freed. > > > > Fine as well. > > However, I will strongly suggest you split this patch into two different > patches, because the issues are unrelated. > > Best regards, > Stefano Babic > > > Special thanks to Stefano Babic for providing developer guidance. > > > > Signed-off-by: Deng xingjing <xjd...@buaa.edu.cn> > > > > --- > > corelib/mtd-interface.c | 1 + > > handlers/remote_handler.c | 1 + > > 2 files changed, 2 insertions(+) > > > > diff --git a/corelib/mtd-interface.c b/corelib/mtd-interface.c > > index c8979687..1364d209 100644 > > --- a/corelib/mtd-interface.c > > +++ b/corelib/mtd-interface.c > > @@ -289,6 +289,7 @@ static void scan_ubi_volumes(struct mtd_ubi_info > *info) > > err = ubi_get_vol_info1(libubi, info->dev_info.dev_num, > > i, &ubi_part->vol_info); > > if (err == -1) { > > + free(ubi_part); > > if (errno == ENOENT || errno == ENODEV) > > continue; > > > > diff --git a/handlers/remote_handler.c b/handlers/remote_handler.c > > index a2ee1775..224c513d 100644 > > --- a/handlers/remote_handler.c > > +++ b/handlers/remote_handler.c > > @@ -197,6 +197,7 @@ static int install_remote_image(struct img_type *img, > > ret = copyimage(request, img, forward_data); > > > > cleanup: > > + free(connect_string); > > zmq_close(request); > > zmq_ctx_destroy(context); > > > >
diff --git a/corelib/mtd-interface.c b/corelib/mtd-interface.c index c8979687..1364d209 100644 --- a/corelib/mtd-interface.c +++ b/corelib/mtd-interface.c @@ -289,6 +289,7 @@ static void scan_ubi_volumes(struct mtd_ubi_info *info) err = ubi_get_vol_info1(libubi, info->dev_info.dev_num, i, &ubi_part->vol_info); if (err == -1) { + free(ubi_part); if (errno == ENOENT || errno == ENODEV) continue; diff --git a/handlers/remote_handler.c b/handlers/remote_handler.c index a2ee1775..224c513d 100644 --- a/handlers/remote_handler.c +++ b/handlers/remote_handler.c @@ -197,6 +197,7 @@ static int install_remote_image(struct img_type *img, ret = copyimage(request, img, forward_data); cleanup: + free(connect_string); zmq_close(request); zmq_ctx_destroy(context);
Here are two memory leak issues I discovered: The first issue is in handlers/remote_handler#static int install_remote_image(struct img_type *img, void __attribute__ ((__unused__)) *data). I focused on line 170, where the variable connect_string = malloc(len); is allocated. This variable is used for ZeroMQ communication. However, regardless of whether the connection is successful or not, it is never freed. To address this, I added a free operation at the cleanup location. The second issue is in corelib/mtd-interface#static void scan_ubi_volumes(struct mtd_ubi_info *info). I focused on line 283, where the variable ubi_part = (struct ubi_part *)calloc(1, sizeof(struct ubi_part)); is allocated. The problem arises in the if (err == -1) branch. If this branch is entered, whether the loop is skipped or a return occurs, this variable is not freed. Special thanks to Stefano Babic for providing developer guidance. Signed-off-by: Deng xingjing <xjdeng@buaa.edu.cn> --- corelib/mtd-interface.c | 1 + handlers/remote_handler.c | 1 + 2 files changed, 2 insertions(+)