Re: [Info-mtools] Incorrect assignment of partition type in mformat/mpartition

[Pali Rohár]

On Sunday 23 September 2018 23:07:53 Alain Knaff wrote:
> Hi,
> 
> On 17/08/18 00:21, Pali Rohár wrote:
> > On Saturday 11 August 2018 17:48:03 Pali Rohár wrote:
> >> Hi!
> >>
> >> I found out that mformat and mpartition incorrectly assigns MBR
> >> partition type. This is because of following problems:
> >>
> >> 1) Begin and end location of partition in function setBeginEnd() is of
> >> type "signed" integer which start overflowing when hitting 1TB partition
> >> size. It should be "unsigned" type, it does not make sense to have
> >> negative value. Result is that 1TB partitions are marked as FAT12
> >> because "end-begin < 4096" is truth (end is negative number).>>
> >> 2) There is missing partition type for FAT32 (MBR ID 0x0C).
> >>
> >> 3) Detection expects that sector size is 512 bytes, which is not truth
> >> for new Native 4K disks with sector size of 4096 bytes.
> >>
> >> In attachment I'm sending a patch which fixes all these 3 problems.
> >>
> >> I created FAT32 image with mformat -F and then via fdisk verified that
> >> partition type is 0x0C.
> > 
> > I did here one mistake. Partition types 0x01, 0x04 and 0x06 depend on
> > number of sectors, and not on number of (mega)bytes -- as was written in
> > original comments. 0x04 is used for original FAT16 and 0x06 for BIGDOS
> > FAT16 (or big FAT12). So 0x04 is only for partitions which has less then
> > 2^16 sectors (which is 32MB when disk sector size is 512b; this is where
> > original comment "<32M" comes from). Therefore sector size is not needed
> > in this function at all.
> > 
> > In attachment I'm sending fixed version of my previous patch
> > mtools_partition.patch.
> > 
> 
> Thanks for the patch.
> 
> As there were no references cited for the changed definitions of the
> partition types, I had to do some googling around, and found the
> following wikipedia article which does indeed seem to support (most of)
> your proposed changes:
> 
> https://en.wikipedia.org/wiki/Partition_type#List_of_partition_IDs
> 
> There is only one difference which struck my eye: in order to chose
> between 0x06 and 0x0e you check whether the partition is entirely below
> the 1024 cylinder mark. However, wikipedia claims that the cutoff should
> be at 8GB (or rather 16 G sectors). The Microsoft source cited by
> wikipedia
> (https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-2000-server/cc977219(v=technet.10)
> claims a cut-off of 4GB...

IIRC Microsoft implementations support FAT16 filesystem only up to the
4GB size. Therefore I guess this is why they specified 4GB limit also
for 0x06 MBR id which belongs to FAT16.

> So, now I'm puzzled which of the 3 is correct. Or maybe you still have
> other supporting documentations which you'd like to share? In any case,
> 8GB does indeed roughly correspond to 1024 cylinders if 63 sectors and
> 255 heads are used (which would indeed be picked by LBA assyst for 8GB)

I do not have any "official" documentation. But 1024 cylinders is for
sure upper limit. When you are using CHS geometry, then you store 10 bits
for cylinders into MBR partition table. Therefore every CHS partition
must end at 1024th cylinder. And because 0x06 MBR type indicates usage
of CHS, maximal theoretical size is of 0x06 partition in number of
sectors is: sectors * heads * 1024.

I guess that people started referring to 8 GB, just because it is "near"
63*255*1024 sectors (=~ 8032.5 MB =~ 7.84 GB).

Number 8 GB itself does not make much sense -- when all addressing was
done by CHS and by DOS interrupts which took tuple (C, H, S).

> In the meantime, I re-arranged the tests to make it easier to understand
> what is going on:

That is not exactly same code.

You forgot for case when FAT12 partition is bigger then 4096 sectors. In
that case 0x06 should be used too. Reason is that 0x01 is supported by
old DOS versions only for FAT12 partitions with less then 4096 sectors.
In my original patch this case was included too.

> if(!type) {
>       if (fat_bits == 0) {
>               /**
>                * Fat bits unknown / not specified. We look
>                * at size to get a rough estimate what FAT
>                * bits are used.  Note: this is only an
>                * estimate, the precise calculation would
>                * involve the number of clusters, which is
>                * not necessarily known here.
>                */
>               /* The cutoff number 32680 is based on cc977219,
>                * corresponding to a FAT12 partition with 4K
>                * clusters */
>               if(end-begin < 32680)
>                       fat_bits = 12;
>               else
>                       fat_bits = 16;
>       }
> 
>       /* Description of various partition types in
>        * https://en.wikipedia.org/wiki/Partition_type#List_of_partition_IDs
>        * and
>        *
> https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-2000-server/cc977219(v=technet.10)
>        */
>       if (fat_bits == 32)
>               /* FAT 32 partition. For now, we disregard the
>                * possibility of FAT 32 CHS partitions */
>               type = 0x0C; /* Win95 FAT32, LBA */
>       else if (end < 65536) {
>               /* FAT 12 or FAT 16 partitions which fit entirely below
>                  the 32M mark */
>               /* The 32M restriction doesn't apply to logical
>                  partitions within an extended partition, but for the
>                  moment mpartition only makes primary partitions */
>               if(fat_bits == 12)
>                       /* FAT 12 partition */
>                       type = 0x01; /* DOS FAT12, CHS */
>               else if (fat_bits == 16)
>                       /* FAT 16 partition */
>                       type = 0x04; /* DOS FAT16, CHS */
>       } else if (end < sectors * heads * 1024)
>               /* FAT 12 or FAT16 partition above the 32M mark
>                * but below the 1024 cyliner mark */
>               /* Shouldn't that be the 8GB mark (wikipedia) or
>                * 4GB (Microsoft)? */
>               type = 0x06; /* DOS BIG FAT16 or FAT12, CHS */
>       else
>               type = 0x0E; /* Win95 BIG FAT16, LBA */
>       }
> 
> Regards,
> 
> Alain

-- 
Pali Rohár
address@hidden

signature.asc
Description: PGP signature