Bug #8449

Tails Upgrader fails to install some IUKs

Added by anonym over 2 years ago. Updated 3 months ago.

Status:ResolvedStart date:12/16/2014
Priority:ElevatedDue date:
Assignee:-% Done:

100%

Category:-
Target version:Tails_2.11
QA Check:Pass Blueprint:
Feature Branch:bugfix/8449-iuk-install-robustness, iuk:bugfix/8449-iuk-install-robustness Easy:
Type of work:Code Affected tool:Upgrader

Description

For instance, using tails-upgrade-frontend to fetch and install the hand-crafted and tiny (20 KB) 1.2.1_to_1.2.2 IUK results in a bricked Tails installation because Tails.module gets corrupted. After installing the upgrade, but before rebooting, the contents of the Tails.module looks normal so it seems like a failure of syncing the kernel's disk cache to disk. Furthermore, this does not occur when calling tails-install-iuk directly, which indicates that the cause has something to do with the fancy stuff wrapping around that call in the frontend, or possibly with perl's IPC::Run library.

Tests show that the following use of nocache (Debian package available in wheezy-backports) fixes the disk corruption:

--- a/lib/Tails/IUK/Install.pm
+++ b/lib/Tails/IUK/Install.pm
@@ -174,7 +174,7 @@ method upgrade_modules_file {
         say $temp_fh $append_str;
         close $temp_fh;

-        run_as_root('cp', '--force', $temp_file, $self->modules_file);
+        run_as_root('nocache', 'cp', '--force', $temp_file, $self->modules_file);


Related issues

Related to Tails - Bug #11839: After automatic upgrade from Tails 2.5 to 2.6, keyboard and mouse stop working Rejected 09/24/2016

Associated revisions

Revision 67bfe13b
Added by intrigeri 3 months ago

Allow the tails-install-iuk user to run "/usr/bin/nocache /bin/cp *" as root (refs: #8449).

Revision f6b28013
Added by intrigeri 3 months ago

Import tentative fix from iuk.git (refs: #8449, #11839).

Taken from branch bugfix/8449-iuk-install-robustness at
commit c92bfd7362d4beccb3feb00a735773ad361f513a.

Revision 40fca366
Added by anonym 3 months ago

Enable the bugfix-8449-iuk-install-robustness APT overlay.

Refs: #8449

Revision 1ab43b9e
Added by anonym 3 months ago

Revert "Import tentative fix from iuk.git (refs: #8449, #11839)."

This reverts commit f6b280136fe8fc3e3bb26979661f7d64daa008b7.

A package new tails-iuk package (2.8) with the patch applied has now
been uploaded.

Revision cc2e3b2c
Added by anonym 3 months ago

Merge branch 'bugfix/8449-iuk-install-robustness' into stable

Fix-committed: #8449

History

#1 Updated by intrigeri over 2 years ago

  • Status changed from Confirmed to In Progress
  • Assignee set to intrigeri
  • % Done changed from 0 to 10

#2 Updated by anonym over 2 years ago

  • Description updated (diff)
  • Assignee deleted (intrigeri)
  • % Done changed from 10 to 0

#3 Updated by intrigeri over 2 years ago

  • Assignee set to intrigeri
  • % Done changed from 0 to 10
  • in the iuk repo, bugfix/8449-iuk-install-robustness (to be merged into the master branch) implements that, and debian_bugfix/8449-iuk-install-robustness (to be merged into the debian branch) adds the dependency on nocache
  • in the main Git repo, bugfix/8449-iuk-install-robustness adds the necessary pinning so that nocache is installed from wheezy-backports

#4 Updated by intrigeri over 2 years ago

  • Assignee changed from intrigeri to anonym
  • % Done changed from 10 to 50
  • QA Check set to Ready for QA

Given this was tested already when preparing 1.2.2, I'm not in the mood to bother going through all the testing again, especially since the reviewer will have to do it anyway. So, please review/test/merge!

#5 Updated by intrigeri over 2 years ago

  • Assignee changed from anonym to intrigeri
  • Target version changed from Tails_1.2.3 to Tails_1.3
  • QA Check changed from Ready for QA to Dev Needed

According to the latest round of tests by anonym, tails-install-iuk fails when we add nocache in the pipeline: the correct .modules file is generated in /tmp but is not copied to the upgraded device, and the frontend reports that tails-install-iuk failed. Note that manually running tails-install-iuk works, both with and without nocache, so possibly something is wrong in the way the frontend is calling tails-install-iuk.

No idea why the exact opposite was reported a month ago, but looks like I should go back to the drawing board.

#6 Updated by intrigeri over 2 years ago

  • Target version changed from Tails_1.3 to Tails_1.3.2

Won't manage to rework this in time for 1.3, sorry.

#7 Updated by intrigeri about 2 years ago

  • Target version changed from Tails_1.3.2 to Hole in the Roof

#8 Updated by intrigeri about 2 years ago

  • Assignee changed from intrigeri to anonym
  • QA Check changed from Dev Needed to Info Needed

The documentation for File::Copy (whose copy method is used in upgrade_modules_file) warns about passing in files as handles instead of names to copy, and says it "may lead to loss of information on some operating systems". So, the copy($self->modules_file->stringify, $temp_fh) we do seems adventurous. Given the above test results (and especially the fact that anonym couldn't reproduce the bug when running tails-install-iuk directly), I guess it might also, indeed, be related to how IPC::Run::SafeHandles tweaks filehandles.

It could be worth trying the following patch:

--- a/lib/Tails/IUK/Install.pm
+++ b/lib/Tails/IUK/Install.pm
@@ -163,12 +163,12 @@ method upgrade_modules_file {
     my $append_str = join("\n", map { file($_)->basename } @installed_squashfs);
     if (length($append_str)) {
         my ($temp_fh, $temp_file) = tempfile;
-        copy($self->modules_file->stringify, $temp_fh)
+        close $temp_fh;
+        copy($self->modules_file->stringify, $temp_file)
             or $self->fatal(sprintf(
                 "Could not copy modules file ('%s') to temporary file ('%s')",
                 $self->modules_file, $temp_file,
             ));
-        close $temp_fh;

         $temp_fh = file($temp_file)->open('a');
         say $temp_fh $append_str;

If that's not enough, then I guell I'll try adding a calls to sync at carefully chosen locations in this piece of code, although I'm pretty sure we've tried that already. anonym, do you remember?

Also, anonym: do you still have the "hand-crafted and tiny (20 KB) 1.2.1_to_1.2.2 IUK" somewhere? It could be useful for me to try and reproduce and fix. Otherwise, well, I'll just re-do the work.

#9 Updated by intrigeri about 2 years ago

I forgot: it would also be useful to try commenting out use IPC::Run::SafeHandles; in Frontend.pm, in case it may be the cause of the problem.

#10 Updated by intrigeri about 2 years ago

  • Assignee changed from anonym to intrigeri
  • QA Check deleted (Info Needed)

intrigeri wrote:

Also, anonym: do you still have the "hand-crafted and tiny (20 KB) 1.2.1_to_1.2.2 IUK" somewhere?

He hasn't it anymore.

#11 Updated by intrigeri 4 months ago

  • Related to Bug #11839: After automatic upgrade from Tails 2.5 to 2.6, keyboard and mouse stop working added

#12 Updated by intrigeri 3 months ago

  • % Done changed from 50 to 10

intrigeri wrote:

According to the latest round of tests by anonym, tails-install-iuk fails when we add nocache in the pipeline: the correct .modules file is generated in /tmp but is not copied to the upgraded device, and the frontend reports that tails-install-iuk failed. Note that manually running tails-install-iuk works, both with and without nocache, so possibly something is wrong in the way the frontend is calling tails-install-iuk.

I suspect that we forgot to add nocache to the sudo configuration back when we tried that (none of the branches that introduced nocache usage have ever done this necessary adjustment), but run tails-install-iuk as root when we tried running it manually. I'll retry this before the other ideas I've mentioned on #8449#note-8.

#13 Updated by intrigeri 3 months ago

intrigeri wrote:

intrigeri wrote:

According to the latest round of tests by anonym, tails-install-iuk fails when we add nocache in the pipeline: the correct .modules file is generated in /tmp but is not copied to the upgraded device, and the frontend reports that tails-install-iuk failed. Note that manually running tails-install-iuk works, both with and without nocache, so possibly something is wrong in the way the frontend is calling tails-install-iuk.

I suspect that we forgot to add nocache to the sudo configuration back when we tried that (none of the branches that introduced nocache usage have ever done this necessary adjustment), but run tails-install-iuk as root when we tried running it manually. I'll retry this before the other ideas I've mentioned on #8449#note-8.

I've quickly tested this on a Tails 3.0~beta1 USB stick, and adding nocache doesn't break the upgrade, as long as sudo is configured properly. Will now test in 2.x, and if this works too, I'll propose a branch that uses nocache for 2.11 or 3.0 (doing it in 2.12 would be useless as there won't be incremental upgrades from 2.12 to 3.0).

#14 Updated by intrigeri 3 months ago

intrigeri wrote:

I've quickly tested this on a Tails 3.0~beta1 USB stick, and adding nocache doesn't break the upgrade, as long as sudo is configured properly. Will now test in 2.x,

It does! So it might be that we essentially had a solution 2y ago, but failed to implement it completely and rejected it for wrong reasons. Oops!

#15 Updated by intrigeri 3 months ago

  • Assignee changed from intrigeri to anonym
  • Target version changed from Hole in the Roof to Tails_2.11
  • % Done changed from 10 to 20
  • QA Check set to Ready for QA
  • Feature Branch set to bugfix/8449-iuk-install-robustness, iuk:bugfix/8449-iuk-install-robustness

These branches add nocache and a call to sync. Not sure it'll fix all problems in this area, but at least nocache did back when this ticket was created. Please review'n'merge:

  • The branch in iuk.git into master there.
  • The branch in tails.git into stable there. It has the changes from iuk.git are applied as a patch in tails.git. If you do a release of tails-iuk, you'll want to drop that patch (otherwise ISO will fail to build).

#16 Updated by anonym 3 months ago

  • Status changed from In Progress to Fix committed
  • Assignee deleted (anonym)
  • % Done changed from 20 to 100
  • QA Check changed from Ready for QA to Pass

Jenkins likes the branch, and code looks good => merged! I'm happy you thought of this fix after all these years... haha. cries a bit too

#17 Updated by segfault 3 months ago

  • Assignee set to intrigeri

Assigning to intrigeri because he said he would work on this during the contributors meeting.

#18 Updated by intrigeri 3 months ago

  • Assignee deleted (intrigeri)

#19 Updated by anonym 3 months ago

  • Status changed from Fix committed to Resolved

Also available in: Atom PDF