[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
?