Re: [PATCH v3 2/4] ovl: Add framework for verity support

[Eric Biggers <ebiggers@kernel.org>]

On Tue, Jun 13, 2023 at 08:18:50AM +0300, Amir Goldstein wrote:
> On Mon, Jun 12, 2023 at 7:32 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Mon, Jun 12, 2023 at 12:27:17PM +0200, Alexander Larsson wrote:
> > > +fs-verity support
> > > +----------------------
> > > +
> > > +When metadata copy up is used for a file, then the xattr
> > > +"trusted.overlay.verity" may be set on the metacopy file. This
> > > +specifies the expected fs-verity digest of the lowerdata file. This
> > > +may then be used to verify the content of the source file at the time
> > > +the file is opened. During metacopy copy up overlayfs can also set
> > > +this xattr.
> > > +
> > > +This is controlled by the "verity" mount option, which supports
> > > +these values:
> > > +
> > > +- "off":
> > > +    The verity xattr is never used. This is the default if verity
> > > +    option is not specified.
> > > +- "on":
> > > +    Whenever a metacopy files specifies an expected digest, the
> > > +    corresponding data file must match the specified digest.
> > > +    When generating a metacopy file the verity xattr will be set
> > > +    from the source file fs-verity digest (if it has one).
> > > +- "require":
> > > +    Same as "on", but additionally all metacopy files must specify a
> > > +    verity xattr. This means metadata copy up will only be used if
> > > +    the data file has fs-verity enabled, otherwise a full copy-up is
> > > +    used.
> >
> > It looks like my request for improved documentation was not taken, which is
> > unfortunate and makes this patchset difficult to review.
> >
> 
> Which one?
> IIRC, you had two requests.
> One very broad to get the overlayfs.rst document up-to-date:
> [1] https://lore.kernel.org/linux-unionfs/20230514190903.GC9528@sol.localdomain/

That isn't an accurate summary of what I said.  I actually pointed out two
specific things that are confusing specifically in the context of this feature.

> But I assume you mean the specific request about this sentence:
> [2] https://lore.kernel.org/linux-unionfs/20230514192227.GE9528@sol.localdomain/

And that was a third specific thing.  I got a detailed response back
(https://lore.kernel.org/linux-unionfs/CAL7ro1GGAfdZG9cHDWE2vnhY5tSE=9MxYi_n_gJHRfaw7zMSgg@mail.gmail.com),
which was helpful.  Unfortunately, the information in that response hasn't yet
found its way into the documentation that is being proposed.

In general the proposed documentation reads like the audience is overlayfs
developers.  It doesn't describe the motivation for the feature or how to use it
in each of the two use cases.  Maybe that is intended, but it's not what I had
expected to see.

Side note: the use of the passive voice, e.g. "the xattr may be set" and "the
xattr may then be used to verify", should be avoided since it makes it unclear
who/what is doing these actions.

- Eric