Avoid reading past the end of the disk in zfsboot.c and biosdisk.c

[allanjude]

Summary

There were still cases (an oddly sized disk) where we might read off the end of the disk

[imp]

Are there other disks that we should give this treatment to? Like all of them?

[allanjude]

In D15844#335074, @imp wrote:

Are there other disks that we should give this treatment to? Like all of them?

This treatment is mostly only necessary where the GELI code rounds reads up to 4kb, since the data is encrypted in 4k sectors.

@ian may need to look at this for his set of patches, but the issue should not exist anywhere else (@tsoome already took care of fixing this for the bcache read-ahead case quite a while ago)

[• ian]

This review was open for less than 9 hours, and was accepted by a reviewer 4 seconds after it was first opened. Seems like the very definition of pro-forma.

More to the point, it's not clear to me that this is correct or necessary. A geli provider is always an integral number of geli-provider-sized blocks, there is no "partial block" at the end which would require doing a short read.

Do we have any idea what led to attemping to read past the end? If it happens only during tasting, that's probably because the current logic/code for tasting is a bit suspect in how it decides which sectors to try, in which case, that's what should be fixed. If it happens during normal filesystem IO then something odd is going on that we should understand better.

[allanjude]

In D15844#335494, @ian wrote:

Do we have any idea what led to attemping to read past the end? If it happens only during tasting, that's probably because the current logic/code for tasting is a bit suspect in how it decides which sectors to try, in which case, that's what should be fixed. If it happens during normal filesystem IO then something odd is going on that we should understand better.

The rounding code is what causes the attempt to read past the end of the disk.
Since the physical disk may be 512b sectors (and in a legacy boot scenario, we can only read it in 512b sectors), but the GELI device is encrypted in 4k sectors, we round the reads up to the nearest aligned 4kb, so they can be decrypted, then return the requested portion of that to the caller.
You cannot decrypt the middle 2kb of a GELI block, hence the rounding. However, if you are trying to read the last 9 sectors of a disk, rounding that up to 16 sectors, reads past the end of the disk, if, and only if, the size of the disk is not evenly divisible by 4kb.

[• ian]

In D15844#335573, @allanjude wrote:

In D15844#335494, @ian wrote:

Do we have any idea what led to attemping to read past the end? If it happens only during tasting, that's probably because the current logic/code for tasting is a bit suspect in how it decides which sectors to try, in which case, that's what should be fixed. If it happens during normal filesystem IO then something odd is going on that we should understand better.

The rounding code is what causes the attempt to read past the end of the disk.
Since the physical disk may be 512b sectors (and in a legacy boot scenario, we can only read it in 512b sectors), but the GELI device is encrypted in 4k sectors, we round the reads up to the nearest aligned 4kb, so they can be decrypted, then return the requested portion of that to the caller.
You cannot decrypt the middle 2kb of a GELI block, hence the rounding. However, if you are trying to read the last 9 sectors of a disk, rounding that up to 16 sectors, reads past the end of the disk, if, and only if, the size of the disk is not evenly divisible by 4kb.

The point is, if something is attempting to read the last 9 512-byte sectors of a geli provider which is formatted with 4k sectors, then insanity has already happened at some higher layer. The geli metadata says what the geli provider sector and media sizes are, and nothing except tasting should ever even attempt to read a size or offset that isn't sector-aligned. The process of creating a geli provider does rounding to ensure there is never a short block at the end if the underlying provider's size isn't an even multiple of the geli provider's size.