bug#58733: installer: coredump generation

bug#58733: installer: coredump generation

[Mathieu Othacehe]

Hello,

This installer sometimes sadly segfaults, most of the time in
libparted. To be able to catch this coredump[1], I ran those commands:

--8<---------------cut here---------------start------------->8---
echo /tmp/core > /proc/sys/kernel/core_pattern
prlimit --core=unlimited --pid=1234
--8<---------------cut here---------------end--------------->8---

The coredump I obtained did not seem to be exploitable, despite the fact
that it weights 155MB:

--8<---------------cut here---------------start------------->8---
mathieu@meije ~/guix [env]$ gdb /gnu/store/1jgcbdzx2ss6xv59w55g3kr3x4935dfb-guile-3.0.8/bin/guile core
...
BFD: warning: /home/mathieu/guix/core has a segment extending past end of file
warning: core file may not match specified executable file.
...
Failed to read a valid object file image from memory.
Core was generated by `/gnu/store/1jgcbdzx2ss6xv59w55g3kr3x4935dfb-guile-3.0.8/bin/guile --no-auto-com'.
--8<---------------cut here---------------end--------------->8---

So I decided to adopt a new strategy and ran:

--8<---------------cut here---------------start------------->8---
$ gdb
$ attach 1234
...
$ gcore
--8<---------------cut here---------------end--------------->8---

to get a viable core dump, and those commands to exploit it (thanks
Josselin!):

--8<---------------cut here---------------start------------->8---
(gdb) info sharedlibrary 
From                To                  Syms Read   Shared Object Library
...
0x00007f892c59c850  0x00007f892c5d3d0b  Yes (*)     /gnu/store/qz7qqrhgcs3ixv8f1k30gwiqr1prm7qs-parted-3.5/lib/libparted.so
(gdb) add-symbol-file  /gnu/store/b0ymz7vjfkcvhbci49q5yk1fi0l9lq49-parted-3.5/lib/libparted.so  0x00007f892c59c850 
add symbol table from file "/gnu/store/b0ymz7vjfkcvhbci49q5yk1fi0l9lq49-parted-3.5/lib/libparted.so" at
	.text_addr = 0x7f892c59c850
(y or n) y
Reading symbols from /gnu/store/b0ymz7vjfkcvhbci49q5yk1fi0l9lq49-parted-3.5/lib/libparted.so...
(gdb) bt
#0  linux_destroy (dev=0x1dc89e0) at arch/linux.c:1615
#1  0x00007f8941aecd37 in ?? () from /gnu/store/1jgcbdzx2ss6xv59w55g3kr3x4935dfb-guile-3.0.8/lib/libguile-3.0.so.1
...
--8<---------------cut here---------------end--------------->8---

I think that it would be great if we could enable coredump generation
from the installer. This way, when a crash occurs and the installer
restarts, it would notice that there is an existing coredump in say
/tmp/coredump_xxx and propose to upload it using the existing dump
mechanism.

Thanks,

Mathieu

[1]: https://issues.guix.gnu.org/58732

bug#58733: installer: coredump generation

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 596 bytes --]


Hello,

> Failed to read a valid object file image from memory.
> Core was generated by `/gnu/store/1jgcbdzx2ss6xv59w55g3kr3x4935dfb-guile-3.0.8/bin/guile --no-auto-com'.

This is reported as: https://issues.guix.gnu.org/58923

> I think that it would be great if we could enable coredump generation
> from the installer. This way, when a crash occurs and the installer
> restarts, it would notice that there is an existing coredump in say
> /tmp/coredump_xxx and propose to upload it using the existing dump
> mechanism.

Here is an attached patch implementing the proposed mechanism.

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-installer-Add-core-dump-support.patch --]
[-- Type: text/x-patch, Size: 3710 bytes --]

From f4d2a1bb4df2f65b650be704bffb7ea469ae0232 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Mon, 31 Oct 2022 13:03:46 +0100
Subject: [PATCH 1/1] installer: Add core dump support.

Fixes: <https://issues.guix.gnu.org/58733>

* gnu/installer.scm (installer-program): Enable core dump generation.
* gnu/installer/dump.scm (%core-dump): New variable.
(prepare-dump): Copy the core dump file.
* gnu/installer/newt/welcome.scm (run-welcome-page): Propose to report an
installation that previously generated a core dump.
---
 gnu/installer.scm              |  6 ++++++
 gnu/installer/dump.scm         | 10 +++++++++-
 gnu/installer/newt/welcome.scm | 15 +++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/gnu/installer.scm b/gnu/installer.scm
index 8a6e604fa5..52c595b5b7 100644
--- a/gnu/installer.scm
+++ b/gnu/installer.scm
@@ -389,6 +389,12 @@ (define installer-builder
                          (ice-9 match)
                          (ice-9 textual-ports))
 
