>Number:         4772
>Category:       port-i386
>Synopsis:       readdisklabel() can construct ludicrous geometries
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    gnats-admin (GNATS administrator)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jan  3 20:05:00 1998
>Last-Modified:
>Originator:     Greg Hudson
>Organization:
MIT
>Release:        1.3
>Environment:
	
System: NetBSD the-light-fantastic 1.2 NetBSD 1.2 (LIGHT) #2: Mon Apr 7 17:36:19 EDT 1997 root@the-light-fantastic:/u1/marthag/src/sys/arch/i386/compile/LIGHT i386
	(This bug has existed for a Long Time and still exists as of this
writing.)


>Description:
readdisklabel() (in arch/i386/i386/disksubr.c) makes the assumption that
the NetBSD MBR partition ends at the end of a cylinder and uses that to
correct the values of d_ntracks and d_nsectors.  That's currently at line
139:

					lp->d_ntracks = dp->dp_ehd + 1;
					lp->d_nsectors = DPSECT(dp->dp_esect);
					lp->d_secpercyl =
					    lp->d_ntracks * lp->d_nsectors;

However, this code leaves the cylinders value alone, so you get ntracks and
nsectors values from the disk's translated geometry and an ncylinders value
from the disk's real geometry.  Frequently, that means the geometry
multiplies out to something an order of magnitude larger than the disk.

This problem has some consequences in fdisk which I will document in a
separate PR.

>How-To-Repeat:
On a disk using a translated geometry, with a NetBSD partition which hasn't
had a disklabel written to it yet, take a look at the geometry reported by
disklabel.

>Fix:
One fix is to adjust the number of cylinders when we set the d_ntracks and
d_nsectors values:

*** disksubr.c.orig	Sat Jan  3 22:52:52 1998
--- disksubr.c.adjust	Sat Jan  3 22:58:34 1998
***************
*** 140,145 ****
--- 140,147 ----
  					lp->d_nsectors = DPSECT(dp->dp_esect);
  					lp->d_secpercyl =
  					    lp->d_ntracks * lp->d_nsectors;
+ 					lp->d_ncylinders =
+ 					    lp->d_nsecperunit / lp->d_secpercyl;
  				}
  			}
  			lp->d_npartitions = RAW_PART + 1 + i;

I tend to prefer a different fix, which is to remove the assumption that the
NetBSD partition ends at the end of a cylinder.  It's not a terrible
assumption (it's made by other operating systems in various places), but it
doesn't buy us a whole lot, and it's not consistent with the way fdisk
determines the translated geometry.

*** disksubr.c.orig	Sat Jan  3 22:52:52 1998
--- disksubr.c.remove	Sat Jan  3 22:57:49 1998
***************
*** 136,145 ****
  					    dp->dp_size;
  					lp->d_partitions[2].p_offset = 
  					    dp->dp_start;
- 					lp->d_ntracks = dp->dp_ehd + 1;
- 					lp->d_nsectors = DPSECT(dp->dp_esect);
- 					lp->d_secpercyl =
- 					    lp->d_ntracks * lp->d_nsectors;
  				}
  			}
  			lp->d_npartitions = RAW_PART + 1 + i;
--- 136,141 ----
>Audit-Trail:
>Unformatted: