Bug #10430

tails-shell-library should not use rmmod instead of modprobe -r

Added by alant 2015-10-27 07:43:29 . Updated 2015-11-03 11:28:31 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Test suite
Target version:
Tails_1.7
Start date:
2015-10-27
Due date:
% Done:

100%

Feature Branch:
bugfix/10430-do-not-use-rmmod
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:
244

Description

While testing 1.7~rc1, we found that tails-shell-library/hardware.sh switched from modprobe -r to rmmod in the following commit:

commit ed9c96f81a2600cd74fd06e4174f3d994735215f
Author: Tails developers <amnesia@boum.org>
Date:   Mon Jan 12 03:01:17 2015 +0100

    Use rmmod so we can replace it with /bin/false symlink

    So we can make the panic mode module removal fail, for the "MAC
    addresse spoofing fails and the module is not removed" scenario.

diff --git a/config/chroot_local-includes/usr/local/lib/tails-shell-library/hardware.sh b/config/chroot_local-includes/usr/local/lib/tails-s
index 66a8079..f56e624 100644
--- a/config/chroot_local-includes/usr/local/lib/tails-shell-library/hardware.sh
+++ b/config/chroot_local-includes/usr/local/lib/tails-shell-library/hardware.sh
@@ -74,5 +74,7 @@ mod_rev_dep() {
 # Unloads module $1, and all modules that (transatively) depends on
 # $1 (i.e. its reverse dependencies).
 unload_module_and_rev_deps() {
-  /sbin/modprobe -r $(mod_rev_dep ${1})
+  for mod in $(mod_rev_dep ${1}); do
+      /sbin/rmmod ${mod}
+  done
 }

However, using rmmod is not equivalent to modprobe -r. Man modprobe reads “modprobe looks in the module directory /lib/modules/`uname -r` for all the modules and other files, except for the optional configuration files in the /etc/modprobe.d directory (see modprobe.d(5)). modprobe will also use module options specified on the kernel command line in the form of .

We have agreed that “the best would be to switch back to modprobe, and then make our MAC spoofing test inject a modprobe wrapper that acts differently only if the ‘-r’ option is present”

Subtasks

History

#1 Updated by anonym 2015-10-29 09:50:21

  • Status changed from Confirmed to In Progress

Applied in changeset commit:78cf869bd601d606d1ba7c31a7f2294d8cc72aab.

#2 Updated by anonym 2015-10-29 09:52:31

  • Assignee changed from anonym to alant
  • % Done changed from 0 to 50
  • QA Check set to Ready for QA
  • Feature Branch set to bugfix/10430-do-not-use-rmmod

Would you mind reviewing this, alant?

#3 Updated by alant 2015-10-29 11:59:20

  • Assignee changed from alant to anonym
  • QA Check changed from Ready for QA to Dev Needed
  1. Code review passes for me for commit 78cf869bd601d606d1ba7c31a7f2294d8cc72aab
  2. bugfix/10430-do-not-use-rmmod does not contain the right commits, please fix
  3. I don’t know how to test this as I don’t currently run the test suite

#4 Updated by anonym 2015-10-30 07:17:14

  • Assignee changed from anonym to alant
  • QA Check changed from Dev Needed to Ready for QA

alant wrote:
> # Code review passes for me for commit 78cf869bd601d606d1ba7c31a7f2294d8cc72aab

Ok, but what about the other commits…

> # bugfix/10430-do-not-use-rmmod does not contain the right commits, please fix

… now that you have properly pulled this branch (as you said over XMPP). :)

> # I don’t know how to test this as I don’t currently run the test suite

Well, I’ve automatically tested it. :) Jenkins has some completely unrelated issues with automatically testing this branch, but eventually you can see how it did here: https://jenkins.tails.boum.org/job/test_Tails_ISO_bugfix-10430-do-not-use-rmmod/

#5 Updated by alant 2015-10-30 11:12:56

  • Assignee deleted (alant)
  • % Done changed from 50 to 100
  • QA Check changed from Ready for QA to Pass

> > # Code review passes for me for commit 78cf869bd601d606d1ba7c31a7f2294d8cc72aab
>
> Ok, but what about the other commits…
>
Fine.

> > # bugfix/10430-do-not-use-rmmod does not contain the right commits, please fix
>
> … now that you have properly pulled this branch (as you said over XMPP). :)
>
Sorry for the noise.

> > # I don’t know how to test this as I don’t currently run the test suite
>
> Well, I’ve automatically tested it. :) Jenkins has some completely unrelated issues with automatically testing this branch, but eventually you can see how it did here: https://jenkins.tails.boum.org/job/test_Tails_ISO_bugfix-10430-do-not-use-rmmod/

Another Tails developper ran the test suite successfully on this branch.

#6 Updated by alant 2015-10-30 11:13:38

  • Status changed from In Progress to Fix committed

Applied in changeset commit:853af96ecf31ccc5cbd2f7ead84bcd32945eff61.

#7 Updated by anonym 2015-11-03 11:28:31

  • Status changed from Fix committed to Resolved