+            ;; Enable core dump generation.
+            (setrlimit 'core #f #f)
+            (call-with-output-file "/proc/sys/kernel/core_pattern"
+              (lambda (port)
+                (format port %core-dump)))
+
             ;; Initialize gettext support so that installers can use
             ;; (guix i18n) module.
             #$init-gettext
diff --git a/gnu/installer/dump.scm b/gnu/installer/dump.scm
index daa02f205a..f91cbae021 100644
--- a/gnu/installer/dump.scm
+++ b/gnu/installer/dump.scm
@@ -28,13 +28,17 @@ (define-module (gnu installer dump)
   #:use-module (web http)
   #:use-module (web response)
   #:use-module (webutils multipart)
-  #:export (prepare-dump
+  #:export (%core-dump
+            prepare-dump
             make-dump
             send-dump-report))
 
 ;; The installer crash dump type.
 (define %dump-type "installer-dump")
 
+;; The core dump file.
+(define %core-dump "/tmp/installer-core-dump")
+
 (define (result->list result)
   "Return the alist for the given RESULT."
   (hash-map->list (lambda (k v)
@@ -66,6 +70,10 @@ (define dump-dir
     ;; syslog
     (copy-file "/var/log/messages" "syslog")
 
+    ;; core dump
+    (when (file-exists? %core-dump)
+      (copy-file %core-dump "core-dump"))
+
     ;; dmesg
     (let ((pipe (open-pipe* OPEN_READ "dmesg")))
       (call-with-output-file "dmesg"
diff --git a/gnu/installer/newt/welcome.scm b/gnu/installer/newt/welcome.scm
index 0bca44d1b2..5d47591d67 100644
--- a/gnu/installer/newt/welcome.scm
+++ b/gnu/installer/newt/welcome.scm
@@ -20,6 +20,7 @@
 (define-module (gnu installer newt welcome)
   #:use-module ((gnu build linux-modules)
                 #:select (modules-loaded))
+  #:use-module (gnu installer dump)
   #:use-module (gnu installer steps)
   #:use-module (gnu installer utils)
   #:use-module (gnu installer newt page)
@@ -132,6 +133,20 @@ (define (run-welcome-page logo)
 the system does not boot, perhaps you will need to add nomodeset to the
 kernel arguments and need to configure the uvesafb kernel module.")
                       (G_ "Pre-install warning")))
+    (when (file-exists? %core-dump)
+      (match
+          (choice-window
+           (G_ "Previous installation failed")
+           (G_ "Continue")
+           (G_ "Report the failure")
+           (G_ "It seems that the previous installation exited unexpectedly \
+and generated a core dump.  Do you want to continue or to report the failure \
+first?"))
+        (1 #t)
+        (2 (raise
+            (condition
+             (&message
+              (message "User abort.")))))))
     (run-menu-page
      (G_ "GNU Guix install")
      (G_ "Welcome to GNU Guix system installer!
-- 
2.38.0

bug#58733: installer: coredump generation

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 222 bytes --]


> Here is an attached patch implementing the proposed mechanism.

I also prepared the attached patch as a follow-up. The idea is to hide
the backtrace page when the user chooses to "Report the failure".

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-installer-Skip-the-backtrace-page-on-user-abort.patch --]
[-- Type: text/x-patch, Size: 4273 bytes --]

From d3f2ce83152a8ea453b407652dbee7b86a64816b Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Mon, 31 Oct 2022 16:43:09 +0100
Subject: [PATCH 1/1] installer: Skip the backtrace page on user abort.

When the user aborts the installation because a core dump is discovered or the
installation command failed, displaying the abort backtrace doesn't make much
sense. Hide it when the abort condition is &user-abort-error and skip directly
to the dump page.

* gnu/installer/steps.scm (&user-abort-error): New variable.
(user-abort-error?): New procedure.
* gnu/installer/newt/final.scm (run-install-failed-page): Raise a
user-abort-error.
* gnu/installer/newt/welcome.scm (run-welcome-page): Ditto.
* gnu/installer.scm (installer-program): Hide the backtrace page and directly
propose to dump the report when the a &user-abort-error is raised.
---
 gnu/installer.scm              | 18 ++++++++++++++----
 gnu/installer/newt/final.scm   |  5 ++---
 gnu/installer/newt/welcome.scm |  3 +--
 gnu/installer/steps.scm        |  8 +++++++-
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/gnu/installer.scm b/gnu/installer.scm
index 52c595b5b7..5cd1af8edf 100644
--- a/gnu/installer.scm
+++ b/gnu/installer.scm
@@ -453,11 +453,21 @@ (define results
                                           key args)
                       (define dump-dir
                         (prepare-dump key args #:result %current-result))
+
+                      (define user-abort?
+                        (match args
+                          (((? user-abort-error? obj)) #t)
+                          (_ #f)))
+
                       (define action
-                        ((installer-exit-error current-installer)
-                         (get-string-all
-                          (open-input-file
-                           (string-append dump-dir "/installer-backtrace")))))
+                        (if user-abort?
+                            'dump
+                            ((installer-exit-error current-installer)
+                             (get-string-all
+                              (open-input-file
+                               (string-append dump-dir
+                                              "/installer-backtrace"))))))
+
                       (match action
                         ('dump
                          (let* ((dump-files
diff --git a/gnu/installer/newt/final.scm b/gnu/installer/newt/final.scm
index 6e55be5067..9f950a0551 100644
--- a/gnu/installer/newt/final.scm
+++ b/gnu/installer/newt/final.scm
@@ -92,9 +92,8 @@ (define (run-install-failed-page)
         ;; Keep going, the installer will be restarted later on.
         #t)
        (3 (raise
-           (condition
-            (&message
-             (message "User abort.")))))))
+            (condition
+             (&user-abort-error))))))
     (_
      (send-to-clients '(installation-failure))
      #t)))
diff --git a/gnu/installer/newt/welcome.scm b/gnu/installer/newt/welcome.scm
index 5d47591d67..326996b005 100644
--- a/gnu/installer/newt/welcome.scm
+++ b/gnu/installer/newt/welcome.scm
@@ -145,8 +145,7 @@ (define (run-welcome-page logo)
         (1 #t)
         (2 (raise
             (condition
-             (&message
-              (message "User abort.")))))))
+             (&user-abort-error))))))
     (run-menu-page
      (G_ "GNU Guix install")
      (G_ "Welcome to GNU Guix system installer!
diff --git a/gnu/installer/steps.scm b/gnu/installer/steps.scm
index 8b25ae97c8..0c505e40e4 100644
--- a/gnu/installer/steps.scm
+++ b/gnu/installer/steps.scm
@@ -28,7 +28,10 @@ (define-module (gnu installer steps)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
   #:use-module (rnrs io ports)
-  #:export (<installer-step>
+  #:export (&user-abort-error
+            user-abort-error?
+
+            <installer-step>
             installer-step
             make-installer-step
             installer-step?
@@ -50,6 +53,9 @@ (define-module (gnu installer steps)
 
             %current-result))
 
+(define-condition-type &user-abort-error &error
+  user-abort-error?)
+
 ;; Hash table storing the step results. Use it only for logging and debug
 ;; purposes.
 (define %current-result (make-hash-table))
-- 
2.38.0

bug#58733: installer: coredump generation

[Ludovic Courtès]

Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

>>From f4d2a1bb4df2f65b650be704bffb7ea469ae0232 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <othacehe@gnu.org>
> Date: Mon, 31 Oct 2022 13:03:46 +0100
> Subject: [PATCH 1/1] installer: Add core dump support.
>
> Fixes: <https://issues.guix.gnu.org/58733>
>
> * gnu/installer.scm (installer-program): Enable core dump generation.
> * gnu/installer/dump.scm (%core-dump): New variable.
> (prepare-dump): Copy the core dump file.
> * gnu/installer/newt/welcome.scm (run-welcome-page): Propose to report an
> installation that previously generated a core dump.

[...]

>>From d3f2ce83152a8ea453b407652dbee7b86a64816b Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <othacehe@gnu.org>
> Date: Mon, 31 Oct 2022 16:43:09 +0100
> Subject: [PATCH 1/1] installer: Skip the backtrace page on user abort.
>
> When the user aborts the installation because a core dump is discovered or the
> installation command failed, displaying the abort backtrace doesn't make much
> sense. Hide it when the abort condition is &user-abort-error and skip directly
> to the dump page.
>
> * gnu/installer/steps.scm (&user-abort-error): New variable.
> (user-abort-error?): New procedure.
> * gnu/installer/newt/final.scm (run-install-failed-page): Raise a
> user-abort-error.
> * gnu/installer/newt/welcome.scm (run-welcome-page): Ditto.
> * gnu/installer.scm (installer-program): Hide the backtrace page and directly
> propose to dump the report when the a &user-abort-error is raised.

Both look reasonable to me, thanks!

Now, we should probably focus on Guile-Parted…

Thanks,
Ludo’.

bug#58733: installer: coredump generation

[Mathieu Othacehe]

Hey,

> Both look reasonable to me, thanks!

Thanks for reviewing :)

> Now, we should probably focus on Guile-Parted…

Yes, I saw you sent a few pointers, that will be my next focus!

Mathieu