Re: [PATCH v2 5/6] ovl: Validate verity xattr when resolving lowerdata

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

 



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.  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.

- Eric



[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux