[parted-devel] [PATCH] libparted: Fail early when detecting nilfs2

[Oldřich Jedlička]

Hi Brian,

Ah, I did not intend to make a behaviour change actually. Anyway, I
checked the sources [1] and the nilfs2 code makes both super blocks
mandatory, so it is reasonable.

Some background. Cryptsetup added SED (self-encrypting drives) support
in version 2.7.0 [2], which uses kernel's support for TCG Opal HW
encryption [3]. This technology is present on many SSD/NVME drives and
can lock the disk regions (encrypt the data in HW and protect the
access to the data encryption key by a password). Reading from such
locked regions ends up in kernel reading errors (unless unlocked by a
password) and when being on SCSI, it might be cryptic/nonsensical [4]
(this is an actual reading error I see) and in my case leads to
degradation of speed (switch from UDMA/133 to UDMA/33).

The patch is actually a workaround for the kernel inability to work
with the Opal disk properly (it needs to handle reading errors from
locked areas better). My intention was to make a workaround for just a
few tools to allow booting without errors (like patching the blkid
tool [5], which is used by udev rules) - hopefully until this is
handled in the kernel. The partprobe tool is not used during boot,
though, but was mentioned in cryptsetup issue [6], so I checked if
there is a simple solution for that too.

I will re-send the patch and add the reasoning.

[1] https://github.com/nilfs-dev/nilfs-utils/blob/master/lib/sb.c#L135-L158
[2] https://lore.kernel.org/all/cd409f6c-5d51-482c-8a26-340822754ff1@gmail.com/
[3] https://www.atpinc.com/blog/tcg-opal-encrytion-sed-self-encryption-drive
[4] https://lore.kernel.org/all/20230623181908.2032764-1-lorenz@brun.one/
[5] https://github.com/util-linux/util-linux/pull/2882
[6] https://gitlab.com/cryptsetup/cryptsetup/-/issues/872

Cheers,
Oldrich.

út 2. 4. 2024 v 2:44 odesílatel Brian C. Lane <bcl at redhat.com> napsal:
>
> On Fri, Mar 29, 2024 at 03:39:44PM +0100, Oldřich Jedlička wrote:
> > The possibly checked LUKS area might be protected with OPAL HW encryption
> > and not yet fully unlocked, so try to detect nilfs2 at the beginning of the
> > area first and if that succeeds, detect it at the end of the area. This
> > prevents having read errors from the locked area.
> >
> > Signed-off-by: Oldřich Jedlička <oldium.pro at gmail.com>
> > ---
> >  libparted/fs/nilfs2/nilfs2.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/libparted/fs/nilfs2/nilfs2.c b/libparted/fs/nilfs2/nilfs2.c
> > index 6204542..40f8567 100644
> > --- a/libparted/fs/nilfs2/nilfs2.c
> > +++ b/libparted/fs/nilfs2/nilfs2.c
> > @@ -118,11 +118,12 @@ nilfs2_probe (PedGeometry* geom)
> >
> >       if (ped_geometry_read(geom, buf, 0, sectors))
> >               sb = (struct nilfs2_super_block*)(buf + 1024);
> > +     if (!sb || !is_valid_nilfs_sb(sb))
> > +             return NULL;
> > +
> >       if (ped_geometry_read(geom, buff2, sb2off, sectors2))
> >               sb2 = (struct nilfs2_super_block*)buff2;
> > -
> > -     if ((!sb || !is_valid_nilfs_sb(sb)) &&
> > -         (!sb2 || !is_valid_nilfs_sb(sb2)))
> > +     if (!sb2 || !is_valid_nilfs_sb(sb2))
> >               return NULL;
> >
> >       /* reserve 4k bytes for secondary superblock */
> > --
> > 2.39.2
> >
> >
>
> Thanks for the patch! And thanks for looking into this, I hadn't had a
> chance to try to dig deeper.
>
> Could you update the commit message with a bit more background, most
> readers probably don't know what your situation is.
>
> You should also point out that this is a behavior change. Previously it
> would fail only if both locations were invalid. In practice I doubt this
> really matters since this is just a filesystem probe to populate the
> label.
>
> Brian
> --
> Brian C. Lane (PST8PDT) - weldr.io - lorax - parted - pykickstart
>
>