| Message ID | eebd90a6029f26e70226f4a66e98cd5349c4565f.1661980927.git.dev@jpoiret.xyz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#57513] installer: Fix segfault on double logical partition removal. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git-branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
Hey, > * 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
Hey Mathieu, Mathieu Othacehe <othacehe@gnu.org> writes: > 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! > Pushed as 4989f6acff3b3fcfbd9dde3e3c2767bd2cd6d49e. > > Thanks, > > Mathieu Thank you for reviewing this so fast! Best,
Hey, > 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
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)