| Message ID | 8771725be2a8b7d65ea6c50a69bb6392b9e903aa.1687345663.git.alexl@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ovl: Add support for fs-verity checking of lowerdata | expand |
On Wed, Jun 21, 2023 at 2:18 PM Alexander Larsson <alexl@redhat.com> 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@redhat.com> Looks good. Reviewed-by: Amir Goldstein <amir73il@gmail.com> > --- > 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); > + > + if (metacopy_data.digest_algo) > + c->metacopy_digest = true; > + > + if (!err) > + 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); > @@ -813,6 +833,12 @@ static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c) > if (err) > goto out_fput; > > + 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)); > ovl_inode_update(d_inode(c->dentry), dget(temp)); > @@ -918,6 +944,19 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode, > if (flags && ((OPEN_FMODE(flags) & FMODE_WRITE) || (flags & O_TRUNC))) > return false; > > + /* Fall back to full copy if no fsverity on source data and we require verity */ > + 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; > + } > + } > + > return true; > } > > @@ -984,6 +1023,8 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c) > if (err) > goto out_free; > > + ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry)); > + ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry)); > ovl_set_upperdata(d_inode(c->dentry)); > out_free: > kfree(capability); > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h > index 9fbbc077643b..e728360c66ff 100644 > --- a/fs/overlayfs/overlayfs.h > +++ b/fs/overlayfs/overlayfs.h > @@ -519,11 +519,14 @@ int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d, > struct ovl_metacopy *metacopy); > bool ovl_is_metacopy_dentry(struct dentry *dentry); > char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int padding); > +int ovl_ensure_verity_loaded(struct path *path); > int ovl_get_verity_xattr(struct ovl_fs *ofs, const struct path *path, > u8 *digest_buf, int *buf_length); > int ovl_validate_verity(struct ovl_fs *ofs, > struct path *metapath, > struct path *datapath); > +int ovl_set_verity_xattr_from(struct ovl_fs *ofs, struct path *src, > + struct ovl_metacopy *metacopy); > int ovl_sync_status(struct ovl_fs *ofs); > > static inline void ovl_set_flag(unsigned long flag, struct inode *inode) > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c > index 927a1133859d..439e23496713 100644 > --- a/fs/overlayfs/util.c > +++ b/fs/overlayfs/util.c > @@ -1188,7 +1188,7 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int pa > } > > /* Call with mounter creds as it may open the file */ > -static int ovl_ensure_verity_loaded(struct path *datapath) > +int ovl_ensure_verity_loaded(struct path *datapath) > { > struct inode *inode = d_inode(datapath->dentry); > const struct fsverity_info *vi; > @@ -1264,6 +1264,37 @@ int ovl_validate_verity(struct ovl_fs *ofs, > return 0; > } > > +int ovl_set_verity_xattr_from(struct ovl_fs *ofs, struct path *src, > + struct ovl_metacopy *metacopy) > +{ > + int err, digest_size; > + > + if (!ofs->config.verity_mode || !S_ISREG(d_inode(src->dentry)->i_mode)) > + return 0; > + > + err = ovl_ensure_verity_loaded(src); > + if (err < 0) { > + pr_warn_ratelimited("lower file '%pd' failed to load fs-verity info\n", > + src->dentry); > + return -EIO; > + } > + > + digest_size = fsverity_get_digest(d_inode(src->dentry), > + metacopy->digest, &metacopy->digest_algo, NULL); > + if (digest_size == 0 || > + WARN_ON_ONCE(digest_size > FS_VERITY_MAX_DIGEST_SIZE)) { > + if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) { > + pr_warn_ratelimited("lower file '%pd' has no fs-verity digest\n", > + src->dentry); > + return -EIO; > + } > + return 0; > + } > + > + metacopy->len += digest_size; > + return 0; > +} > + > /* > * ovl_sync_status() - Check fs sync status for volatile mounts > * > -- > 2.40.1 >
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@redhat.com> > --- > 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
On Mon, Jul 3, 2023 at 9:30 PM Eric Biggers <ebiggers@kernel.org> wrote: > > 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@redhat.com> > > --- > > 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(). Yeah, that makes a lot of sense. > > + > > + 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; Yeah. > > } > > @@ -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. I wonder why this wasn't already done for the ovl_set_upperdata(), but it seems ok so I did this. > > + 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(). Done. All these changes are in git: https://github.com/alexlarsson/linux/commits/overlay-verity
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); + + if (metacopy_data.digest_algo) + c->metacopy_digest = true; + + if (!err) + 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); @@ -813,6 +833,12 @@ static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c) if (err) goto out_fput; + 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)); ovl_inode_update(d_inode(c->dentry), dget(temp)); @@ -918,6 +944,19 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode, if (flags && ((OPEN_FMODE(flags) & FMODE_WRITE) || (flags & O_TRUNC))) return false; + /* Fall back to full copy if no fsverity on source data and we require verity */ + 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; + } + } + return true; } @@ -984,6 +1023,8 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c) if (err) goto out_free; + ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry)); + ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry)); ovl_set_upperdata(d_inode(c->dentry)); out_free: kfree(capability); diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 9fbbc077643b..e728360c66ff 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -519,11 +519,14 @@ int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d, struct ovl_metacopy *metacopy); bool ovl_is_metacopy_dentry(struct dentry *dentry); char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int padding); +int ovl_ensure_verity_loaded(struct path *path); int ovl_get_verity_xattr(struct ovl_fs *ofs, const struct path *path, u8 *digest_buf, int *buf_length); int ovl_validate_verity(struct ovl_fs *ofs, struct path *metapath, struct path *datapath); +int ovl_set_verity_xattr_from(struct ovl_fs *ofs, struct path *src, + struct ovl_metacopy *metacopy); int ovl_sync_status(struct ovl_fs *ofs); static inline void ovl_set_flag(unsigned long flag, struct inode *inode) diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 927a1133859d..439e23496713 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -1188,7 +1188,7 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int pa } /* Call with mounter creds as it may open the file */ -static int ovl_ensure_verity_loaded(struct path *datapath) +int ovl_ensure_verity_loaded(struct path *datapath) { struct inode *inode = d_inode(datapath->dentry); const struct fsverity_info *vi; @@ -1264,6 +1264,37 @@ int ovl_validate_verity(struct ovl_fs *ofs, return 0; } +int ovl_set_verity_xattr_from(struct ovl_fs *ofs, struct path *src, + struct ovl_metacopy *metacopy) +{ + int err, digest_size; + + if (!ofs->config.verity_mode || !S_ISREG(d_inode(src->dentry)->i_mode)) + return 0; + + err = ovl_ensure_verity_loaded(src); + if (err < 0) { + pr_warn_ratelimited("lower file '%pd' failed to load fs-verity info\n", + src->dentry); + return -EIO; + } + + digest_size = fsverity_get_digest(d_inode(src->dentry), + metacopy->digest, &metacopy->digest_algo, NULL); + if (digest_size == 0 || + WARN_ON_ONCE(digest_size > FS_VERITY_MAX_DIGEST_SIZE)) { + if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) { + pr_warn_ratelimited("lower file '%pd' has no fs-verity digest\n", + src->dentry); + return -EIO; + } + return 0; + } + + metacopy->len += digest_size; + return 0; +} + /* * ovl_sync_status() - Check fs sync status for volatile mounts *
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@redhat.com> --- fs/overlayfs/copy_up.c | 45 ++++++++++++++++++++++++++++++++++++++-- fs/overlayfs/overlayfs.h | 3 +++ fs/overlayfs/util.c | 33 ++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 3 deletions(-)