Re: [PATCH] module: fix lost error code from codetag_load_module()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing call to codetag_unload_module() in the error path leaves dangling module pointers in codetag_types IDR.
--

--- Patch [1]: [PATCH] module: fix lost error code from codetag_load_module() ---
commit 5006baafc1e00091b0abb9c8d393876e887ccb31
Author: Hao Ge <[email protected]>

module: fix lost error code from codetag_load_module()

This commit fixes a bug where the error code from codetag_load_module()
was previously lost. It ensures load_module() correctly captures the error
and aborts module loading upon failure.

> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3581,7 +3581,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  			goto sysfs_cleanup;
>  	}
>  
> -	if (codetag_load_module(mod))
> +	err = codetag_load_module(mod);
> +	if (err)
>  		goto sysfs_cleanup;

[Severity: High]
This is a pre-existing issue, but by fixing the error path here, we might
expose a use-after-free bug during module load failures.

If codetag_load_module() fails partway through the list of codetag_types:

lib/codetag.c:codetag_load_module() {
	...
	list_for_each_entry(cttype, &codetag_types, link) {
		ret = codetag_module_init(cttype, mod);
		if (ret)
			break;
	}
	...
}

It returns an error without rolling back the types that were already
successfully initialized. We then jump to sysfs_cleanup, which eventually
calls module_deallocate() in the free_module block to free the module memory.

However, there is no call to codetag_unload_module() anywhere in this cleanup
path to clean up the dangling references from the codetag_types IDR.

Will this leave dangling pointers that result in a use-after-free dereference
of the freed module memory when codetags are later iterated (for example,
when reading from /proc/allocinfo)?

lib/codetag.c:codetag_next_ct() {
	...
			ct = get_first_module_ct(cmod);
	...
}

>  
>  	/* Get rid of temporary copy. */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1