On Wed, Jun 21, 2023 at 01:18:28PM +0200, Alexander Larsson wrote:
> During regular metacopy, if lowerdata file has fs-verity enabled, and
> the verity option is enabled, we add the digest to the metacopy xattr.
>
> If verity is required, and lowerdata does not have fs-verity enabled,
> fall back to full copy-up (or the generated metacopy would not
> validate).
>
> Signed-off-by: Alexander Larsson <alexl@xxxxxxxxxx>
> ---
> fs/overlayfs/copy_up.c | 45 ++++++++++++++++++++++++++++++++++++++--
> fs/overlayfs/overlayfs.h | 3 +++
> fs/overlayfs/util.c | 33 ++++++++++++++++++++++++++++-
> 3 files changed, 78 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 68f01fd7f211..fce7d048673c 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -544,6 +544,7 @@ struct ovl_copy_up_ctx {
> bool origin;
> bool indexed;
> bool metacopy;
> + bool metacopy_digest;
> };
>
> static int ovl_link_up(struct ovl_copy_up_ctx *c)
> @@ -641,8 +642,21 @@ static int ovl_copy_up_metadata(struct ovl_copy_up_ctx *c, struct dentry *temp)
> }
>
> if (c->metacopy) {
> - err = ovl_check_setxattr(ofs, temp, OVL_XATTR_METACOPY,
> - NULL, 0, -EOPNOTSUPP);
> + struct path lowerdatapath;
> + struct ovl_metacopy metacopy_data = OVL_METACOPY_INIT;
> +
> + ovl_path_lowerdata(c->dentry, &lowerdatapath);
> + if (WARN_ON_ONCE(lowerdatapath.dentry == NULL))
> + err = -EIO;
> + else
> + err = ovl_set_verity_xattr_from(ofs, &lowerdatapath, &metacopy_data);
There's no dedicated verity xattr anymore, so maybe ovl_set_verity_xattr_from()
should be renamed to something like ovl_get_verity_digest().
> +
> + if (metacopy_data.digest_algo)
> + c->metacopy_digest = true;
> +
> + if (!err)
> + err = ovl_set_metacopy_xattr(ofs, temp, &metacopy_data);
> +
> if (err)
> return err;
The error handling above is a bit weird. Some early returns would make it
easier to read:
ovl_path_lowerdata(c->dentry, &lowerdatapath);
if (WARN_ON_ONCE(lowerdatapath.dentry == NULL))
return -EIO;
err = ovl_get_verity_digest(ofs, &lowerdatapath, &metacopy_data);
if (err)
return err;
if (metacopy_data.digest_algo)
c->metacopy_digest = true;
err = ovl_set_metacopy_xattr(ofs, temp, &metacopy_data);
if (err)
return err;
> }
> @@ -751,6 +765,12 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
> if (err)
> goto cleanup;
>
> + if (c->metacopy_digest)
> + ovl_set_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
> + else
> + ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
> + ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry));
> +
> if (!c->metacopy)
> ovl_set_upperdata(d_inode(c->dentry));
> inode = d_inode(c->dentry);
Maybe the line 'inode = d_inode(c->dentry);' should be moved earlier, and then
'inode' used instead of 'd_inode(c->dentry)' later on.
> + if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
> + struct path lowerdata;
> +
> + ovl_path_lowerdata(dentry, &lowerdata);
> +
> + if (WARN_ON_ONCE(lowerdata.dentry == NULL) ||
> + ovl_ensure_verity_loaded(&lowerdata) ||
> + !fsverity_get_info(d_inode(lowerdata.dentry))) {
> + return false;
Please use !fsverity_active() instead of !fsverity_get_info().
- Eric
