Bug 15817 - Inconsistent error reporting for modprobe -r
Summary: Inconsistent error reporting for modprobe -r
Status: NEW
Alias: None
Product: Busybox
Classification: Unclassified
Component: Other (show other bugs)
Version: 1.35.x
Hardware: All Linux
: P5 minor
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-10-11 12:45 UTC by bouwenwannes
Modified: 2023-10-11 12:47 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description bouwenwannes 2023-10-11 12:45:26 UTC
Hello,

I noticed an issue when removing modules that contain dependencies with modprobe -r.

In do_modprobe (modutils/modprobe.c:416) we iterate through all items in m->deps. The first item is the module itself that we want to remove. Subsequent entries are modules that the current driver depends on.

So, in the first iteration the variable first is set to 1 and an error is generated if we fail to remove the module (rc = bb_delete_module(m2->modname, O_EXCL);). After the first module, the variable first is set to 0 and we continue removing all the dependencies.

If a dependency cannot be removed (with the same rc = bb_delete_module(m2->modname, O_EXCL); call), we ignore the return code and just continue to the next one.

When the while loop exits, we in the end return rc (modutils/modprobe.c:508). Hence, the return code of the last dependency that we tried to remove is returned.

This is in my opinion not correct. The last dependency could still be in use by another driver, which leads to a non-zero return code of bb_delete_module, while in practice there is no problem. I suggest following patch to always reset the return code to 0 for all the dependencies (so after first = 0) to get consistent error reporting.

diff --git a/modutils/modprobe.c b/modutils/modprobe.c
--- a/modutils/modprobe.c
+++ b/modutils/modprobe.c
@@ -462,6 +462,7 @@ static int do_modprobe(struct module_ent
 			}
 			/* do not error out if *deps* fail to unload */
 			first = 0;
+			rc = 0;
 			continue;
 		}

Kind regards,

Wannes