[PATCH] installer: Fix segfault on double logical partition removal.

  • Done
  • quality assurance status badge
Details
2 participants
  • Josselin Poiret
  • Mathieu Othacehe
Owner
unassigned
Submitted by
Josselin Poiret
Severity
normal
J
J
Josselin Poiret wrote on 31 Aug 2022 23:22
(address . guix-patches@gnu.org)(name . Josselin Poiret)(address . dev@jpoiret.xyz)
eebd90a6029f26e70226f4a66e98cd5349c4565f.1661980927.git.dev@jpoiret.xyz
* gnu/installer/parted.scm (auto-partition!): Avoid removing logical
partitions twice.
---
gnu/installer/parted.scm | 6 ++++++
1 file changed, 6 insertions(+)

Toggle diff (28 lines)
diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scm
index 641a1f45e8..84fdbe24fb 100644
--- a/gnu/installer/parted.scm
+++ b/gnu/installer/parted.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2018, 2019 Mathieu Othacehe <m.othacehe@gmail.com>
;;; Copyright © 2019, 2020, 2022 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2020 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2022 Josselin Poiret <dev@jpoiret.xyz>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -983,6 +984,11 @@ (define* (auto-partition! disk
(for-each
(lambda (partition)
(and (data-partition? partition)
+ ;; Do not remove logical partitions ourselves, since
+ ;; disk-remove-partition* will remove all the logical partitions
+ ;; residing on an extended partition, which would lead to a
+ ;; double-remove and ensuing SEGFAULT.
+ (not (logical-partition? partition))
(disk-remove-partition* disk partition)))
non-boot-partitions)

base-commit: 47c11772dfe840a536ed7ec438fe832878f51054
--
2.37.2
M
M
Mathieu Othacehe wrote on 1 Sep 2022 18:48
(name . Josselin Poiret)(address . dev@jpoiret.xyz)(address . 57513-done@debbugs.gnu.org)
87y1v32g7g.fsf@gnu.org
Hey,

Toggle quote (3 lines)
> * gnu/installer/parted.scm (auto-partition!): Avoid removing logical
> partitions twice.

I was able to reproduce the issue by creating an extended partition
containing a single logical partition using the manual partitioning tool
then, the automatic one right after.

It resulted in a segfault, which is fixed by your patch, that's a very
nice catch!

Pushed as 4989f6acff3b3fcfbd9dde3e3c2767bd2cd6d49e.

Thanks,

Mathieu
Closed
J
J
Josselin Poiret wrote on 1 Sep 2022 21:16
(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 57513-done@debbugs.gnu.org)
87tu5qzz0t.fsf@jpoiret.xyz
Hey Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

Toggle quote (7 lines)
> I was able to reproduce the issue by creating an extended partition
> containing a single logical partition using the manual partitioning tool
> then, the automatic one right after.
>
> It resulted in a segfault, which is fixed by your patch, that's a very
> nice catch!

I have to thank KE0VVT on IRC, who provided a core dump file! This was
surprisingly easier to debug than I thought, for those interested, I
built the installer using the same Guix commit, and loaded the guile
core dump file in gdb. I then used `guix build parted
--with-debug-info=parted` and loaded the resulting libparted.so library
using `info sections` to find out where the .text of libparted.so was
loaded in the core file, and `add-symbol-file
/gnu/store/path/to/libparted.so 0xaddress` to load the symbols. That
way, I could see that ped_disk_remove_partition was invoked for a disk
that had an empty partition list, hence leading me to this double remove
problem!

Toggle quote (6 lines)
> Pushed as 4989f6acff3b3fcfbd9dde3e3c2767bd2cd6d49e.
>
> Thanks,
>
> Mathieu

Thank you for reviewing this so fast!

Best,
--
Josselin Poiret
Closed
M
M
Mathieu Othacehe wrote on 2 Sep 2022 09:50
(name . Josselin Poiret)(address . dev@jpoiret.xyz)(address . 57513-done@debbugs.gnu.org)
871qsub4f8.fsf_-_@gnu.org
Hey,

Toggle quote (12 lines)
> I have to thank KE0VVT on IRC, who provided a core dump file! This was
> surprisingly easier to debug than I thought, for those interested, I
> built the installer using the same Guix commit, and loaded the guile
> core dump file in gdb. I then used `guix build parted
> --with-debug-info=parted` and loaded the resulting libparted.so library
> using `info sections` to find out where the .text of libparted.so was
> loaded in the core file, and `add-symbol-file
> /gnu/store/path/to/libparted.so 0xaddress` to load the symbols. That
> way, I could see that ped_disk_remove_partition was invoked for a disk
> that had an empty partition list, hence leading me to this double remove
> problem!

I remember resorting to way less convenient solutions in the past to
achieve something similar. Feel free to add this little memo to the
documentation or as a code comment if you have the opportunity :).

Thanks again,

Mathieu
Closed
?
Your comment

This issue is archived.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 57513
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch