Bug #8449

Tails Upgrader fails to install some IUKs

Added by anonym 2014-12-16 14:52:30 . Updated 2017-03-09 13:56:49 .

Status:
Resolved
Priority:
Elevated
Assignee:
Category:
Target version:
Start date:
2014-12-16
Due date:
% Done:

100%

Feature Branch:
bugfix/8449-iuk-install-robustness, iuk:bugfix/8449-iuk-install-robustness
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Upgrader
Deliverable for:

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);

Subtasks


Related issues

Related to Tails - Bug #11839: After automatic upgrade from Tails 2.5 to 2.6, keyboard and mouse stop working Rejected 2016-09-24
Related to Tails - Bug #14754: Partially applied incremental upgrades cause all kinds of trouble Confirmed 2019-03-21

History

#1 Updated by intrigeri 2014-12-16 17:37:10

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

#2 Updated by anonym 2014-12-16 18:02:48

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

#3 Updated by intrigeri 2014-12-16 18:31:15

  • 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 2015-01-06 11:00:05

  • 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 2015-01-13 18:32:22

  • 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 2015-02-10 18:17:24

  • 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 2015-03-29 15:43:56

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

#8 Updated by intrigeri 2015-04-09 11:07:59

  • 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 2015-04-09 11:10:39

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 2015-04-25 03:25:16

  • 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 2017-02-01 10:32:31

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

#12 Updated by intrigeri 2017-03-03 12:14:26

  • % 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 Bug #8449#note-8.

#13 Updated by intrigeri 2017-03-03 12:47:39

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 Bug #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 2017-03-03 15:23:34

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 2017-03-03 17:09:05

  • 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 2017-03-05 13:43:42

  • 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 2017-03-08 14:02:13

  • Assignee set to intrigeri

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

#18 Updated by intrigeri 2017-03-08 14:21:27

  • Assignee deleted (intrigeri)

#19 Updated by anonym 2017-03-09 13:56:49

  • Status changed from Fix committed to Resolved

#20 Updated by intrigeri 2018-11-19 17:04:24

  • related to Bug #14754: Partially applied incremental upgrades cause all kinds of trouble added