[U-Boot] [PATCH v1] DOS_PBR block type is also valid dos block type.

[Stephen Warren]

On 03/11/2013 08:59 PM, Sonic Zhang wrote:
> Hi Stephen,
> 
> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 03/11/2013 03:56 AM, sonic.adi at gmail.com wrote:
>>> From: Sonic Zhang <sonic.zhang at analog.com>
>>>
>>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos().
>>
>> What problem does this solve?
>>
>> I don't believe this change is correct. The purpose of test_part_dos()
>> is to determine whether a block device contains an MS-DOS partition table.
>>
>> Such a partition table is present in an MBR, but not a PBR. A PBR
>> contains a *FAT file-system, and does not include a partition table.
> 
> The SD card formated by windows 7 into one FAT partition can't be
> initialized correct in u-boot function init_part() after you reuse the
> function test_block_type() in function test_part_dos(). So, files on
> that partition can't be displayed when running command "fatls mmc 0".
> 
> The only difference in your change is to mark dos partition with flag
> DOS_PBR invalid.

Thanks for sending me the disk image.

The image is a mess; it's been manipulated by a variety of tools at
different times that have left rather a lot of cruft there.

The first sector does appear to be an actual MBR, containing a single
partition starting at LBA 0x10 (byte offset 0x2000), and quite large in
size. At LBA 0x10, I do see what may be the start of a FAT16
file-system. So far, so good.

However, the partition table contains the string "FAT32" at 0x52, and
also the string "mkdosfs" at 0x03. I believe that in the past, mkdosfs
was used on this card to create a raw FAT filesystem without any
partition table. Then later, some partitioning tool was run to create
the partition I mentioned above. Finally you said that Windows was used
to create the FAT filesystem within the partition. However, the
partitioning tool didn't wipe out the region of the MBR that contains
the boot code, and hence didn't wipe out the "FAT32" filesystem signature.

Finally, in LBA 3 (byte offset 0x600), I see another sector that looks
remarkably like the start of a (presumably long-gone) FAT filesystem.
Perhaps an old partition table on this device contained a partition that
started in this (non-cylinder-aligned) sector. This sector contains the
same "mkdosfs" and "FAT32" signatures.

If we take your patch, we end up with the following situation:

With your strange partition table:

  ls mmc 0
  ls mmc 0:auto
    -> Thinks there's a partition table, so works, and picks
       partition 1.
  ls mmc 0:0
    -> Explicit request for "partition" 0 (whole-disk). This option
       doesn't make sense here, since the whole-disk is not a
       file-system, but rather a partitioned device.

With a real raw FAT filesystem; no partitions:

  ls mmc 0
  ls mmc 0:auto
    -> Thinks there's a partition table, so tries to access a non-
        existent partition table entryrather than the whole disk,
        so automatic mode fails.
  ls mmc 0:0
    -> Explicit request for "partition" 0 (whole-disk), so works.

So the issue is that the automatic handling of raw FAT filesystems (i.e.
use of the entire disk rather than the first partition) fails with your
patch. Perhaps it's acceptable that people with raw FAT filesystems must
explicitly specify ":0" to access the whole disk, and we accept that
automatic mode won't work? I'll let Tom or Wolfgang make the call.

As far as I can tell, the Linux kernel never looks at the "FAT" or
"FAT32" strings in the MBR, and hence accepts your disk as having a
partition table. And since in Linux you must always use a specific
device (/dev/sda or /dev/sdaN), this issue doesn't arise. U-Boot's
automatic partition-or-whole-device selection is something Linux doesn't do.

One other thing to note: commands such as "mmc part" or "part list"
won't work for your disk. After my patch d1efb64 "disk: part_dos: don't
claim whole-disk FAT filesystems", if test_block_type()!=DOS_MBR (i.e.
in your case), then print_partition_extended() will simply print an
error. Before that patch, if test_block_type()==DOS_PBR (i.e. in your
case) then print_partition_extended() would print a fake partition table
entry that covered the whole disk. Neither action is correct for your
disk since it imagine that there was a raw FAT filesystem covering the
entire disk. In other words, U-Boot's partition table printing commands
never worked correctly on your disk, even if accessing the file-system
(accidentally?) used to!

Another solution here is for you to simply:

# Back up your MBR in case something goes wrong.
dd if=/dev/whatever of=backup.bin bs=1 count=512

# Zero out the boot code portion of your MBR,
# which will also zero out the false "FAT32" signature.
dd if=/dev/zero of=/dev/whatever bs=1 count=446 conv=notrunc

Alternatively, if there's still some command in Windows that will
install a regular MS-DOS/Windows MBR boot code onto your disk, use that
(fdisk /mbr???). Presumably such a command would over-write only those
same first 446 bytes, and leave the actual partition table in tact.