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

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

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

From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
To: jnemeth%victoria.tc.ca@localhost
Cc: eau%phear.org@localhost, gnats-bugs%NetBSD.org@localhost, 
netbsd-bugs%NetBSD.org@localhost,
        tsutsui%ceres.dti.ne.jp@localhost
Subject: Re: bin/37656: fdisk segfault on invalid MBR entries on non-x86 
geometry
         detection
Date: Sat, 5 Jan 2008 01:48:08 +0900

 jnemeth%victoria.tc.ca@localhost wrote:
 
 > } my understanding is that on non-x86 arch you check the MBR to guess the
 > 
 >      It does on x86 as well (see below and note that none of the head
 > numbers makes sense).
 
 On x86 machines intuit_translated_geometry() is not called directly
 but get_geometry() is called first.
 
 > } geometry ? what about brand new harddrives, MBR must always be null no ?
 > 
 >      Not sure how this is handled, but yes the MBR would be all 0s.
 
 The original investigation by Eric is:
 ---
 >> So I guess it's failing in this subpart of intuit_translated_geometry()
 :
 >> 
 >> [...]
 >> 1459     /* Try to deduce the number of heads from two different mappings. 
 >> */
 >> 1460     for (i = 0; i < MBR_PART_COUNT * 2 - 1; i++) {
 >> 1461         if (get_mapping(i, &c1, &h1, &s1, &a1) < 0)
 >> 1462             continue;
 >> 1463         a1 -= s1;
 >> 1464         for (j = i + 1; j < MBR_PART_COUNT * 2; j++) {
 >> 1465             if (get_mapping(j, &c2, &h2, &s2, &a2) < 0)
 >> 1466                 continue;
 >> 1467             a2 -= s2;
 >> 1468             num = (uint64_t)h1 * a2 - (uint64_t)h2 * a1;
 >> 1469             denom = (uint64_t)c2 * a1 - (uint64_t)c1 * a2;
 >> 1470             if (denom != 0 && num % denom == 0) {
 >> 1471                 xheads = num / denom;
 >> 1472                 xsectors = a1 / (c1 * xheads + h1);
 >> 1473                 break;
 >> 1474             }
 >> 1475         }
 >> 1476         if (xheads != -1)
 >> 1477             break;
 >> 1478     }
 >> [...]
 >> 
 >> But I'm not sure what it is doing... :/
 >> 
 >> I keep investigating..
 >
 > the crash was due by a divide by 0 on xsectors line 1472, as xheads == 0,
 > because num == 0, num == 0 because h1 == 0 and h2 == 0
 > returned by get_mappings()...
 ---
 
 Maybe we have to change this line
 >> 1470             if (denom != 0 && num % denom == 0) {
 to
 >> 1470             if (denom != 0 && num != 0 && num % denom == 0) {
 to avoid a possible division by zero?
 ---
 Izumi Tsutsui