grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Two Small Patches (x86 VolId & Sun Label Checking)


From: ehem+grub
Subject: Re: Two Small Patches (x86 VolId & Sun Label Checking)
Date: Tue, 28 Dec 2010 22:25:53 -0800 (PST)

>From: Vladimir '?-coder/phcoder' Serbinenko <address@hidden>
> On 12/26/2010 10:15 PM, address@hidden wrote:
> > Quite simple, the disk slice scheme detection routines vary in the
> > quality of their detection. In particular, the MSDOS-style detection is
> > *extremely* brittle. The only even mildly distinguishing characteristic
> > it finds is ensuring only the msb of the boot-flag byte is set. The other
> > thing it looks for is the 0xAA55 signature, but that is merely a signal
> > to PC-BIOSes that the disk is bootable; as such *any* bootable disk for a
> > IBM-PC will have that signature, whether or not it is actually using the
> > MSDOS-style header. A 1 in 65536 chance of a false positive is bad.

> Actually 1 in 2^(7*4+16) =2^44 in you take into account the both checks
> and consider every possible sector equiprobable. While this is a
> problem, it's design problem of this partitioning label. More sanity
> checks are possible but they would be heuristics and increase the
> possibility of false negative. So every additional sanity check is to be
> considered on case-by-case basis.

Alas, seeing how every possible sector is very much *not* equiprobable,
there is real potential for problems here. Taking a look at the example I
very quickly generated. It came straight out of the generic Linux `fdisk`
program (press 's' for a Sun disklabel), with the only alteration being
to mark it bootable for PC-BIOSes (helpful if one wants a PC-BIOS to boot
from it) and adjusting the checksum to compensate. The result, no need
for anything extra, those 28 bits came set to zero for free. Effectively,
any disk that is bootable in a x86 PC will test positive for having the
MS-DOS-style table (most will in fact have one, but a few oddballs
won't).



> > Whereas most of the other schemes have actual magic numbers for the
> > disk-slice scheme, that is *not* merely a flag for whether it is okay to
> > boot from or not (plus checksums, which push them to 1 in 2^32 chance of
> > incorrect detection).
> >
> > Take a look at the attached file, it is ment as a header for a 512KB
> > image (`dd if=/dev/zero count=1023 2>/dev/null | cat sample /dev/stdin >
> > full_sample`). The only reason it will be correctly detected as a
> > SunOS-style disk label is that routine gets tried first, the MSDOS-style
> > detection would take it as valid.

> Recent GRUB don't reject multiple disklabels per disk and you can access
> all the partitions described by all of them. E.g:
> (hd0,msdos1) vs (hd0,sun1)
> While false-positive looks ugly in ls and slows GRUB down (checking for
> filesystem in ghost partitions) it's only mildly affected. False
> negatives on the other hand may prevent GRUB from booting altogether

> Improving quality of partmap detection is a good goal but be aware of
> the price of heuristics.

That isn't my thinking. I was thinking of having it test for the various
schemes first, then choosing the best fit. While trying multiple schemes
is viable for grub_partition_iterate(), it doesn't work when installing
boot code or attempting to do partition modification (since both of these
*must* know in order to function).



Organizational item here. Is the existing layout of <task>/<arch> for the
best? (task would be boot/partmap/parttool, arch is pretty much every .c
file in grub-core/partmap) I wonder if perhaps a structure like
<arch>/<task> would work better?

Seeing how the boot code/installation code *must* know the
disk-label/partitioning-type in order to fit within the limitations
thereof, the latter seems to make sense. Certainly the existing layout is
pretty conventional, but the latter seems to match how things need to be
shared better.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         address@hidden PGP F6B23DE0         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
2477\___\_|_/DC21 03A0 5D61 985B <-PGP-> F2BE 6526 ABD2 F6B2\_|_/___/3DE0





reply via email to

[Prev in Thread] Current Thread [Next in Thread]