install/43295: sysinst dumps core before editing MBR on arc and hpcmips

[tsutsui%ceres.dti.ne.jp@localhost]

>Number:         43295
>Category:       install
>Synopsis:       sysinst dumps core before editing MBR on mips
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    install-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed May 12 13:25:00 +0000 2010
>Originator:     Izumi Tsutsui
>Release:        NetBSD 5.1_RC1
>Organization:
>Environment:
System: NetBSD 5.1_RC1 (RAMDISK) #0: Sat Apr 24 22:53:26 UTC 2010 \
builds%b7.netbsd.org@localhost:/home/builds/ab/netbsd-5-1-RC1-hpcmips/201004250032Z-obj/home/builds/ab/netbsd-5-1-RC1/src/sys/arch/hpcmips/compile/RAMDISK
Architecture: mipsel
Machine: hpcmips, also happens on arc both gxemul and Express5800/230

>Description:
sysinst dumps core during sysinst just before editing MBR partition
after selecting "a: Edit the MBR partition table":

---

 The Current MBR partition table is shown below.
 Flgs: a => Active partition, I => Install here.
 Select the partition you wish to chage:

 [1]   Segmentation fault (core dumped) sysinst
#

---

where "Total disk size 512 MB." should be shown.

>How-To-Repeat:

Try install NetBSD/hpcmips 5.1_RC1 on hpcmips.
You can try it using gxemul:

# pkg_add gxemul

% dd if=/dev/zero of=gxemul.img bs=1m count=512
% ftp ftp.NetBSD.org:/pub/NetBSD/NetBSD-5.1_RC1/hpcmips/installation/netbsd.gz
% gxemul -Xe mobilepro780 -d gxemul.img netbsd.gz

> a: Installation messages in English
> a: Install NetBSD to hard disk
> b: Yes
  Hit enter to continue
> b: Minimal installation
  sectors [63]: (enter)
  heads [255]: (enter)
> a: Edit the MBR partition table

Then you will get core.

>Fix:
The message is printed in src/distrib/utils/sysinst/mbr.c:set_mbr_header():

---
static void
set_mbr_header(menudesc *m, void *arg)
{
        mbr_info_t *mbri = arg;
        static menu_ent *opts;
        static int num_opts;
        mbr_info_t *ext;
        menu_ent *op;
        int i;
        int left;

        msg_display(MSG_editparttable);

        msg_table_add(MSG_part_header, dlsize/sizemult, multname, multname,
            multname, multname);
---

A variable "dlsize" has been changed from int to daddr_t in pullup-5 #758:
http://mail-index.NetBSD.org/source-changes/2009/05/18/msg221460.html
> cvs rdiff -u -r1.136 -r1.136.2.1 src/distrib/utils/sysinst/defs.h

> -int dlcyl, dlhead, dlsec, dlsize, dlcylsize;
> +int dlcyl, dlhead, dlsec, dlcylsize;
> +daddr_t dlsize;

but format characters in msg.mbr.* was not changed from %d:

---
/* the %s's will expand into three character strings */
message part_header
{   Total disk size %d %s.

---

Probably that's the problem.

In the perfect world we should fix msgc(1) not to dump core even if
wring type variables are passed, but in this case (dlsize/sizemult)
won't be >2G since sysinst would choose proper sizemult to print
humanized numbers, so the following simple fix will work.


Index: mbr.c
===================================================================
RCS file: /cvsroot/src/distrib/utils/sysinst/mbr.c,v
retrieving revision 1.79.14.1
diff -u -r1.79.14.1 mbr.c
--- mbr.c       18 May 2009 19:35:13 -0000      1.79.14.1
+++ mbr.c       12 May 2010 12:40:20 -0000
@@ -1152,11 +1152,12 @@
        mbr_info_t *ext;
        menu_ent *op;
        int i;
-       int left;
+       int unitsize, left;
 
        msg_display(MSG_editparttable);
 
-       msg_table_add(MSG_part_header, dlsize/sizemult, multname, multname,
+       unitsize = dlsize / sizemult;
+       msg_table_add(MSG_part_header, unitsize, multname, multname,
            multname, multname);
 
        if (num_opts == 0) {

---
Izumi Tsutsui