Guile-Parted crashes the installer on i686-linux

DoneSubmitted by Ludovic Courtès.
Details
3 participants
  • Jonathan Brielmaier
  • Ludovic Courtès
  • Mathieu Othacehe
Owner
unassigned
Severity
important
L
L
Ludovic Courtès wrote on 17 May 2019 21:46
(address . bug-Guix@gnu.org)
87h89syidd.fsf@gnu.org
Hello,
On i686-linux, the installer crashes when it reaches the partitioningpage with a null-pointer exception (or sometimes with a plain SIGSEGV),in the call to ‘partition-disk’ in ‘esp-partition?’. Screenshotsof two different crashes (null-pointer exception and SIGSEGV) attached.
The null-pointer exception happens when starting from a blank targetimage (fresh ‘qemu-img create’), while the other crash happens (IIRC)when the disk already has a partition table.
Mathieu, any ideas?
This is holding the 1.0.1 release (I actually have all the files readyfor upload after hours of builds :-/). We’ll have to decide whether tohold off until we have a fix, or whether to skip i686 altogether, or toship an install image where only the manual install may be used.
Thanks in advance!
Ludo’.
Attachment: i686-crash.png
Attachment: i686-crash-gmp.png
J
J
Jonathan Brielmaier wrote on 17 May 2019 22:03
(address . 35783@debbugs.gnu.org)
90464466-424e-9092-7cba-673ebacf2c24@web.de
On my laptop (Thinkpad X240) as well as in QEMU I'll get the first error(i686-crash.png). It makes no difference if I do separate home partitionor not. Encrypted partition or not still leads to this error...
L
L
Ludovic Courtès wrote on 17 May 2019 22:48
control message for bug #35783
(address . control@debbugs.gnu.org)
87ftpcyfhl.fsf@gnu.org
severity 35783 important
L
L
Ludovic Courtès wrote on 17 May 2019 23:22
Re: bug#35783: Guile-Parted crashes the installer on i686-linux
(address . 35783@debbugs.gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
877eaoydw8.fsf@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:
Toggle quote (5 lines)> On i686-linux, the installer crashes when it reaches the partitioning> page with a null-pointer exception (or sometimes with a plain SIGSEGV),> in the call to ‘partition-disk’ in ‘esp-partition?’. Screenshots> of two different crashes (null-pointer exception and SIGSEGV) attached.
Reverting 7d567af46b4e10ffafb1d0f76b524f5781460598 fixes the problem, soI guess this has to do with the fact that the previous partition objects(those listed in ‘initial-partitions’) are invalidated or something.
Now we need another way to addresshttps://issues.guix.gnu.org/issue/35731.
Ideas?
Thanks,Ludo’.
M
M
Mathieu Othacehe wrote on 18 May 2019 11:30
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 35783@debbugs.gnu.org)
87h89shzyq.fsf@gmail.com
Hey Ludo,
Toggle quote (4 lines)> Reverting 7d567af46b4e10ffafb1d0f76b524f5781460598 fixes the problem, so> I guess this has to do with the fact that the previous partition objects> (those listed in ‘initial-partitions’) are invalidated or something.
Yes I think that's what happened, even though I don't get why it didn'tcause an issue on x64.
Toggle quote (4 lines)>> Now we need another way to address> <https://issues.guix.gnu.org/issue/35731>.
I have a first draft, I'm about to test right now :)
Mathieu
From 3c6018a448f38e263aeb5a23fbc88d226a048d67 Mon Sep 17 00:00:00 2001From: Mathieu Othacehe <m.othacehe@gmail.com>Date: Sat, 18 May 2019 11:25:09 +0200Subject: [PATCH] draft: Fix esp user-partition creation.
--- gnu/installer/newt/partition.scm | 5 +---- gnu/installer/parted.scm | 12 ++++++++---- 2 files changed, 9 insertions(+), 8 deletions(-)
Toggle diff (41 lines)diff --git a/gnu/installer/newt/partition.scm b/gnu/installer/newt/partition.scmindex f8e318fa0d..cd9d46316a 100644--- a/gnu/installer/newt/partition.scm+++ b/gnu/installer/newt/partition.scm@@ -752,10 +752,7 @@ by pressing the Exit button.~%~%"))) (disk-commit disk) disk))) (scheme (symbol-append method '- (run-scheme-page)))- (user-partitions (append- (auto-partition! disk #:scheme scheme)- (create-special-user-partitions- (disk-partitions disk)))))+ (user-partitions (auto-partition! disk #:scheme scheme))) (run-disk-page (list disk) user-partitions #:guided? #t))) ((eq? method 'manual)diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scmindex 4ccc0b1f51..ac9098cbde 100644--- a/gnu/installer/parted.scm+++ b/gnu/installer/parted.scm@@ -1001,10 +1001,14 @@ swap partition, a root partition and a home partition." (mount-point "/home"))))))) (new-partitions* (force-user-partitions-formatting new-partitions)))- (create-adjacent-partitions! disk- new-partitions*- #:last-partition-end- (or end-esp-partition 0)))))+ (append+ (if esp-partition+ (partition->user-partition esp-partition)+ '())+ (create-adjacent-partitions! disk+ new-partitions*+ #:last-partition-end+ (or end-esp-partition 0)))))) ;;-- 2.17.1
M
M
Mathieu Othacehe wrote on 18 May 2019 12:25
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 35783@debbugs.gnu.org)
87ftpchxel.fsf@gmail.com
With this almost indentical patch, disk partitioning seems fine with andwithout existing esp partition on x64. I'll try to run more tests.
Mathieu
From 1e0734c4829725cdee6cab3cb05332ffd2a36a57 Mon Sep 17 00:00:00 2001From: Mathieu Othacehe <m.othacehe@gmail.com>Date: Sat, 18 May 2019 11:25:09 +0200Subject: [PATCH] draft: Fix esp user-partition creation.
--- gnu/installer/newt/partition.scm | 5 +---- gnu/installer/parted.scm | 12 ++++++++---- 2 files changed, 9 insertions(+), 8 deletions(-)
Toggle diff (41 lines)diff --git a/gnu/installer/newt/partition.scm b/gnu/installer/newt/partition.scmindex f8e318fa0d..cd9d46316a 100644--- a/gnu/installer/newt/partition.scm+++ b/gnu/installer/newt/partition.scm@@ -752,10 +752,7 @@ by pressing the Exit button.~%~%"))) (disk-commit disk) disk))) (scheme (symbol-append method '- (run-scheme-page)))- (user-partitions (append- (auto-partition! disk #:scheme scheme)- (create-special-user-partitions- (disk-partitions disk)))))+ (user-partitions (auto-partition! disk #:scheme scheme))) (run-disk-page (list disk) user-partitions #:guided? #t))) ((eq? method 'manual)diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scmindex 4ccc0b1f51..196fa99cf4 100644--- a/gnu/installer/parted.scm+++ b/gnu/installer/parted.scm@@ -1001,10 +1001,14 @@ swap partition, a root partition and a home partition." (mount-point "/home"))))))) (new-partitions* (force-user-partitions-formatting new-partitions)))- (create-adjacent-partitions! disk- new-partitions*- #:last-partition-end- (or end-esp-partition 0)))))+ (append+ (if esp-partition+ (list (partition->user-partition esp-partition))+ '())+ (create-adjacent-partitions! disk+ new-partitions*+ #:last-partition-end+ (or end-esp-partition 0)))))) ;;-- 2.17.1
L
L
Ludovic Courtès wrote on 18 May 2019 13:50
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 35783@debbugs.gnu.org)
87imu8vv5i.fsf@gnu.org
Hi Mathieu,
Mathieu Othacehe <m.othacehe@gmail.com> skribis:
Toggle quote (3 lines)> With this almost indentical patch, disk partitioning seems fine with and> without existing esp partition on x64. I'll try to run more tests.
I was fiddling with this and had arrived to a similar patch, we’re inperfect symbiosis. :-)
I’ve done some testing both in an EFI and a non-EFI setup with QEMU, andit seems to work well; I’ll do some more testing as well.
Toggle quote (5 lines)>>From 1e0734c4829725cdee6cab3cb05332ffd2a36a57 Mon Sep 17 00:00:00 2001> From: Mathieu Othacehe <m.othacehe@gmail.com>> Date: Sat, 18 May 2019 11:25:09 +0200> Subject: [PATCH] draft: Fix esp user-partition creation.
[...]
Toggle quote (19 lines)> --- a/gnu/installer/parted.scm> +++ b/gnu/installer/parted.scm> @@ -1001,10 +1001,14 @@ swap partition, a root partition and a home partition."> (mount-point "/home")))))))> (new-partitions* (force-user-partitions-formatting> new-partitions)))> - (create-adjacent-partitions! disk> - new-partitions*> - #:last-partition-end> - (or end-esp-partition 0)))))> + (append> + (if esp-partition> + (list (partition->user-partition esp-partition))> + '())> + (create-adjacent-partitions! disk> + new-partitions*> + #:last-partition-end> + (or end-esp-partition 0))))))
Perhaps add something like this to the docstring of ‘auto-partition!’:
Return the complete list of partitions on DISK, including the ESP when it exists.
Longer-term it would be good to audit Guile-Parted: it probablyshouldn’t be possible for Guile-Parted to refer to “defunct” Partedobjects.
Thank you for the quick response!
Ludo’.
L
L
Ludovic Courtès wrote on 18 May 2019 15:44
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 35783@debbugs.gnu.org)
87lfz3vpw4.fsf@gnu.org
It seems to be working well for me, including non-EFI i686.
I use the attached script to test an full install from the ISO, with orwithout UEFI (though you cannot boot the install UEFI system because theEFI variables set by GRUB are not saved.)
Ludo’.
#!/bin/shset -eset -xISO="$(./pre-inst-env guix system disk-image --file-system-type=iso9660 gnu/system/install.scm)"qemu-img create -f qcow2 /tmp/t.img 10G
#cp "$(guix build ovmf)/share/firmware/ovmf_x64.bin" /tmp#chmod +w /tmp/ovmf_x64.binEFI_OPTS="-bios $(guix build ovmf)/share/firmware/ovmf_x64.bin"
exec qemu-system-x86_64 -enable-kvm -hda /tmp/t.img -cdrom "$ISO" -m 1024 -boot d -net user -net nic,model=virtio -no-reboot $EFI_OPTS
L
L
Ludovic Courtès wrote on 19 May 2019 12:09
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 35783-done@debbugs.gnu.org)
87zhniu55q.fsf@gnu.org
Hi,
I went ahead and pushed the patch asd68de958b60426798ed62797ff7c96c327a672ac.
I’ll build the release from that commit.
Thanks,Ludo’.
Closed
M
M
Mathieu Othacehe wrote on 19 May 2019 16:24
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 35783@debbugs.gnu.org)
87v9y6cyjp.fsf@gmail.com
Hey Ludo,
Toggle quote (4 lines)> Longer-term it would be good to audit Guile-Parted: it probably> shouldn’t be possible for Guile-Parted to refer to “defunct” Parted> objects.
Yup, with hindsight I realize that keeping Guile-Parted so low-level wasa mistake. With a few more abstractions (gnu installer parted) could beless complicated.
Anyway, thanks for pushing this patch.
Mathieu
L
L
Ludovic Courtès wrote on 20 May 2019 10:17
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 35783@debbugs.gnu.org)
87tvdpwndy.fsf@gnu.org
Hi Mathieu,
Mathieu Othacehe <m.othacehe@gmail.com> skribis:
Toggle quote (8 lines)>> Longer-term it would be good to audit Guile-Parted: it probably>> shouldn’t be possible for Guile-Parted to refer to “defunct” Parted>> objects.>> Yup, with hindsight I realize that keeping Guile-Parted so low-level was> a mistake. With a few more abstractions (gnu installer parted) could be> less complicated.
I don’t know; as a rule of thumb, I think it’s good to make bindings adirect mapping to the underlying library, and to build abstractions ontop of that.
That said, my point was more that it shouldn’t be possible to get anull-pointer exception or a SIGSEGV when using Guile-Parted, even if youmake a mistake. In this case, it seems that the underlying C object hadbeen reclaimed somehow; the bindings should protect against that.
Thoughts?
Ludo’.
?
Your comment

This issue is archived.

To comment on this conversation send email to 35783@debbugs.gnu.org