[Bug 15817] New: Inconsistent error reporting for modprobe -r

[bugzilla at busybox.net]

https://bugs.busybox.net/show_bug.cgi?id=15817

            Bug ID: 15817
           Summary: Inconsistent error reporting for modprobe -r
           Product: Busybox
           Version: 1.35.x
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: minor
          Priority: P5
         Component: Networking
          Assignee: unassigned at busybox.net
          Reporter: bouwenwannes at gmail.com
                CC: busybox-cvs at busybox.net
  Target Milestone: ---

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

-- 
You are receiving this mail because:
You are on the CC list for the bug.