Date: Sun, 22 May 2011 20:29:10 -0600 From: Warner Losh <imp@bsdimp.com> To: Marcel Moolenaar <marcel@xcllnt.net> Cc: Adrian Chadd <adrian@FreeBSD.org>, src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, "Andrey V. Elsukov" <ae@FreeBSD.org>, Stefan Farfeleder <stefanf@FreeBSD.org>, svn-src-head@FreeBSD.org Subject: Re: svn commit: r221972 - head/sys/geom/part Message-ID: <42C49AE5-C8EA-44A0-AF88-16130BACE912@bsdimp.com> In-Reply-To: <6AE10D76-AC2F-4D7B-A985-EE072949ECC4@xcllnt.net> References: <201105152003.p4FK3tnS050889@svn.freebsd.org> <20110522093302.GA2638@mole.fafoe.narf.at> <BANLkTikoBK4ZCHB488eRgbySPBcXC0nnow@mail.gmail.com> <BD2C1F30-854A-4AF2-A9D8-654F3E4E84A8@bsdimp.com> <6AE10D76-AC2F-4D7B-A985-EE072949ECC4@xcllnt.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On May 22, 2011, at 2:28 PM, Marcel Moolenaar wrote:
>=20
> On May 22, 2011, at 9:26 AM, Warner Losh wrote:
>=20
>>=20
>> On May 22, 2011, at 7:03 AM, Adrian Chadd wrote:
>>=20
>>> This also bit me on embedded platform stuff.
>>>=20
>>> Is it possible to disable this by default for now and have it just =
warn loudly?
>>> And/or hide the default value behind a kernel configuration variable
>>> so we can disable it
>>> (but still get the warnings) for now?
>>=20
>> Or just delete it entirely as a bad idea? We had this with Marcel's =
warning for a long time that turned out to be utterly bogus so we =
removed it.
>=20
> Really?
>=20
> The warning wasn't bogus. It was there to help us understand what was
> going on and we found over time that it was a harmless condition.
> That's why we removed the warning. The warning was good to raise
> awareness.
Except from the moment it went in we knew that the problem it was =
reporting was often due to the artificial geometries used throughout the =
system. That was the root cause of that problem: the firmware reported =
fake geometry and the MBR described a different geometry. The rest of =
the world ignored the firmware reported geometry and used what was =
described in the MBR.
> This is in no way similar to what we have now. Here we have to deal =
with
> broken and fundamentally invalid MBRs that has caused us harm before =
and
> we're trying to do something sensible. As it turns out, a whole bunch =
of
> people have invalid MBRs, probably caused by crappy tools. Now what do
> you suggest we should do? Accept it silently and suffer the =
consequences
> later, should we raise awareness so that administrators can try and =
fix
> things or should we reject the MBR out of pedanticism?
>=20
> Rather than just calling it a bad idea, why not come up with something
> constructive?
Looking at one of my flash drives that shows the problem:
da0: 15423MB (31588351 512 byte sectors: 255H 63S/T 1966C)
GEOM_PART: partition 1 has end offset beyond last LBA: 31588350 > =
31588325
So why does gpart think the last LBA is 25 less than it really is. =
Well, let's do some sanity checks first. fdisk -s da0 tells us:
/dev/da0: 1966 cyl 255 hd 63 sec
Part Start Size Type Flags
1: 63 31588288 0x0b 0x00
which puts the ending sector at 31588350, which should be fine. =
Diskinfo reports the correct info:
sudo diskinfo da0
da0 512 16173235712 31588351 0 0 1966 =
255 63
I'm pretty sure this problem is due to a bug in g_part_mbr.c:
basetable->gpt_last =3D msize - (msize % basetable->gpt_sectors) =
- 1;
This is wrong, or at least it is a widely disregarded part of what makes =
up an MBR. When I correct the size, the geom code is fine. There's no =
requirement in MBR that a partition end of any particular boundary, =
although sometimes you'll find mistaken documentation that suggests this =
is the case. Reading between the lines at =
http://www.boot-us.com/gloss03.htm suggests that this restriction was =
only for disks < 8GB in size (from the fact it said that all the =
partitions can be described with the CHS fields). This is one of the =
things I learned when I tried to make fdisk enforce that: this isn't a =
requirement of MBR as it is implemented in the wild today.
Replacing the above with:
basetable->gpt_last =3D msize - 1;
seems to fix the problem. I'm unsure if this should be unconditional, =
or just for disks that are larger than 8GB in size. Given that my thumb =
drive that triggers the bogus behavior is 16GB in size, I don't know for =
sure. I have one or two 2GB drives that didn't trigger the problem. =
They work either way.
Please note: this is with a USB drive that I've not repartitioned since =
I brought it home. So this isn't a tool issue. This is likely why 6/7 =
of phk's USB thumb drives have failed.
So please, let's change the code. A similar change is necessary when =
creating the MBR as well, at least for media that's 8GB or larger.
I'll admit that I was wrong about WHERE the bogusity was (I thought it =
was the checks). I was right that it was a bogisty in the FreeBSD code, =
and nothing wrong with my media. MacOS, Linux and Windows handle it =
correctly, which strongly suggests there's a bug in FreeBSD's MBR =
handling.
Warner=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?42C49AE5-C8EA-44A0-AF88-16130BACE912>