[9fans] BUG: disk/fdisk.c: corrupts MBR

[t***@polynum.com]

Hello,

Since I'm playing with the installation, I have been hit once again
with disk/fdisk corrupting the MBR and rendering other systems
unbootable.

The explanation is arithmetic truncation. disk/fdisk.c does this:

p->start = lba/sec2cyl;

and, if modifications are done (for example creating a Plan9 partition,
of marking a primary partition active), when written back, the entries
are "restored" by:

*endlba = (vlong)p->start*sec2cyl;

Of course, if the values were not integer multiples of sec2cyl, the
entry point for the PBS is corrupt. (For example, on *BSD, the principle
is the same as for Plan9: a "primary" partition is a sequence of
adjacent bytes, furthermore subdivided; but one special subdivision
[a] is at the same offset (at start) and holds the bootcode and a
disklabel describing the subdivision; if the MBR is corrupt, neither
boot nor disklabel so user can conclude: everything lost). And the first
travel in the flying saucer will also be the last...

And since the values in the MBR have almost no relation today with the
physical access of data; and furthermore, since partitions created by
other systems for other systems should be left alone (that's the main
purpose of the MBR: establish borders to tell others: this is my realm!
go away!)...

I will submit patches later, if no one does the job before, since there
are saved values (p->ctlstart = lba; etc.) that are here and not used,
so I have to review the code closely.

Until then: if you wish to dual boot, create the Plan9 partition before
with another system present on disk (and do print the layout of the MBR)
in order to skip this step in installation.

[erik quanstrom]

Post by t***@polynum.com
Hello,
Since I'm playing with the installation, I have been hit once again
with disk/fdisk corrupting the MBR and rendering other systems
unbootable.
p->start = lba/sec2cyl;
and, if modifications are done (for example creating a Plan9 partition,
of marking a primary partition active), when written back, the entries
*endlba = (vlong)p->start*sec2cyl;
Of course, if the values were not integer multiples of sec2cyl, the
entry point for the PBS is corrupt. (For example, on *BSD, the principle
is the same as for Plan9: a "primary" partition is a sequence of
adjacent bytes, furthermore subdivided; but one special subdivision
[a] is at the same offset (at start) and holds the bootcode and a
disklabel describing the subdivision; if the MBR is corrupt, neither
boot nor disklabel so user can conclude: everything lost). And the first
travel in the flying saucer will also be the last...

can you just set sec2cyl=1?

- erik

[t***@polynum.com]

Post by erik quanstrom

Post by t***@polynum.com
p->start = lba/sec2cyl;
and, if modifications are done (for example creating a Plan9 partition,
of marking a primary partition active), when written back, the entries
*endlba = (vlong)p->start*sec2cyl;

can you just set sec2cyl=1?

This wouls probably do the trick. But the code is clean and setting just
this is not---to my taste.

The ideal would be to simply patch the existing with changed values (no
need to be efficient for 4 * 16 bytes), and leave clearly alone values
set by others.

And to verify the magic signature for, at least, the active partition
(it would have spot the problem).

[erik quanstrom]

Post by t***@polynum.com

Post by erik quanstrom
can you just set sec2cyl=1?

This wouls probably do the trick. But the code is clean and setting just
this is not---to my taste.
The ideal would be to simply patch the existing with changed values (no
need to be efficient for 4 * 16 bytes), and leave clearly alone values
set by others.
And to verify the magic signature for, at least, the active partition
(it would have spot the problem).

the code assumes c/h/s. and if it (libdisk) can't find the geometry, it makes stuff
up. that seems like a dubious assumption. not very clean at all.

i was thinking that setting cyc2cyl=1 should cleanly remove this feature.
it could then be reenabled with a command-line flag if necessary for
some really ancient h/w.

- erik

[t***@polynum.com]

Post by erik quanstrom
the code assumes c/h/s. and if it (libdisk) can't find the geometry, it makes stuff
up. that seems like a dubious assumption. not very clean at all.
i was thinking that setting cyc2cyl=1 should cleanly remove this feature.
it could then be reenabled with a command-line flag if necessary for
some really ancient h/w.

We should be careful, because bootloaders whether take the c/h/s, or the
as is lba values (and the incorrect assumption is that c/h/s == lba
since this will defeat the purpose of lba).

But in the code, there are p->ctlstart and p->ctlend, that have the
correct values... and are not used. Perhaps simply using these values
when writing will suffice.

But whether I or another have/has to look carefully: doing blunders in
Plan9 zone is unfortunate but... Messing others' areas must be
forbidden. disk/fdisk(8) is the program that impacts others.

The MBR is the BIOS entry point so it shall not be botched. Furthermore,
with a booloader that has not the capability to select the primary to
boot, if the MBR is partially wrong, and the active partition not the
correct one, you have to have... a plan B (but perhaps not the well
known one ;).