Skip to content

Conversation

@nrclark
Copy link
Contributor

@nrclark nrclark commented Oct 30, 2025

Fix some corner-cases with CHS calculation when filling out MBR partition tables. Make sure that CHS values are correctly clamped the requested LBA is too big to be representable as a CHS, and fix the calculation when LBA=0.

When creating a hybrid GPT/MBR, change the GPT protective partition size from (hd->gpt_location / 512 + GPT_SECTORS - 2) to GPT_SECTORS, which should be more generally correct (and evaluates to the same value in the hybrid case).

Fix some corner-cases with CHS calculation when filling out MBR
partition tables. Make sure that CHS values are correctly clamped
the requested LBA is too big to be representable as a CHS, and fix
the calculation when LBA=0.

When creating a hybrid GPT/MBR, change the GPT protective partition
size from (hd->gpt_location / 512 + GPT_SECTORS - 2) to GPT_SECTORS,
which is more generally correct (and evaluates to the same value
in the hybrid case).

Signed-off-by: Nicholas Clark <[email protected]>
@nrclark
Copy link
Contributor Author

nrclark commented Nov 1, 2025

@michaelolbrich some context on this: I'm working on a PR that will add support for 4kB block sizes (or 2^N, (N>=9) more generally). While doing that work, I came across a couple of smaller items that I also wanted to fix. I thought that it would make sense to split them out from my other branch, so that they can be evaluated separately.

The change to hdimage_setup_chs() intersects with the new block-size feature because it my branch changes how GPT_SECTORS works. While I was in there I also read a few writeups on CHS calculation to learn what the current code does. While evaluating / reading lba_to_chs(), I came across some corner cases that might not be handled correctly today. Those are:

  • CHS values aren't zero-indexed, so LBA=0 should map to CHS=0,0,1
  • the output should cap at {0xFE, 0xFF, 0xFF}

Feel free to take or close this PR as you see fit, since lba_to_chs() since CHS values are mostly skipped by systems these days and the other change could also be put in the upcoming block-size PR.

I'd be especially grateful if you could check out the change to hdimage_setup_chs(), and maybe give some context for how entry->total_sectors is currently calculated. My take on it was that since the actual partition being created holds the GPT table, that the size of the GPT table should also be the size of the partition. That's true today, but the current calculation also adds two sectors and then subtracts 2. I couldn't come up with a reason for this, and thought that it might have been a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant