On Fri, Jun 16, 2023 at 8:24 AM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
>
> On Fri, Jun 16, 2023 at 08:07:43AM +0300, Amir Goldstein wrote:
> > > I would really like to get the overlay.verity xattr support in
> > > upstream pretty soon, because without it I can't release a stable
> > > version of the composefs userspace code. I don't want to release
> > > something that is using an xattr format that is not guaranteed to be
> > > stable.
> > >
> >
> > Alex,
> >
> > Pondering about this last sentence.
> >
> > The overlay.verity xattr format is already in its 3rd revision since
> > the beginning of development.
> >
> > When it was bare digest, it might have made sense to have no
> > header that describes the format.
> >
> > When the algo byte was added, that was already a very big hint
> > that a proper header was in order.
> >
> > Now that you had to change the meaning of the byte, it is very hard
> > to argue that the xattr format is guaranteed to be stable - in the sense
> > that it can never change again.
> >
> > Please add a minimal header to the overlay.verity xattr format,
> > similar to ovl_fb, that will allow composefs/overlayfs to be
> > maintained as the separate projects that they are.
> >
> > Something like this?
> >
> > /* On-disk format for "verity" xattr */
> > struct ovl_verity {
> > u8 version; /* 0 */
> > u8 len; /* size of this header + size of digest */
> > u8 flags;
> > u8 algo; /* digest algo */
> > u8 digest[];
> > };
> >
> > I realize that digest size may be inferred from xattr size,
> > but it is better to state the stored digest size explicitly and verify
> > that it matches expected xattr size.
>
> If it was up to me I think I'd keep it even more "minimal" just do:
>
> struct ovl_verity {
> u8 version; /* 0 */
> u8 algo; /* digest algo */
> u8 digest[];
> };
>
> It's up to you, though.
Please keep len and flags.
Nothing to be gained from removing them.
> Keep in mind, the definition of a fsverity digest as
> algorithm ID + raw digest is pretty fundamental to fsverity. It's not
> especially prone to change. The confusion we had was just over what type of
> algorithm ID to use.
>
> (There is some inconsistency about whether the algorithm ID is u8, u16, or u32.
> However, it's u8 on-disk in the fsverity_descriptor. So it's fine for the
> overlayfs xattr to use u8.)
>
> >
> > Does the digest buffer passed to fsnotify helpers have any
> > memory alignment requirements?
>
> I think you're asking about the raw_digest argument to fsverity_get_digest()?
> No, there's no alignment requirement.
>
Yes, that is what I meant.
Thanks,
Amir.
