grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Glenn Washburn
Subject: Re: [PATCH 3/4] luks2: set up dummy sector size during scan
Date: Fri, 20 May 2022 19:13:53 -0500

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?

Glenn



reply via email to

[Prev in Thread] Current Thread [Next in Thread]