From 4d95848b8c5d2bc400bcf795e5307f01b404dcd4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 14 Feb 2024 20:32:55 -0500 Subject: [PATCH 1/3] rofiles-fuse: Port to statx This allows us to query fsverity efficiently. --- src/rofiles-fuse/main.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/rofiles-fuse/main.c b/src/rofiles-fuse/main.c index 937ee4a6..dc988987 100644 --- a/src/rofiles-fuse/main.c +++ b/src/rofiles-fuse/main.c @@ -235,16 +235,16 @@ callback_link (const char *from, const char *to) * return -EROFS. Otherwise return 0. */ static gboolean -can_write_stbuf (const struct stat *stbuf) +can_write_stbuf (const struct statx *stbuf) { /* If it's not a regular file or symlink, ostree won't hardlink it, so allow * writes - it might be a FIFO or device that somehow * ended up underneath our mount. */ - if (!(S_ISREG (stbuf->st_mode) || S_ISLNK (stbuf->st_mode))) + if (!(S_ISREG (stbuf->stx_mode) || S_ISLNK (stbuf->stx_mode))) return TRUE; /* If the object isn't hardlinked, it's OK to write */ - if (stbuf->st_nlink <= 1) + if (stbuf->stx_nlink <= 1) return TRUE; /* Otherwise, it's a hardlinked file or symlink; it must be * immutable. @@ -276,9 +276,9 @@ gioerror_to_errno (GIOErrorEnum e) } static int -verify_write_or_copyup (const char *path, const struct stat *stbuf, gboolean *out_did_copyup) +verify_write_or_copyup (const char *path, const struct statx *stbuf, gboolean *out_did_copyup) { - struct stat stbuf_local; + struct statx stbuf_local; if (out_did_copyup) *out_did_copyup = FALSE; @@ -286,7 +286,9 @@ verify_write_or_copyup (const char *path, const struct stat *stbuf, gboolean *ou /* If a stbuf wasn't provided, gather it now */ if (!stbuf) { - if (fstatat (basefd, path, &stbuf_local, AT_SYMLINK_NOFOLLOW) == -1) + if (statx (basefd, path, AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT, STATX_BASIC_STATS, + &stbuf_local) + < 0) { if (errno == ENOENT) return 0; @@ -401,7 +403,7 @@ static int do_open (const char *path, mode_t mode, struct fuse_file_info *finfo) { int fd; - struct stat stbuf; + struct statx stbuf; path = ENSURE_RELPATH (path); @@ -421,7 +423,7 @@ do_open (const char *path, mode_t mode, struct fuse_file_info *finfo) if (fd == -1) return -errno; - if (fstat (fd, &stbuf) == -1) + if (statx (fd, "", AT_EMPTY_PATH, STATX_BASIC_STATS, &stbuf) == -1) { (void)close (fd); return -errno; From ed4bd88a3ea2d0cda14e520555917f30cb94d9e1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 14 Feb 2024 20:33:17 -0500 Subject: [PATCH 2/3] rofiles-fuse: Check fsverity flag for copyup We need to do a copyup if fsverity is enabled. Sadly to do this we can't just use ostree_break_hardlink as is. --- ci/gh-install.sh | 1 + ci/installdeps.sh | 2 +- src/rofiles-fuse/main.c | 99 +++++++++++++++++++++----------------- tests/test-rofiles-fuse.sh | 20 +++++++- 4 files changed, 77 insertions(+), 45 deletions(-) diff --git a/ci/gh-install.sh b/ci/gh-install.sh index 966edb28..2260883b 100755 --- a/ci/gh-install.sh +++ b/ci/gh-install.sh @@ -79,6 +79,7 @@ case "$ID" in docbook-xsl e2fslibs-dev elfutils + fsverity fuse gnupg gobject-introspection diff --git a/ci/installdeps.sh b/ci/installdeps.sh index 89b7ac12..f63e2d4e 100755 --- a/ci/installdeps.sh +++ b/ci/installdeps.sh @@ -19,7 +19,7 @@ pkg_install_buildroot pkg_builddep ostree pkg_install sudo which attr fuse strace \ libubsan libasan libtsan redhat-rpm-config \ - elfutils + elfutils fsverity-utils if test -n "${CI_PKGS:-}"; then pkg_install ${CI_PKGS} fi diff --git a/src/rofiles-fuse/main.c b/src/rofiles-fuse/main.c index dc988987..a1dda3aa 100644 --- a/src/rofiles-fuse/main.c +++ b/src/rofiles-fuse/main.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -243,6 +244,11 @@ can_write_stbuf (const struct statx *stbuf) */ if (!(S_ISREG (stbuf->stx_mode) || S_ISLNK (stbuf->stx_mode))) return TRUE; +#ifdef STATX_ATTR_VERITY + /* Can't write to fsverity files */ + if (stbuf->stx_attributes & STATX_ATTR_VERITY) + return FALSE; +#endif /* If the object isn't hardlinked, it's OK to write */ if (stbuf->stx_nlink <= 1) return TRUE; @@ -275,11 +281,48 @@ gioerror_to_errno (GIOErrorEnum e) } } +// The libglnx APIs take a stat buffer, so we need to be able to +// convert from statx. +static inline void +statx_to_stat (const struct statx *stxbuf, struct stat *stbuf) +{ + stbuf->st_dev = makedev (stxbuf->stx_dev_major, stxbuf->stx_dev_minor); + stbuf->st_rdev = makedev (stxbuf->stx_rdev_major, stxbuf->stx_rdev_minor); + stbuf->st_ino = stxbuf->stx_ino; + stbuf->st_mode = stxbuf->stx_mode; + stbuf->st_nlink = stxbuf->stx_nlink; + stbuf->st_uid = stxbuf->stx_uid; + stbuf->st_gid = stxbuf->stx_gid; + stbuf->st_size = stxbuf->stx_size; + stbuf->st_blksize = stxbuf->stx_blksize; +} + +// A copy of ostree_break_hardlink but without the check for hardlinks, which +// is mainly relevant for regular files, where we need to handle verity. +static gboolean +copyup (int dfd, const char *path, const struct statx *stxbuf, GError **error) +{ + if (S_ISREG (stxbuf->stx_mode)) + { + struct stat stbuf; + statx_to_stat (stxbuf, &stbuf); + // Note GLNX_FILE_COPY_OVERWRITE always uses O_TMPFILE+rename + return glnx_file_copy_at (dfd, path, &stbuf, dfd, path, GLNX_FILE_COPY_OVERWRITE, NULL, + error); + } + else + { + // For symlinks, we can just directly call the ostree API. This avoids + // more code duplication because atomically copying symlinks requires + // a temp-link dance. + return ostree_break_hardlink (dfd, path, FALSE, NULL, error); + } +} + static int verify_write_or_copyup (const char *path, const struct statx *stbuf, gboolean *out_did_copyup) { struct statx stbuf_local; - if (out_did_copyup) *out_did_copyup = FALSE; @@ -304,7 +347,7 @@ verify_write_or_copyup (const char *path, const struct statx *stbuf, gboolean *o if (opt_copyup) { g_autoptr (GError) tmp_error = NULL; - if (!ostree_break_hardlink (basefd, path, FALSE, NULL, &tmp_error)) + if (!copyup (basefd, path, stbuf, &tmp_error)) return -gioerror_to_errno ((GIOErrorEnum)tmp_error->code); if (out_did_copyup) *out_did_copyup = TRUE; @@ -417,53 +460,23 @@ do_open (const char *path, mode_t mode, struct fuse_file_info *finfo) else { /* Write */ - - /* We need to specially handle O_TRUNC */ - fd = openat (basefd, path, finfo->flags & ~O_TRUNC, mode); - if (fd == -1) - return -errno; - - if (statx (fd, "", AT_EMPTY_PATH, STATX_BASIC_STATS, &stbuf) == -1) + if (statx (basefd, path, AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT, STATX_BASIC_STATS, &stbuf) + == -1) { - (void)close (fd); - return -errno; - } - - gboolean did_copyup; - int r = verify_write_or_copyup (path, &stbuf, &did_copyup); - if (r != 0) - { - (void)close (fd); - return r; - } - - /* In the copyup case, we need to re-open */ - if (did_copyup) - { - (void)close (fd); - /* Note that unlike the initial open, we will pass through - * O_TRUNC. More ideally in this copyup case we'd avoid copying - * the whole file in the first place, but eh. It's not like we're - * high performance anyways. - */ - fd = openat (basefd, path, finfo->flags & ~(O_EXCL | O_CREAT), mode); - if (fd == -1) + if (errno != ENOENT) return -errno; } else { - /* In the non-copyup case we handle O_TRUNC here, after we've verified - * the hardlink state above with verify_write_or_copyup(). - */ - if (finfo->flags & O_TRUNC) - { - if (ftruncate (fd, 0) == -1) - { - (void)close (fd); - return -errno; - } - } + gboolean did_copyup; + int r = verify_write_or_copyup (path, &stbuf, &did_copyup); + if (r != 0) + return r; } + + fd = openat (basefd, path, finfo->flags, mode); + if (fd == -1) + return -errno; } finfo->fh = fd; diff --git a/tests/test-rofiles-fuse.sh b/tests/test-rofiles-fuse.sh index a56a76c6..0ac55764 100755 --- a/tests/test-rofiles-fuse.sh +++ b/tests/test-rofiles-fuse.sh @@ -26,7 +26,7 @@ skip_without_user_xattrs setup_test_repository "bare" -echo "1..12" +echo "1..13" cd ${test_tmpdir} mkdir mnt @@ -192,3 +192,21 @@ sed -i -e s,first,second, mnt/firstfile assert_file_has_content_literal mnt/firstfile "second" echo "ok copyup" + +copyup_reset +echo nonhardlinked > checkout-test2/nonhardlinked +if fsverity enable checkout-test2/nonhardlinked 2>err.txt; then + orig_inode=$(stat -c %i checkout-test2/nonhardlinked) + echo "updated content" > mnt/nonhardlinked + new_inode=$(stat -c %i checkout-test2/nonhardlinked) + assert_not_streq "${orig_inode}" "${new_inode}" + # And via chmod + fsverity enable checkout-test2/nonhardlinked + orig_inode=$(stat -c %i checkout-test2/nonhardlinked) + chmod 0700 mnt/nonhardlinked + new_inode=$(stat -c %i checkout-test2/nonhardlinked) + assert_not_streq "${orig_inode}" "${new_inode}" + echo "ok copyup fsverity" +else + skip "no fsverity support: $(cat err.txt)" +fi \ No newline at end of file From d0afefcacea32d30b4e4b8d291ea8c7dfe73f1ce Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 15 Feb 2024 08:07:40 -0500 Subject: [PATCH 3/3] rofiles-fuse: Remove unused parameter The logic simplified, so we don't need it anymore. --- src/rofiles-fuse/main.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/rofiles-fuse/main.c b/src/rofiles-fuse/main.c index a1dda3aa..0c267ea9 100644 --- a/src/rofiles-fuse/main.c +++ b/src/rofiles-fuse/main.c @@ -320,11 +320,9 @@ copyup (int dfd, const char *path, const struct statx *stxbuf, GError **error) } static int -verify_write_or_copyup (const char *path, const struct statx *stbuf, gboolean *out_did_copyup) +verify_write_or_copyup (const char *path, const struct statx *stbuf) { struct statx stbuf_local; - if (out_did_copyup) - *out_did_copyup = FALSE; /* If a stbuf wasn't provided, gather it now */ if (!stbuf) @@ -349,8 +347,6 @@ verify_write_or_copyup (const char *path, const struct statx *stbuf, gboolean *o g_autoptr (GError) tmp_error = NULL; if (!copyup (basefd, path, stbuf, &tmp_error)) return -gioerror_to_errno ((GIOErrorEnum)tmp_error->code); - if (out_did_copyup) - *out_did_copyup = TRUE; } else return -EROFS; @@ -367,7 +363,7 @@ verify_write_or_copyup (const char *path, const struct statx *stbuf, gboolean *o do \ { \ path = ENSURE_RELPATH (path); \ - int r = verify_write_or_copyup (path, NULL, NULL); \ + int r = verify_write_or_copyup (path, NULL); \ if (r != 0) \ return r; \ } \ @@ -468,8 +464,7 @@ do_open (const char *path, mode_t mode, struct fuse_file_info *finfo) } else { - gboolean did_copyup; - int r = verify_write_or_copyup (path, &stbuf, &did_copyup); + int r = verify_write_or_copyup (path, &stbuf); if (r != 0) return r; }