Re: [PATCH 3/4] luks2: set up dummy sector size during scan

[Fabian Vogt]

Hi,

Am Samstag, 21. Mai 2022, 02:13:53 CEST schrieb Glenn Washburn:
> On Mon, 07 Feb 2022 14:15:07 +0100
> Josselin Poiret via Grub-devel <grub-devel@gnu.org> wrote:
> 
> > Hi Fabian,
> > 
> > Fabian Vogt <fvogt@suse.de> writes:
> > 
> > > Did you have a look at my approach? That effectively does the same, but 
> > > using a
> > > single ioctl instead of anything complex with DM directly.
> 
> I skipped this thread because I didn't really care about probing for
> LUKS2 (I don't use it). However, things change and now I'm evaluating
> the different patch series that implement this. I had missed Fabian's
> patch and I'm just now seeing it and this thread.
> 
> Now that I've taken a look at Fabian's patch, I think its the better
> way to go for getting the sector size. The reason is that it uses
> grub_util_get_fd_size() to get the size and sector size of the cheat
> mount device. That function's implementation is platform dependant. So,
> for instance, if FreeBSD has a LUKS* implementation that is not
> libdevicemapper based, we'll correctly get the number of sectors and
> sector size. But yes, its very unlikely that FreeBSD will support LUKS.
> However, this same code should allow us to get the correct sector size
> for GELI devices also. I don't believe GRUB has GELI probe support, so
> ths would make things easier for whoever wants to develop it. The same
> goes for future cryptodisk backends.
> 
> > I agree that it's sufficient for sector_size, but we still need the
> > cryptodisk algorithm so that grub-install will know which crypto modules
> > to include.  libdm is actually a helper library around dm-specific
> 
> I'm now thinking that GRUB should use Fabian's patch to get the sector
> size and number of sectors and Josselin's method of querying device
> mapper to get the other properies. We could have Josselin's patch first
> check if log_sector_size is set and if not then try to get it from DM.
> However, I think that's overkill and not really useful because
> grub_util_get_fd_size() should always give us the correct information,
> otherwise we've found a kernel bug. Also, by not having to parse out
> the sector size from the dm-crypt params string, we avoid much the
> complexity of that code, which has been the source of several bugs in
> reviewing a couple iterations of that patch.
> 
> > ioctls, since they are apparently annoying to use, so in the end we're
> > still using the same interface :)
> 
> Technically different interface, the code Fabian is using is not using
> device mapper related ioctls, they are generic block device ioctls (on
> Linux systems).
> 
> > As for the 4 patches, since the actual sector_size is available to us, I
> > think using 512 in all cases or adding a specific 0 sector_size is
> > something we should avoid.
> 
> I agree.
> 
> Fabian, would you be able to send an updated patch with Patrick's
> suggestions in the near future?

Sure! I'll be back from vacation after end of May, I hope that still counts.

Cheers,
Fabian

> Glenn