Re: install/53220: sysinst dumps core with extended partitioning.

[Robert Elz <kre%munnari.OZ.AU@localhost>]

The following reply was made to PR install/53220; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: install/53220: sysinst dumps core with extended partitioning.
Date: Wed, 02 May 2018 04:03:30 +0700

 Ignore the vnd red herring, that turned out to be relevant to nothing (though
 the change from vnconfig to vndconfig was certainly the right thing to do).
 
 The actual cause for the core dump is the pm_lvm_find() function,
 it assumes that lvms has been allocated to point to a suitable block
 of storage, but when we !have_lvm:
 
                 if (!have_lvm)
                         remove_lvm_options();
                 else if (!(lvms = calloc(MAX_LVM_VG, sizeof(*lvms))))
                         have_lvm = 0;
 
 the menu entries related to lvm get dleeted, so there's no way to refer to
 them, but lvms is just left uninitialized.
 
 Later partman() does
 
                 menu_num_entries = pm_upddevlist(&(menudesc){.opts = menu_entrie
 
 and pm_upddevlist() does
 
         changed = 0;
         /* Mark all devices as not found */
         SLIST_FOREACH(pm_i, &pm_head, l)
                 if (pm_i->found > 0)
                         pm_i->found = 0;
         /* Detect all present devices */
         (void)find_disks("partman");
         pm_lvm_find();
         pm_clean();
 
 pm_lvm_find() does
 
       for (i = 0; i < MAX_LVM_VG; i++) { 
                 if (! lvms[i].blocked)  
                         continue; 
 
 (and much more) using that uninit'd lvms.
 
 This doesn't cause a problem with a fully populated root filesys
 sitting under sysinst, as in that case have_lvm == 1, lvms gets calloc()'d
 and all is fine.
 
 To test this, I actually made pm_lvm_find() simply return 0 if (!have_lvm )
 and with that change, the partman menu appeared (and at that point I
 stopped testing...   I did not have things set up in a way that the sets were
 available, and playing with the partition editor would have meant changing
 the setup I have used over and over again while hunting this down (just
 in case there turns out to be more to hunt it is nice to keep as is)
 
 However I think a better fix is:
 
 Index: partman.c
 ===================================================================
 RCS file: /cvsroot/src/usr.sbin/sysinst/partman.c,v
 retrieving revision 1.19
 diff -u -r1.19 partman.c
 --- partman.c	1 May 2018 08:27:39 -0000	1.19
 +++ partman.c	1 May 2018 20:54:13 -0000
 @@ -2609,7 +2609,8 @@
  			pm_i->found = 0;
  	/* Detect all present devices */
  	(void)find_disks("partman");
 -	pm_lvm_find();
 +	if (have_lvm)
 +		pm_lvm_find();
  	pm_clean();
  
  	if (m == NULL || arg == NULL)
 
 
 In addition it might also be a good idea to turn pm_lvm_find() into a
 function returning void, rather than its current int return, which is
 always 0, which is always (the fragment above is the only use)
 ignored - though that's just futzing around.
 
 kre