Skip to contentSkip to conversation

modpost: prevent leak when early return no suffix .o in read_symbols()

PATCH In review Robertus Diawan Chris 3 participants 3 messages Jun 24 – Jul 24 Kbuild

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 modpost: prevent leak when early return no suffix .o in read_symbols()

The allocation for elf info symsearch and hdr from parse_elf() haven't been released when return because of modname didn't have suffix ".o". And it seems like the suffix ".o" check did not depends on parse_elf() to succeed first. So, move the suffix ".o" check before checking parse_elf() result to prevent resource leak when the modname didn't have suffix ".o" and return early.

This is reported by Coverity Scan as "Resource leak".

Fixes 8c9ce89c5b63 ("modpost: simplify mod->name allocation")

---
v1 -> v2:
- Move the suffix ".o" check before parse_elf() instead of adding parse_elf_finish() in the suffix ".o" check because the suffix ".o" check did not depends on parse_elf() and to minimize overhead handling the check (suggested by Nathan Chancellor).
- To make the subject title represents the new change better, change the subject title from "modpost: release allocation when early return no suffix .o in read_symbols()" to "modpost: prevent leak when early return no suffix .o in read_symbols()".

v1:
https://lore.kernel.org/all/[email protected]/

 scripts/mod/modpost.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
scripts/mod/modpost.c +3 -3
index abbcd3fc1394..6eb2714521a7 100644
@@ -1580,14 +1580,14 @@ static void read_symbols(const char *modname)
 	struct elf_info info = { };
 	Elf_Sym *sym;
 
-	if (!parse_elf(&info, modname))
-		return;
-
 	if (!strends(modname, ".o")) {
 		error("%s: filename must be suffixed with .o\n", modname);
 		return;
 	}
 
+	if (!parse_elf(&info, modname))
+		return;
+
 	/* strip trailing .o */
 	mod = new_module(modname, strlen(modname) - strlen(".o"));
 

base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6

Signature · 1 line
2.54.0
maintainer Reviewed-by on v2 1/1
#1 lore raw
Quoted 8 lines · Robertus Diawan Chris

The allocation for elf info symsearch and hdr from parse_elf() haven't been released when return because of modname didn't have suffix ".o". And it seems like the suffix ".o" check did not depends on parse_elf() to succeed first. So, move the suffix ".o" check before checking parse_elf() result to prevent resource leak when the modname didn't have suffix ".o" and return early.

[...]

Signature · 2 lines
Cheers,
Nathan
maintainer
#2 lore raw
In reply to Robertus Diawan Chris
Quoted 1 line · Robertus Diawan Chris

modpost: prevent leak when early return no suffix .o in read_symbols()

Applied to kbuild/linux.git (kbuild-next-unstable), thanks!

[1/1] modpost: prevent leak when early return no suffix .o in read_symbols()

      https://git.kernel.org/kbuild/c/f2308dd4

Please look out for regression or issue reports or other follow up comments, as they may result in the patch/series getting dropped, reverted or modified (e.g. trailers).

Patches applied to the kbuild-next-unstable branch are accepted pending wider testing in linux-next and any post-commit review; they will generally be moved to the kbuild-next branch in about a week if no issues are found.

Best regards,

Signature · 1 line
Nicolas