[PATCH 1/3] libkmod: kmod_builtin_get_modinfo: free modinfo on error

[PATCH 1/3] libkmod: kmod_builtin_get_modinfo: free modinfo on error

[Yauheni Kaliuta]

The function allocates array but on building it if get_string()
fails it returns the error leaving the array allocated. The caller
does not care about it in error case either.

Free it to fix memory leak.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 libkmod/libkmod-builtin.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libkmod/libkmod-builtin.c b/libkmod/libkmod-builtin.c
index aaec5ddb0609..fc9a37644261 100644
--- a/libkmod/libkmod-builtin.c
+++ b/libkmod/libkmod-builtin.c
@@ -314,6 +314,7 @@ ssize_t kmod_builtin_get_modinfo(struct kmod_ctx *ctx, const char *modname,
 		offset = get_string(iter, pos, &line, &linesz);
 		if (offset <= 0) {
 			count = (offset) ? -errno : -EOF;
+			free(*modinfo);
 			goto fail;
 		}
 
-- 
2.29.2

[PATCH 2/3] depmod: output_builtin_alias_bin: free idx on error path

[Yauheni Kaliuta]

idx is allocated in the beginning but it's not freed if there is
a failure after the allocation.

Change the error path: return immediately if idx allocation fails
and then free it in both success and error path at the end.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/depmod.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tools/depmod.c b/tools/depmod.c
index 875e31480818..2c03dfe7dc28 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -2412,10 +2412,8 @@ static int output_builtin_alias_bin(struct depmod *depmod, FILE *out)
 		return 0;
 
 	idx = index_create();
-	if (idx == NULL) {
-		ret = -ENOMEM;
-		goto fail;
-	}
+	if (idx == NULL)
+		return -ENOMEM;
 
 	ret = kmod_module_get_builtin(depmod->ctx, &builtin);
 	if (ret < 0) {
@@ -2458,13 +2456,12 @@ static int output_builtin_alias_bin(struct depmod *depmod, FILE *out)
 
 	if (count)
 		index_write(idx, out);
-
-	index_destroy(idx);
-
 fail:
 	if (builtin)
 		kmod_module_unref_list(builtin);
 
+	index_destroy(idx);
+
 	return ret;
 }
 
-- 
2.29.2

[PATCH 3/3] libkmod: kmod_log_null: qualify ctx argument as const

[Yauheni Kaliuta]

kmod_log_null() does not change ctx (does nothing).

Fix warnings

In file included from libkmod/libkmod-index.c:33:
libkmod/libkmod-index.c: In function ‘index_mm_open’:
libkmod/libkmod-index.c:757:6: warning: passing argument 1 of ‘kmod_log_null’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  757 |  DBG(ctx, "file=%s\n", filename);

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 libkmod/libkmod-internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index b22ac2a5f89e..398af9ce756a 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -11,7 +11,7 @@
 #include "libkmod.h"
 
 static _always_inline_ _printf_format_(2, 3) void
-	kmod_log_null(struct kmod_ctx *ctx, const char *format, ...) {}
+	kmod_log_null(const struct kmod_ctx *ctx, const char *format, ...) {}
 
 #define kmod_log_cond(ctx, prio, arg...) \
 	do { \
-- 
2.29.2

Re: [PATCH 1/3] libkmod: kmod_builtin_get_modinfo: free modinfo on error

[Lucas De Marchi]

Thanks, those 3 fixes were applied.

Lucas De Marchi

On Sun, Nov 29, 2020 at 8:47 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> The function allocates array but on building it if get_string()
> fails it returns the error leaving the array allocated. The caller
> does not care about it in error case either.
>
> Free it to fix memory leak.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---
>  libkmod/libkmod-builtin.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libkmod/libkmod-builtin.c b/libkmod/libkmod-builtin.c
> index aaec5ddb0609..fc9a37644261 100644
> --- a/libkmod/libkmod-builtin.c
> +++ b/libkmod/libkmod-builtin.c
> @@ -314,6 +314,7 @@ ssize_t kmod_builtin_get_modinfo(struct kmod_ctx *ctx, const char *modname,
>                 offset = get_string(iter, pos, &line, &linesz);
>                 if (offset <= 0) {
>                         count = (offset) ? -errno : -EOF;
> +                       free(*modinfo);
>                         goto fail;
>                 }
>
> --
> 2.29.2
>