stand/common/disk.c: Read the partition table relative to the start of the drive

[allanjude]

Summary

If a disk is of an oddball size (200mb + 512b, in the case of rootgen.sh), when disk_open() is called on a GELI encrypted partition, attempts to read the partition table fail, as they pass through the decryption process which turns the already plaintext data into jibberish.

When reading the partition table, always pass a slice and partition setting of -1, and an offset of 0.

[imp]

I'm leery of committing 'I'm not sure why, but this seems to make it work' kind of code.

On the other hand, Ian cleans up this part of the code a lot so maybe it's not a big deal and we'll just use his cleanup when it's ready. He wants testability, and this will further that need.

stand/i386/libi386/biosdisk.c

908 ↗

(On Diff #43877)

I'd think you'd want to return an error here...

[allanjude]

In D15847#335079, @imp wrote:

I'm leery of committing 'I'm not sure why, but this seems to make it work' kind of code.

On the other hand, Ian cleans up this part of the code a lot so maybe it's not a big deal and we'll just use his cleanup when it's ready. He wants testability, and this will further that need.

To clarify: The part I do not understand is why in the case where the disk is exactly 200mb, the system boots before attempting this pathological case, and failing.

I will spend a bit more time on it today and see what I can find.

stand/i386/libi386/biosdisk.c

908 ↗

(On Diff #43877)

No, we definitely do not. This is where it is trying to read sector 0, to read the partition table. If we error here, it won't boot. This change makes it not apply the decrypt pass after having read the partition table, because decrypted the not-actually-encrypted partition table turned it into jibberish data.

[allanjude]

Update to also not decrypt the left over bits of partial 4k sectors

[• ian]

stand/i386/libi386/biosdisk.c

908 ↗

(On Diff #43877)

If this is happening while reading sector 0, then something has gone very wrong, because to read sector 0 you need to set disk_devdesc.d_slice and d_partition to -1. If those are -1 as they should be, then the error is likely to be that lines 853 and 856 are actually reading an array index of -1 and that's how this code is coming into play at all when it shouldn't be.

[allanjude]

stand/i386/libi386/biosdisk.c

908 ↗

(On Diff #43877)

It is happening when calling disk_open() on the partition, but maybe that is a better place to try to address it.

[allanjude]

Update with a much more satisfying fix, as suggested by @ian

[• ian]

I think this looks like a good solution; I'm surprised that trying to munge just the d_offset of the original disk_devdisk while leaving the other fields untouched actually worked as well as it did for so long.

In some ways it's starting to feel like it would be better to make some sort of layering inherent in the disk_devdesc somehow to create the ability to do geom-like stacking (not related to this change of course, just musing about the future).

[• ian]

It might be worth adding another sentence to the summary on commit, along the lines of "Setting the slice to -1 prevents a false positive when checking the cached values of which slices are geli-encrypted in biosdisk.c"