Re: bin/37656: fdisk segfault on invalid MBR entries on non-x86 geometry detection

[jnemeth%victoria.tc.ca@localhost (John Nemeth)]

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

From: jnemeth%victoria.tc.ca@localhost (John Nemeth)
To: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: bin/37656: fdisk segfault on invalid MBR entries on non-x86 
geometry detection
Date: Tue, 1 Jan 2008 12:35:29 -0800

 On Apr 18,  4:27am, eau%phear.org@localhost wrote:
 }
 } >Number:         37656
 } >Synopsis:       fdisk segfault on invalid MBR entries on non-x86 geometry 
detection
 } >State:          open
 } >Class:          sw-bug
 } >Arrival-Date:   Tue Jan 01 16:20:00 +0000 2008
 } >Originator:     Eric Auge
 } >Release:        netbsd 4.0
 } >Environment:
 } NetBSD client 4.0_STABLE NetBSD 4.0_STABLE (GENERIC) #0: Fri Dec 28
 } 13:23:05 CET 2007
 } 
root%bearcave.no.phear.org@localhost:/usr/obj/sys/arch/cobalt/compile/GENERIC 
cobalt
 } >Description:
 } On a cobalt, after few install tests, I got the following
 } 
 } # fdisk -S /dev/wd0
 } [1]   Trace/BPT trap          fdisk -S /dev/wd0
 } 
 } I was unable to partition the hard drive in order to have a correct 
partitioning scheme for installing my cobalt.
 } 
 } fdisk seems to make an almost correct sanity check on 
 } get_mapping function except in some cases where head returned in the MBR is 
0 on 2 consecutives mappings while these values are trusted to make division, 
fdisk crash with a divide by zero problem.
 } 
 } initial sanity check was the following line 1525 in fdisk.c:
 } [...]
 }     /* Sanity check the data against all zeroes */
 }     if ((*cylinder == 0) && (*sector == 0) && (*head == 0))
 }         return -1;
 } [...]
 } >How-To-Repeat:
 } have a fucked up MBR on your hardrive and try to install port-cobalt.
 } >Fix:
 } --- fdisk.c     2008-01-01 11:18:07.000000000 +0100
 } +++ fdisk-new.c 2008-01-01 11:18:46.000000000 +0100
 } @@ -1541,8 +1541,8 @@
 }                 *absolute = le32toh(part->mbrp_start)
 }                     + le32toh(part->mbrp_size) - 1;
 }         }
 } -       /* Sanity check the data against all zeroes */
 } -       if ((*cylinder == 0) && (*sector == 0) && (*head == 0))
 } +       /* Sanity check the data against cyl/sec/head zeroes */
 } +       if ((*cylinder == 0) || (*sector == 0) || (*head == 0))
 }                 return -1;
 }         /* Sanity check the data against max values */
 }         if ((((*cylinder * MAXHEAD) + *head) * MAXSECTOR + *sector) < 
*absolute)
 
      This change is wrong.  It is perfectly legal for cylinder and head
 to be 0.  This change would exclude any partition that starts on
 cylinder 0 from consideration, which is where the first partition
 normally starts.  It would also exclude any partition that starts with
 head 0 which is where primary partitions other then the first one
 normally start.  The effect is that you would most likely exclude all
 partitions.  The bug is in the code that tries to deduce the number of
 heads.  I'll have to think about how to fix this properly.
 
 }-- End of excerpt from eau%phear.org@localhost