ps2dev / ps2sdk

Homebrew PS2 SDK
Other
908 stars 130 forks source link

USBHDFSD does not mount unpartitioned disks #81

Closed blowfish64 closed 5 years ago

blowfish64 commented 5 years ago

The current version of USBHDFSD module does not mount unpartitioned disks, even if they are formatted as FAT16/FAT32 using the whole disk.

The file in question is part_driver.c: the fact is that the code to mount the whole disk is there, but due to how part_getPartitionRecord and part_getPartitionTable are written, it gets never executed.

By making the following 3 changes to the file in question, the problem gets fixed and disks with both with and without a partition table get mounted correctly (tested by compiling uLaunchELF 4.43a)

/iop/usb/usbhdfsd/src/part_driver.c

static USBHD_INLINE int part_getPartitionRecord(mass_dev* dev, part_raw_record* raw, part_record* rec)
{
    rec->sid = raw->sid;
    rec->start = getI32(raw->startLBA);
    rec->count = getI32(raw->size);

    if(rec->sid != 0x00)
    {   /*  Windows appears to check if the start LBA is not 0 and whether the start LBA is within the disk.
            There may be checks against the size, but I didn't manage to identify a pattern.
            If the disk has no partition table (i.e. disks with "removable" media), then this check is also one safeguard. */
        if((rec->start == 0) || (rec->start >= dev->maxLBA))
            return 1;else return 0;//this change is to accomodate the following change
    }
    else return 1;//return 1 in case of invalid or non populated partition record
    //return 0;
}
static int part_getPartitionTable(mass_dev* dev, part_table* part)
{
    part_raw_record* part_raw;
    int ret;
    unsigned int i;
    unsigned char* sbuf;

    ret = READ_SECTOR(dev, 0, sbuf);  // read sector 0 - Disk MBR or boot sector
    if ( ret < 0 )
    {
        XPRINTF("USBHDFSD: part_getPartitionTable read failed %d!\n", ret);
        return -EIO;
    }

    printf("USBHDFSD: boot signature %X %X\n", sbuf[0x1FE], sbuf[0x1FF]);
    if (sbuf[0x1FE] == 0x55 && sbuf[0x1FF] == 0xAA)
    {
        for ( i = 0; i < 4; i++)
        {
            part_raw = ( part_raw_record* )(  sbuf + 0x01BE + ( i * 16 )  );
            if((part_getPartitionRecord(dev, part_raw, &part->record[i]) != 0) && (i==0))//if(part_getPartitionRecord(dev, part_raw, &part->record[i]) != 0) //return no partition only if the first record is invalid
                return 0;   //Invalid record encountered, so the table is probably invalid.
        }
        return 4;
    }
    else
    {
        for ( i = 0; i < 4; i++)
        {
            part->record[i].sid = 0;;
        }
        return 0;
    }
}

Because of how it was written, the only way to mount the disk as a whole was to have a partition table indeed with one of the records specifying the whole disk.

sp193 commented 5 years ago

Am I correct to say that there are two main groups of changes:

But what if the first partition slot is just unused? Is such a disk really illegal?

I think I did my tests last year, with one of my thumb drives that had no partition table. But it has been so long that I forgot whether it really had no partition table. That thumb drive was erased various times after that, so I have no way to find out anymore.

The existing code just verifies whether all the 4 partition entries are valid for the disk, before deeming the disk as having a valid partition table. The criteria for a determining whether a partition is valid are as follows:

For this to not work, that means that either your 4 partition entries had 0s for partition types or non-zero start LBAs. Perhaps even a combination of both. If so, can we conclude that the errata is with not considering a partition type of 0x00 as invalid? But why should it only check the first partition?

blowfish64 commented 5 years ago

Generally speaking, only Iomega ZIP disks have the partition records 1, 2 and 3 empty, and the actual partition resides in record 4.

To be more correct, the code should check if at least one partition is not invalid, but I have never come across a USB hard disk or thumb drive which had the first record empty (or, which is the same, a single partition in a record different than the first).

By reading the code, you can notice that the check against start LBA is only done for partition system ID different than 0x00 so the errata is indeed about not considering 0x00 partition type invalid, in this way the other function code always returns that there are 4 partitions, never executing the part of code where the partition records are missing and trying the whole disk.

I agree that checking only the first partition record is not perfectly the right thing, as maybe mispartitioned HDDs might have such a strange setup.

As for testing, I tried different USB thumb drives formatted by myself with mkdosfs both using the whole disk and by making a single partition via fdisk, in both cases I zero-filled the first sector to start anew, and the result was always the same: disks formatted using the whole disk were never mounted by either uLE and OPL, while partitioned disks always mounted at the first try.

sp193 commented 5 years ago

You're right. It has to check that at least 1 partition is valid, for things to work. Please try using the latest commit (fbc39455), and let me know if it helps.

I always thought that Linux would allow you to have not all partition slots used. When we partition disks, we can see the device assignments (i.e. /dev/sda*). If you deleted a partition before, you could find something like /dev/sda1 and /dev/sda3, but not the deleted partition. But, I could be wrong because I have been going off memory. Linux isn't my main OS either.

Thanks for bringing this up BTW.

blowfish64 commented 5 years ago

Just tried recompiling uLE and OPL with the new version of the module and the result is that they both do work as expected, mounting partitioned and unpartitioned disks flawlessly.

If you want, we can call it a day :)

Just a sidenote: actually you can even tell fdisk to create, for example, partitions in records 1 and 4, however Linux will still report them as /dev/sda1 and /dev/sda2. Logical partitions start from /dev/sda5, but that's another matter.

sp193 commented 5 years ago

I see. Yes, perhaps that was what I saw (my PC was missing /dev/sda4).

If you have no further comments regarding this issue, we can close this. Personally, I think the change was acceptable and makes more sense than just leaving things be, even if it actually only happens when you use the GNU tools.

Once again, thanks.