[RFC][PATCH 0/5] shmem/fsverity: Prepare for mandatory integrity enforcement

[RFC][PATCH 0/5] shmem/fsverity: Prepare for mandatory integrity enforcement

[Roberto Sassu]

The biggest advantage of fsverity compared to file-based solutions like IMA
is the lower overhead added for integrity measurement and enforcement.
Although fsverity offers the same file representation as a digest, it works
instead at page granularity rather than file granularity. Only the pages
that are going to be used will be evaluated by fsverity, while IMA has to
read the whole file.

Although fsverity has a built-in enforcement mechanism, it is basically
discretionary, it can be enabled (or disabled by replacing the file) by
the user himself. A mandatory mechanism could be used to impose fsverity
protection for files involved in certain events, defined with a policy.

Integrity Policy Enforcement (IPE) is one of such mandatory mechanisms
designed to work in conjunction with fsverity (IMA integration is planned).
However, at this stage (v7), IPE is storing fsverity data at file
instantiation time rather than at usage time. One disadvantage of such
approach is that a security blob needs to be allocated and maintained for
such purpose, which makes the code unnecessarily more complex. A much
better approach would be to retrieve the information when needed, without
storing it in a security blob.

Another gap that currently limits the applicability of fsverity is the
missing support for the tmpfs filesystem. Files could still be executed
from it (e.g. during the boot process) and would still need to be verified
by IPE. Not verifying those files would considerably reduce the
effectiveness of the integrity protection. Alternatively, requiring that
the initial ram disk is signed limits the applicability of the solution
only to embedded systems, where the initial ram disk can be also
distributed. General purpose OSes instead regenerate it locally depending
on the system configuration, and after critical packages changes.

Given that tmpfs is not persistent, the question is if support should be
implemented in a similar way of persistent filesystems, such as ext4. And,
while fsverity protection needs to be enabled on a file each time a tmpfs
filesystem is mounted, probably implementing fsverity support in the same
way is simpler.

Another question is whether and when pages should be checked. In a
preliminary test, checking a page after it was swapped in resulted in the
page not being considered valid by fsverity. For now there are no calls to
fsverity_verify_page().

The implementation in this patch set is not necessarily the most efficient,
and does not consider the specific features of tmpfs. The goal was to aim
at correctness, by following closely the implementation for f2fs, doing the
minimal changes necessary to make it work for tmpfs (e.g. replacing
read_mapping_page() with shmem_read_mapping_page()).

Other than with trivial operations, this patch set has been tested with
the xfstests-dev testsuite, also under severe memory pressure to ensure
that everything still works when a page is swapped out.

Make the following changes: provide the fsverity_get_file_digest() and
fsverity_sig_validated() for IPE to retrieve fsverity information at usage
time, and additionally revalidate the signature at every file open
(patches 1, 2); make fsverity suitable to protect files in the rootfs
filesystem (patch 3); fix a small problem in shmem_read_mapping_page_gfp()
(patch 4) and finally add support for fsverity in tmpfs (patch 5).

An open point, not addressed by this patch set, is how to enable the
fsverity protection for files in the rootfs filesystem, given that it is
too early for user space to invoke the ioctl() system call. It should not
be a problem to enable fsverity from the kernel, depending on a labelling
policy.

Roberto Sassu (5):
  fsverity: Introduce fsverity_get_file_digest()
  fsverity: Revalidate built-in signatures at file open
  fsverity: Do initialization earlier
  shmem: Avoid segfault in shmem_read_mapping_page_gfp()
  shmem: Add fsverity support

 Documentation/filesystems/fsverity.rst |  18 ++
 MAINTAINERS                            |   1 +
 fs/Kconfig                             |   7 +
 fs/verity/enable.c                     |   6 +-
 fs/verity/fsverity_private.h           |   7 +-
 fs/verity/init.c                       |   2 +-
 fs/verity/open.c                       |  43 +++-
 fs/verity/signature.c                  |   6 +-
 include/linux/fsverity.h               |  16 ++
 include/linux/shmem_fs.h               |  27 +++
 mm/Makefile                            |   2 +
 mm/shmem.c                             |  71 ++++++-
 mm/shmem_verity.c                      | 267 +++++++++++++++++++++++++
 13 files changed, 463 insertions(+), 10 deletions(-)
 create mode 100644 mm/shmem_verity.c

-- 
2.32.0

[RFC][PATCH 1/5] fsverity: Introduce fsverity_get_file_digest()

[Roberto Sassu]

Since the fsverity_info structure is defined internally in fsverity, expose
the fsverity file digest through the new function
fsverity_get_file_digest().

Given that an fsverity file is guaranteed to be immutable, also the
retrieved file digest is stable and won't change.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/verity/open.c         | 24 ++++++++++++++++++++++++
 include/linux/fsverity.h | 10 ++++++++++
 2 files changed, 34 insertions(+)

diff --git a/fs/verity/open.c b/fs/verity/open.c
index 92df87f5fa38..9127c77c6539 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -218,6 +218,30 @@ void fsverity_free_info(struct fsverity_info *vi)
 	kmem_cache_free(fsverity_info_cachep, vi);
 }
 
+/*
+ * Copy the file digest and associated algorithm taken from the passed
+ * fsverity_info structure to the locations supplied by the caller.
+ *
+ * Return: the digest size on success, a negative value on error
+ */
+ssize_t fsverity_get_file_digest(struct fsverity_info *info, u8 *buf,
+				 size_t bufsize, enum hash_algo *algo)
+{
+	enum hash_algo a;
+
+	a = match_string(hash_algo_name, HASH_ALGO__LAST,
+			 info->tree_params.hash_alg->name);
+	if (a < 0)
+		return a;
+
+	if (bufsize < hash_digest_size[a])
+		return -ERANGE;
+
+	*algo = a;
+	memcpy(buf, info->file_digest, hash_digest_size[*algo]);
+	return hash_digest_size[*algo];
+}
+
 static bool validate_fsverity_descriptor(struct inode *inode,
 					 const struct fsverity_descriptor *desc,
 					 size_t desc_size)
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index b568b3c7d095..877a7f609dd9 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -13,6 +13,7 @@
 
 #include <linux/fs.h>
 #include <uapi/linux/fsverity.h>
+#include <crypto/hash_info.h>
 
 /* Verity operations for filesystems */
 struct fsverity_operations {
@@ -137,6 +138,8 @@ int fsverity_ioctl_measure(struct file *filp, void __user *arg);
 int fsverity_file_open(struct inode *inode, struct file *filp);
 int fsverity_prepare_setattr(struct dentry *dentry, struct iattr *attr);
 void fsverity_cleanup_inode(struct inode *inode);
+ssize_t fsverity_get_file_digest(struct fsverity_info *info, u8 *buf,
+				 size_t bufsize, enum hash_algo *algo);
 
 /* read_metadata.c */
 
@@ -187,6 +190,13 @@ static inline void fsverity_cleanup_inode(struct inode *inode)
 {
 }
 
+static inline ssize_t fsverity_get_file_digest(struct fsverity_info *info,
+					       u8 *buf, size_t bufsize,
+					       enum hash_algo *algo)
+{
+	return -EOPNOTSUPP;
+}
+
 /* read_metadata.c */
 
 static inline int fsverity_ioctl_read_metadata(struct file *filp,
-- 
2.32.0

[RFC][PATCH 2/5] fsverity: Revalidate built-in signatures at file open

[Roberto Sassu]

Fsverity signatures are validated only upon request by the user by setting
the requirement through procfs or sysctl.

However, signatures are validated only when the fsverity-related
initialization is performed on the file. If the initialization happened
while the signature requirement was disabled, the signature is not
validated again.

Keep track in the fsverity_info structure if the signature was validated
and, based on that and on the signature requirement, perform signature
validation at every call of fsverity_file_open() (the behavior remains the
same if the requirement is not set).

Finally, expose the information of whether the signature was validated
through the new function fsverity_sig_validated(). It could be used for
example by IPE to enforce the signature requirement in a mandatory way (the
procfs/sysctl methods are discretionary).

NOTE: revalidation is not performed if the keys in the fs-verity keyring
changed; this would probably require a more sophisticated mechanism such as
one based on sequence numbers.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/verity/fsverity_private.h |  7 +++++--
 fs/verity/open.c             | 19 ++++++++++++++++++-
 fs/verity/signature.c        |  6 ++++--
 include/linux/fsverity.h     |  6 ++++++
 4 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
index a7920434bae5..bcd5c0587e42 100644
--- a/fs/verity/fsverity_private.h
+++ b/fs/verity/fsverity_private.h
@@ -75,6 +75,7 @@ struct fsverity_info {
 	u8 root_hash[FS_VERITY_MAX_DIGEST_SIZE];
 	u8 file_digest[FS_VERITY_MAX_DIGEST_SIZE];
 	const struct inode *inode;
+	bool sig_validated;
 };
 
 /* Arbitrary limit to bound the kmalloc() size.  Can be changed. */
@@ -138,14 +139,16 @@ void __init fsverity_exit_info_cache(void);
 
 /* signature.c */
 
+extern int fsverity_require_signatures;
+
 #ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
-int fsverity_verify_signature(const struct fsverity_info *vi,
+int fsverity_verify_signature(struct fsverity_info *vi,
 			      const u8 *signature, size_t sig_size);
 
 int __init fsverity_init_signature(void);
 #else /* !CONFIG_FS_VERITY_BUILTIN_SIGNATURES */
 static inline int
-fsverity_verify_signature(const struct fsverity_info *vi,
+fsverity_verify_signature(struct fsverity_info *vi,
 			  const u8 *signature, size_t sig_size)
 {
 	return 0;
diff --git a/fs/verity/open.c b/fs/verity/open.c
index 9127c77c6539..22c6644b0282 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -242,6 +242,17 @@ ssize_t fsverity_get_file_digest(struct fsverity_info *info, u8 *buf,
 	return hash_digest_size[*algo];
 }
 
+/*
+ * Provide the information of whether the fsverity built-in signature was
+ * validated.
+ *
+ * Return: true if the signature was validated, false if not
+ */
+bool fsverity_sig_validated(struct fsverity_info *info)
+{
+	return info->sig_validated;
+}
+
 static bool validate_fsverity_descriptor(struct inode *inode,
 					 const struct fsverity_descriptor *desc,
 					 size_t desc_size)
@@ -333,13 +344,19 @@ static int ensure_verity_info(struct inode *inode)
 	size_t desc_size;
 	int err;
 
-	if (vi)
+	if (vi && (!fsverity_require_signatures || vi->sig_validated))
 		return 0;
 
 	err = fsverity_get_descriptor(inode, &desc, &desc_size);
 	if (err)
 		return err;
 
+	if (vi) {
+		err = fsverity_verify_signature(vi, desc->signature,
+						le32_to_cpu(desc->sig_size));
+		goto out_free_desc;
+	}
+
 	vi = fsverity_create_info(inode, desc, desc_size);
 	if (IS_ERR(vi)) {
 		err = PTR_ERR(vi);
diff --git a/fs/verity/signature.c b/fs/verity/signature.c
index 143a530a8008..dbe6b3b0431c 100644
--- a/fs/verity/signature.c
+++ b/fs/verity/signature.c
@@ -16,7 +16,7 @@
  * /proc/sys/fs/verity/require_signatures
  * If 1, all verity files must have a valid builtin signature.
  */
-static int fsverity_require_signatures;
+int fsverity_require_signatures;
 
 /*
  * Keyring that contains the trusted X.509 certificates.
@@ -37,7 +37,7 @@ static struct key *fsverity_keyring;
  *
  * Return: 0 on success (signature valid or not required); -errno on failure
  */
-int fsverity_verify_signature(const struct fsverity_info *vi,
+int fsverity_verify_signature(struct fsverity_info *vi,
 			      const u8 *signature, size_t sig_size)
 {
 	const struct inode *inode = vi->inode;
@@ -82,6 +82,8 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
 		return err;
 	}
 
+	vi->sig_validated = true;
+
 	pr_debug("Valid signature for file digest %s:%*phN\n",
 		 hash_alg->name, hash_alg->digest_size, vi->file_digest);
 	return 0;
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index 877a7f609dd9..85e52333d1b8 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -140,6 +140,7 @@ int fsverity_prepare_setattr(struct dentry *dentry, struct iattr *attr);
 void fsverity_cleanup_inode(struct inode *inode);
 ssize_t fsverity_get_file_digest(struct fsverity_info *info, u8 *buf,
 				 size_t bufsize, enum hash_algo *algo);
+bool fsverity_sig_validated(struct fsverity_info *info);
 
 /* read_metadata.c */
 
@@ -197,6 +198,11 @@ static inline ssize_t fsverity_get_file_digest(struct fsverity_info *info,
 	return -EOPNOTSUPP;
 }
 
+static inline bool fsverity_sig_validated(struct fsverity_info *info)
+{
+	return false;
+}
+
 /* read_metadata.c */
 
 static inline int fsverity_ioctl_read_metadata(struct file *filp,
-- 
2.32.0

[RFC][PATCH 3/5] fsverity: Do initialization earlier

[Roberto Sassu]

Perform fsverity initialization with core_initcall(), to ensure that
fsverity is available also very early during the boot process.

More specifically, allow files in the rootfs filesystem (from an initial
ram disk) to be protected with fsverity and be checked with LSMs such as
IPE.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/verity/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/verity/init.c b/fs/verity/init.c
index c98b7016f446..910083919e1d 100644
--- a/fs/verity/init.c
+++ b/fs/verity/init.c
@@ -58,4 +58,4 @@ static int __init fsverity_init(void)
 	fsverity_exit_info_cache();
 	return err;
 }
-late_initcall(fsverity_init)
+core_initcall(fsverity_init)
-- 
2.32.0

[RFC][PATCH 4/5] shmem: Avoid segfault in shmem_read_mapping_page_gfp()

[Roberto Sassu]

Check the hwpoison page flag only if the page is valid in
shmem_read_mapping_page_gfp(). The PageHWPoison() macro tries to access
the page flags and cannot work on an error pointer.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 mm/shmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 23c91a8beb78..427863cbf0dc 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -4222,7 +4222,7 @@ struct page *shmem_read_mapping_page_gfp(struct address_space *mapping,
 	else
 		unlock_page(page);
 
-	if (PageHWPoison(page))
+	if (!IS_ERR(page) && PageHWPoison(page))
 		page = ERR_PTR(-EIO);
 
 	return page;
-- 
2.32.0

[RFC][PATCH 5/5] shmem: Add fsverity support

[Roberto Sassu]

Make the necessary modifications to support fsverity in tmpfs.

First, implement the fsverity operations (in a similar way of f2fs). These
operations make use of shmem_read_mapping_page() instead of
read_mapping_page() to handle the case where the page has been swapped out.
The fsverity descriptor is placed at the end of the file and its location
is stored in an xattr.

Second, implement the ioctl operations to enable, measure and read fsverity
metadata.

Lastly, add calls to fsverity functions, to ensure that fsverity-relevant
operations are checked and handled by fsverity (file open, attr set, inode
evict).

Fsverity support can be enabled through the kernel configuration and
remains enabled by default for every tmpfs filesystem instantiated (there
should be no overhead, unless fsverity is enabled for a file).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/filesystems/fsverity.rst |  18 ++
 MAINTAINERS                            |   1 +
 fs/Kconfig                             |   7 +
 fs/verity/enable.c                     |   6 +-
 include/linux/shmem_fs.h               |  27 +++
 mm/Makefile                            |   2 +
 mm/shmem.c                             |  69 ++++++-
 mm/shmem_verity.c                      | 267 +++++++++++++++++++++++++
 8 files changed, 394 insertions(+), 3 deletions(-)
 create mode 100644 mm/shmem_verity.c

diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index 1d831e3cbcb3..71186cebf15d 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -533,6 +533,24 @@ Currently, f2fs verity only supports a Merkle tree block size of 4096.
 Also, f2fs doesn't support enabling verity on files that currently
 have atomic or volatile writes pending.
 
+tmpfs
+-----
+
+tmpfs supports fsverity since Linux v5.17.
+
+Fsverity support for tmpfs can be enabled at build time through the kernel
+configuration option ``CONFIG_TMPFS_VERITY``. If enabled, it is also
+automatically enabled at mount time for every tmpfs filesystem
+instantiated.
+
+Like f2fs, tmpfs stores the verity metadata (Merkle tree and
+fsverity_descriptor) past the end of the file, starting at the first
+64K boundary beyond i_size. Also, like f2fs, it stores the fsverity
+descriptor location in an xattr.
+
+Currently, tmpfs verity only supports the case where the Merkle tree
+block size and page size are the same.
+
 Implementation details
 ======================
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 9096c64d8d09..118cf9d58601 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19137,6 +19137,7 @@ L:	linux-mm@kvack.org
 S:	Maintained
 F:	include/linux/shmem_fs.h
 F:	mm/shmem.c
+F:	mm/shmem_verity.c
 
 TOMOYO SECURITY MODULE
 M:	Kentaro Takeda <takedakn@nttdata.co.jp>
diff --git a/fs/Kconfig b/fs/Kconfig
index a6313a969bc5..d67f7a1cdcb6 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -229,6 +229,13 @@ config TMPFS_INODE64
 
 	  If unsure, say N.
 
+config TMPFS_VERITY
+	bool "fsverity support for tmpfs (EXPERIMENTAL)"
+	depends on FS_VERITY && TMPFS && TMPFS_XATTR
+
+	help
+	  Enable fsverity protection for files in the tmpfs filesystem.
+
 config ARCH_SUPPORTS_HUGETLBFS
 	def_bool n
 
diff --git a/fs/verity/enable.c b/fs/verity/enable.c
index 60a4372aa4d7..9cd64cbe3579 100644
--- a/fs/verity/enable.c
+++ b/fs/verity/enable.c
@@ -13,6 +13,7 @@
 #include <linux/pagemap.h>
 #include <linux/sched/signal.h>
 #include <linux/uaccess.h>
+#include <linux/shmem_fs.h>
 
 /*
  * Read a file data page for Merkle tree construction.  Do aggressive readahead,
@@ -31,7 +32,10 @@ static struct page *read_file_data_page(struct file *filp, pgoff_t index,
 		else
 			page_cache_sync_readahead(filp->f_mapping, ra, filp,
 						  index, remaining_pages);
-		page = read_mapping_page(filp->f_mapping, index, NULL);
+		if (shmem_file(filp))
+			page = shmem_read_mapping_page(filp->f_mapping, index);
+		else
+			page = read_mapping_page(filp->f_mapping, index, NULL);
 		if (IS_ERR(page))
 			return page;
 	}
diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
index 166158b6e917..07b9a142c7d3 100644
--- a/include/linux/shmem_fs.h
+++ b/include/linux/shmem_fs.h
@@ -10,6 +10,9 @@
 #include <linux/xattr.h>
 #include <linux/fs_parser.h>
 
+#define SHMEM_VERITY_IN_PROGRESS 0x00000001
+#define SHMEM_XATTR_NAME_VERITY "v"
+
 /* inode in-kernel data */
 
 struct shmem_inode_info {
@@ -44,6 +47,7 @@ struct shmem_sb_info {
 	spinlock_t shrinklist_lock;   /* Protects shrinklist */
 	struct list_head shrinklist;  /* List of shinkable inodes */
 	unsigned long shrinklist_len; /* Length of shrinklist */
+	bool verity;		      /* Fsverity enabled or not */
 };
 
 static inline struct shmem_inode_info *SHMEM_I(struct inode *inode)
@@ -51,10 +55,33 @@ static inline struct shmem_inode_info *SHMEM_I(struct inode *inode)
 	return container_of(inode, struct shmem_inode_info, vfs_inode);
 }
 
+static inline bool shmem_verity_in_progress(struct inode *inode)
+{
+	struct shmem_inode_info *info = SHMEM_I(inode);
+
+	return IS_ENABLED(CONFIG_FS_VERITY) &&
+	       (info->flags & SHMEM_VERITY_IN_PROGRESS);
+}
+
+static inline void shmem_verity_set_in_progress(struct inode *inode)
+{
+	struct shmem_inode_info *info = SHMEM_I(inode);
+
+	info->flags |= SHMEM_VERITY_IN_PROGRESS;
+}
+
+static inline void shmem_verity_clear_in_progress(struct inode *inode)
+{
+	struct shmem_inode_info *info = SHMEM_I(inode);
+
+	info->flags &= ~SHMEM_VERITY_IN_PROGRESS;
+}
+
 /*
  * Functions in mm/shmem.c called directly from elsewhere:
  */
 extern const struct fs_parameter_spec shmem_fs_parameters[];
+extern const struct fsverity_operations shmem_verityops;
 extern int shmem_init(void);
 extern int shmem_init_fs_context(struct fs_context *fc);
 extern struct file *shmem_file_setup(const char *name,
diff --git a/mm/Makefile b/mm/Makefile
index d6c0042e3aa0..f15b48dbd235 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -54,6 +54,8 @@ obj-y			:= filemap.o mempool.o oom_kill.o fadvise.o \
 			   interval_tree.o list_lru.o workingset.o \
 			   debug.o gup.o mmap_lock.o $(mmu-y)
 
+obj-$(CONFIG_TMPFS_VERITY)	+= shmem_verity.o
+
 # Give 'page_alloc' its own module-parameter namespace
 page-alloc-y := page_alloc.o
 page-alloc-$(CONFIG_SHUFFLE_PAGE_ALLOCATOR) += shuffle.o
diff --git a/mm/shmem.c b/mm/shmem.c
index 427863cbf0dc..f36e1a493610 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -78,6 +78,7 @@ static struct vfsmount *shm_mnt;
 #include <linux/userfaultfd_k.h>
 #include <linux/rmap.h>
 #include <linux/uuid.h>
+#include <linux/fsverity.h>
 
 #include <linux/uaccess.h>
 
@@ -1089,6 +1090,10 @@ static int shmem_setattr(struct user_namespace *mnt_userns,
 	if (error)
 		return error;
 
+	error = fsverity_prepare_setattr(dentry, attr);
+	if (error)
+		return error;
+
 	if (S_ISREG(inode->i_mode) && (attr->ia_valid & ATTR_SIZE)) {
 		loff_t oldsize = inode->i_size;
 		loff_t newsize = attr->ia_size;
@@ -1156,6 +1161,7 @@ static void shmem_evict_inode(struct inode *inode)
 		}
 	}
 
+	fsverity_cleanup_inode(inode);
 	simple_xattrs_free(&info->xattrs);
 	WARN_ON(inode->i_blocks);
 	shmem_free_inode(inode->i_sb);
@@ -1827,7 +1833,8 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
 		return -EFBIG;
 repeat:
 	if (sgp <= SGP_CACHE &&
-	    ((loff_t)index << PAGE_SHIFT) >= i_size_read(inode)) {
+	    ((loff_t)index << PAGE_SHIFT) >= i_size_read(inode) &&
+	    !fsverity_active(inode) && !shmem_verity_in_progress(inode)) {
 		return -EINVAL;
 	}
 
@@ -2485,7 +2492,7 @@ shmem_write_end(struct file *file, struct address_space *mapping,
 {
 	struct inode *inode = mapping->host;
 
-	if (pos + copied > inode->i_size)
+	if (pos + copied > inode->i_size && !shmem_verity_in_progress(inode))
 		i_size_write(inode, pos + copied);
 
 	if (!PageUptodate(page)) {
@@ -2805,6 +2812,56 @@ static long shmem_fallocate(struct file *file, int mode, loff_t offset,
 	return error;
 }
 
+static bool shmem_sb_has_verity(struct inode *inode)
+{
+	return SHMEM_SB(inode->i_sb)->verity;
+}
+
+static int shmem_ioc_enable_verity(struct file *filp, unsigned long arg)
+{
+	if (!shmem_sb_has_verity(file_inode(filp)))
+		return -EOPNOTSUPP;
+
+	return fsverity_ioctl_enable(filp, (const void __user *)arg);
+}
+
+static int shmem_ioc_measure_verity(struct file *filp, unsigned long arg)
+{
+	if (!shmem_sb_has_verity(file_inode(filp)))
+		return -EOPNOTSUPP;
+
+	return fsverity_ioctl_measure(filp, (void __user *)arg);
+}
+
+static int shmem_ioc_read_verity_metadata(struct file *filp, unsigned long arg)
+{
+	if (!shmem_sb_has_verity(file_inode(filp)))
+		return -EOPNOTSUPP;
+
+	return fsverity_ioctl_read_metadata(filp, (const void __user *)arg);
+}
+
+static long shmem_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	switch (cmd) {
+	case FS_IOC_ENABLE_VERITY:
+		return shmem_ioc_enable_verity(filp, arg);
+	case FS_IOC_MEASURE_VERITY:
+		return shmem_ioc_measure_verity(filp, arg);
+	case FS_IOC_READ_VERITY_METADATA:
+		return shmem_ioc_read_verity_metadata(filp, arg);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+#ifdef CONFIG_TMPFS_VERITY
+static int shmem_file_open(struct inode *inode, struct file *filp)
+{
+	return fsverity_file_open(inode, filp);
+}
+#endif
+
 static int shmem_statfs(struct dentry *dentry, struct kstatfs *buf)
 {
 	struct shmem_sb_info *sbinfo = SHMEM_SB(dentry->d_sb);
@@ -3673,6 +3730,10 @@ static int shmem_fill_super(struct super_block *sb, struct fs_context *fc)
 	}
 	sb->s_export_op = &shmem_export_ops;
 	sb->s_flags |= SB_NOSEC;
+#ifdef CONFIG_TMPFS_VERITY
+	sb->s_vop = &shmem_verityops;
+	sbinfo->verity = true;
+#endif
 #else
 	sb->s_flags |= SB_NOUSER;
 #endif
@@ -3825,6 +3886,10 @@ static const struct file_operations shmem_file_operations = {
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= shmem_fallocate,
+	.unlocked_ioctl	= shmem_ioctl,
+#ifdef CONFIG_TMPFS_VERITY
+	.open		= shmem_file_open,
+#endif
 #endif
 };
 
diff --git a/mm/shmem_verity.c b/mm/shmem_verity.c
new file mode 100644
index 000000000000..f5f5c7394dda
--- /dev/null
+++ b/mm/shmem_verity.c
@@ -0,0 +1,267 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Google LLC
+ * Copyright 2021 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ */
+
+/*
+ * Implementation of fsverity_operations for tmpfs.
+ *
+ * Like ext4, tmpfs stores the verity metadata (Merkle tree and
+ * fsverity_descriptor) past the end of the file, starting at the first 64K
+ * boundary beyond i_size.
+ *
+ * Using a 64K boundary rather than a 4K one keeps things ready for
+ * architectures with 64K pages, and it doesn't necessarily waste space on-disk
+ * since there can be a hole between i_size and the start of the Merkle tree.
+ */
+
+#include <linux/xattr.h>
+#include <linux/fsverity.h>
+#include <linux/shmem_fs.h>
+#include <linux/quotaops.h>
+
+#define SHMEM_VERIFY_VER	(1)
+
+static inline loff_t shmem_verity_metadata_pos(const struct inode *inode)
+{
+	return round_up(inode->i_size, 65536);
+}
+
+/*
+ * Read some verity metadata from the inode.  __vfs_read() can't be used because
+ * we need to read beyond i_size.
+ */
+static int pagecache_read(struct inode *inode, void *buf, size_t count,
+			  loff_t pos)
+{
+	while (count) {
+		size_t n = min_t(size_t, count,
+				 PAGE_SIZE - offset_in_page(pos));
+		struct page *page;
+		void *addr;
+
+		page = shmem_read_mapping_page(inode->i_mapping,
+					       pos >> PAGE_SHIFT);
+		if (IS_ERR(page))
+			return PTR_ERR(page);
+
+		addr = kmap_atomic(page);
+		memcpy(buf, addr + offset_in_page(pos), n);
+		kunmap_atomic(addr);
+
+		put_page(page);
+
+		buf += n;
+		pos += n;
+		count -= n;
+	}
+	return 0;
+}
+
+/*
+ * Write some verity metadata to the inode for FS_IOC_ENABLE_VERITY.
+ * kernel_write() can't be used because the file descriptor is readonly.
+ */
+static int pagecache_write(struct inode *inode, const void *buf, size_t count,
+			   loff_t pos)
+{
+	if (pos + count > inode->i_sb->s_maxbytes)
+		return -EFBIG;
+
+	while (count) {
+		size_t n = min_t(size_t, count,
+				 PAGE_SIZE - offset_in_page(pos));
+		struct page *page;
+		void *fsdata;
+		void *addr;
+		int res;
+
+		res = pagecache_write_begin(NULL, inode->i_mapping, pos, n, 0,
+					    &page, &fsdata);
+		if (res)
+			return res;
+
+		addr = kmap_atomic(page);
+		memcpy(addr + offset_in_page(pos), buf, n);
+		kunmap_atomic(addr);
+
+		res = pagecache_write_end(NULL, inode->i_mapping, pos, n, n,
+					  page, fsdata);
+		if (res < 0)
+			return res;
+		if (res != n)
+			return -EIO;
+
+		buf += n;
+		pos += n;
+		count -= n;
+	}
+	return 0;
+}
+
+/*
+ * Format of tmpfs verity xattr.  This points to the location of the verity
+ * descriptor within the file data rather than containing it (the code was taken
+ * from fs/f2fs/verity.c).
+ */
+struct fsverity_descriptor_location {
+	__le32 version;
+	__le32 size;
+	__le64 pos;
+};
+
+static int shmem_begin_enable_verity(struct file *filp)
+{
+	struct inode *inode = file_inode(filp);
+	int err;
+
+	if (shmem_verity_in_progress(inode))
+		return -EBUSY;
+
+	/*
+	 * Since the file was opened readonly, we have to initialize the quotas
+	 * here and not rely on ->open() doing it.
+	 */
+	err = dquot_initialize(inode);
+	if (err)
+		return err;
+
+	shmem_verity_set_in_progress(inode);
+	return 0;
+}
+
+static int shmem_end_enable_verity(struct file *filp, const void *desc,
+				   size_t desc_size, u64 merkle_tree_size)
+{
+	struct inode *inode = file_inode(filp);
+	struct shmem_inode_info *info = SHMEM_I(inode);
+	u64 desc_pos = shmem_verity_metadata_pos(inode) + merkle_tree_size;
+	struct fsverity_descriptor_location dloc = {
+		.version = cpu_to_le32(SHMEM_VERIFY_VER),
+		.size = cpu_to_le32(desc_size),
+		.pos = cpu_to_le64(desc_pos),
+	};
+	int err = 0;
+
+	/*
+	 * If an error already occurred (which fs/verity/ signals by passing
+	 * desc == NULL), then only clean-up is needed.
+	 */
+	if (desc == NULL)
+		goto cleanup;
+
+	/* Append the verity descriptor. */
+	err = pagecache_write(inode, desc, desc_size, desc_pos);
+	if (err)
+		goto cleanup;
+
+	/*
+	 * Write all pages (both data and verity metadata).  Note that this must
+	 * happen before clearing SHMEM_VERITY_IN_PROGRESS; otherwise pages
+	 * beyond i_size won't be written properly.
+	 */
+	err = filemap_write_and_wait(inode->i_mapping);
+	if (err)
+		goto cleanup;
+
+	/* Set the verity xattr. */
+	err = simple_xattr_set(&info->xattrs, SHMEM_XATTR_NAME_VERITY, &dloc,
+			       sizeof(dloc), XATTR_CREATE, NULL);
+	if (err)
+		goto cleanup;
+
+	/* Finally, set the verity inode flag. */
+	inode_set_flags(inode, S_VERITY, inode->i_flags | S_VERITY);
+	mark_inode_dirty_sync(inode);
+
+	shmem_verity_clear_in_progress(inode);
+	return 0;
+
+cleanup:
+	/*
+	 * Verity failed to be enabled, so clean up by truncating any verity
+	 * metadata that was written beyond i_size (both from cache and from
+	 * disk) and clearing FI_VERITY_IN_PROGRESS.
+	 */
+	shmem_truncate_range(inode, 0, inode->i_size);
+	shmem_verity_clear_in_progress(inode);
+	return err;
+}
+
+static int shmem_get_verity_descriptor(struct inode *inode, void *buf,
+				       size_t buf_size)
+{
+	struct fsverity_descriptor_location dloc;
+	struct shmem_inode_info *info = SHMEM_I(inode);
+	int res;
+	u32 size;
+	u64 pos;
+
+	/* Get the descriptor location */
+	res = simple_xattr_get(&info->xattrs, SHMEM_XATTR_NAME_VERITY, &dloc,
+			       sizeof(dloc));
+	if (res < 0 && res != -ERANGE)
+		return res;
+	if (res != sizeof(dloc) ||
+	    dloc.version != cpu_to_le32(SHMEM_VERIFY_VER)) {
+		pr_err("Unknown verity xattr format inode %lu\n", inode->i_ino);
+		return -EINVAL;
+	}
+	size = le32_to_cpu(dloc.size);
+	pos = le64_to_cpu(dloc.pos);
+
+	/* Get the descriptor */
+	if (pos + size < pos || pos + size > inode->i_sb->s_maxbytes ||
+	    pos < shmem_verity_metadata_pos(inode) || size > INT_MAX) {
+		pr_err("Invalid verity xattr for inode %lu\n", inode->i_ino);
+		return -EINVAL;
+	}
+	if (buf_size) {
+		if (size > buf_size)
+			return -ERANGE;
+		res = pagecache_read(inode, buf, size, pos);
+		if (res)
+			return res;
+	}
+	return size;
+}
+
+static struct page *shmem_read_merkle_tree_page(struct inode *inode,
+						pgoff_t index,
+						unsigned long num_ra_pages)
+{
+	DEFINE_READAHEAD(ractl, NULL, NULL, inode->i_mapping, index);
+	struct page *page;
+
+	index += shmem_verity_metadata_pos(inode) >> PAGE_SHIFT;
+
+	page = find_get_page_flags(inode->i_mapping, index, FGP_ACCESSED);
+	if (!page || !PageUptodate(page)) {
+		if (page)
+			put_page(page);
+		else if (num_ra_pages > 1)
+			page_cache_ra_unbounded(&ractl, num_ra_pages, 0);
+		page = shmem_read_mapping_page(inode->i_mapping, index);
+	}
+	return page;
+}
+
+static int shmem_write_merkle_tree_block(struct inode *inode, const void *buf,
+					 u64 index, int log_blocksize)
+{
+	loff_t pos = shmem_verity_metadata_pos(inode) +
+		     (index << log_blocksize);
+
+	return pagecache_write(inode, buf, 1 << log_blocksize, pos);
+}
+
+const struct fsverity_operations shmem_verityops = {
+	.begin_enable_verity	= shmem_begin_enable_verity,
+	.end_enable_verity	= shmem_end_enable_verity,
+	.get_verity_descriptor	= shmem_get_verity_descriptor,
+	.read_merkle_tree_page	= shmem_read_merkle_tree_page,
+	.write_merkle_tree_block = shmem_write_merkle_tree_block,
+};
-- 
2.32.0

Re: [RFC][PATCH 4/5] shmem: Avoid segfault in shmem_read_mapping_page_gfp()

[Ajay Garg]

Hi Roberto.

Identical patch has been floated earlier via :
https://lore.kernel.org/linux-mm/CAMZfGtUp6dkT4OWzLhL8whqNnXAbfVw5c6AQogHzY3bbM_k2Qw@mail.gmail.com/T/#m2189d135b9293de9b4a11362f0179c17b254d5ab


Thanks and Regards,
Ajay

On Fri, Nov 12, 2021 at 6:15 PM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> Check the hwpoison page flag only if the page is valid in
> shmem_read_mapping_page_gfp(). The PageHWPoison() macro tries to access
> the page flags and cannot work on an error pointer.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  mm/shmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 23c91a8beb78..427863cbf0dc 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -4222,7 +4222,7 @@ struct page *shmem_read_mapping_page_gfp(struct address_space *mapping,
>         else
>                 unlock_page(page);
>
> -       if (PageHWPoison(page))
> +       if (!IS_ERR(page) && PageHWPoison(page))
>                 page = ERR_PTR(-EIO);
>
>         return page;
> --
> 2.32.0
>

RE: [RFC][PATCH 4/5] shmem: Avoid segfault in shmem_read_mapping_page_gfp()

[Roberto Sassu]

> From: Ajay Garg [mailto:ajaygargnsit@gmail.com]
> Sent: Friday, November 12, 2021 1:54 PM
> Hi Roberto.
> 
> Identical patch has been floated earlier via :
> https://lore.kernel.org/linux-
> mm/CAMZfGtUp6dkT4OWzLhL8whqNnXAbfVw5c6AQogHzY3bbM_k2Qw@mail.
> gmail.com/T/#m2189d135b9293de9b4a11362f0179c17b254d5ab

Hi Ajay

thanks, I was not aware.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> Thanks and Regards,
> Ajay
> 
> On Fri, Nov 12, 2021 at 6:15 PM Roberto Sassu <roberto.sassu@huawei.com>
> wrote:
> >
> > Check the hwpoison page flag only if the page is valid in
> > shmem_read_mapping_page_gfp(). The PageHWPoison() macro tries to
> access
> > the page flags and cannot work on an error pointer.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  mm/shmem.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index 23c91a8beb78..427863cbf0dc 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -4222,7 +4222,7 @@ struct page
> *shmem_read_mapping_page_gfp(struct address_space *mapping,
> >         else
> >                 unlock_page(page);
> >
> > -       if (PageHWPoison(page))
> > +       if (!IS_ERR(page) && PageHWPoison(page))
> >                 page = ERR_PTR(-EIO);
> >
> >         return page;
> > --
> > 2.32.0
> >

Re: [RFC][PATCH 4/5] shmem: Avoid segfault in shmem_read_mapping_page_gfp()

[Eric Biggers]

On Fri, Nov 12, 2021 at 01:44:10PM +0100, Roberto Sassu wrote:
> Check the hwpoison page flag only if the page is valid in
> shmem_read_mapping_page_gfp(). The PageHWPoison() macro tries to access
> the page flags and cannot work on an error pointer.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

This looks like a recent regression from the commit:

	commit b9d02f1bdd98f38e6e5ecacc9786a8f58f3f8b2c
	Author: Yang Shi <shy828301@gmail.com>
	Date:   Fri Nov 5 13:41:10 2021 -0700

	    mm: shmem: don't truncate page if memory failure happens

Can you please send this fix out as a standalone patch, to the right people and
including the appropriate "Fixes" tag?

- Eric

Re: [RFC][PATCH 5/5] shmem: Add fsverity support

[Eric Biggers]

On Fri, Nov 12, 2021 at 01:44:11PM +0100, Roberto Sassu wrote:
> Make the necessary modifications to support fsverity in tmpfs.
> 
> First, implement the fsverity operations (in a similar way of f2fs). These
> operations make use of shmem_read_mapping_page() instead of
> read_mapping_page() to handle the case where the page has been swapped out.
> The fsverity descriptor is placed at the end of the file and its location
> is stored in an xattr.
> 
> Second, implement the ioctl operations to enable, measure and read fsverity
> metadata.
> 
> Lastly, add calls to fsverity functions, to ensure that fsverity-relevant
> operations are checked and handled by fsverity (file open, attr set, inode
> evict).
> 
> Fsverity support can be enabled through the kernel configuration and
> remains enabled by default for every tmpfs filesystem instantiated (there
> should be no overhead, unless fsverity is enabled for a file).
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

I don't see how this makes sense at all.  The point of fs-verity is to avoid
having to hash the whole file when verifying it.  However, obviously the whole
file still has to be hashed to build the Merkle tree in the first place.  That
makes sense for a persistent filesystem where a file can be written once and
verified many times.  I don't see how it makes sense for tmpfs, where files have
to be re-created on every boot.  You might as well just hash the whole file.

Also, you didn't implement actually verifying the data (by calling
fsverity_verify_page()), so this patch doesn't really do anything anyway.

- Eric

Re: [RFC][PATCH 2/5] fsverity: Revalidate built-in signatures at file open

[Eric Biggers]

On Fri, Nov 12, 2021 at 01:44:08PM +0100, Roberto Sassu wrote:
> Fsverity signatures are validated only upon request by the user by setting
> the requirement through procfs or sysctl.
> 
> However, signatures are validated only when the fsverity-related
> initialization is performed on the file. If the initialization happened
> while the signature requirement was disabled, the signature is not
> validated again.

I'm not sure this really matters.  If someone has started using a verity file
before the require_signatures sysctl was set, then there is already a race
condition; this patch doesn't fix that.  Don't you need to set the
require_signatures sysctl early enough anyway?

- Eric

RE: [RFC][PATCH 4/5] shmem: Avoid segfault in shmem_read_mapping_page_gfp()

[Roberto Sassu]

> From: Eric Biggers [mailto:ebiggers@kernel.org]
> Sent: Friday, November 12, 2021 7:56 PM
> On Fri, Nov 12, 2021 at 01:44:10PM +0100, Roberto Sassu wrote:
> > Check the hwpoison page flag only if the page is valid in
> > shmem_read_mapping_page_gfp(). The PageHWPoison() macro tries to
> access
> > the page flags and cannot work on an error pointer.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> 
> This looks like a recent regression from the commit:
> 
> 	commit b9d02f1bdd98f38e6e5ecacc9786a8f58f3f8b2c
> 	Author: Yang Shi <shy828301@gmail.com>
> 	Date:   Fri Nov 5 13:41:10 2021 -0700
> 
> 	    mm: shmem: don't truncate page if memory failure happens
> 
> Can you please send this fix out as a standalone patch, to the right people and
> including the appropriate "Fixes" tag?

Hi Eric

it looks there is another patch. Given that it was proposed before,
I will drop mine. Thanks anyway.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

RE: [RFC][PATCH 5/5] shmem: Add fsverity support

[Roberto Sassu]

> From: Eric Biggers [mailto:ebiggers@kernel.org]
> Sent: Friday, November 12, 2021 8:12 PM
> On Fri, Nov 12, 2021 at 01:44:11PM +0100, Roberto Sassu wrote:
> > Make the necessary modifications to support fsverity in tmpfs.
> >
> > First, implement the fsverity operations (in a similar way of f2fs). These
> > operations make use of shmem_read_mapping_page() instead of
> > read_mapping_page() to handle the case where the page has been swapped
> out.
> > The fsverity descriptor is placed at the end of the file and its location
> > is stored in an xattr.
> >
> > Second, implement the ioctl operations to enable, measure and read fsverity
> > metadata.
> >
> > Lastly, add calls to fsverity functions, to ensure that fsverity-relevant
> > operations are checked and handled by fsverity (file open, attr set, inode
> > evict).
> >
> > Fsverity support can be enabled through the kernel configuration and
> > remains enabled by default for every tmpfs filesystem instantiated (there
> > should be no overhead, unless fsverity is enabled for a file).
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> 
> I don't see how this makes sense at all.  The point of fs-verity is to avoid
> having to hash the whole file when verifying it.  However, obviously the whole
> file still has to be hashed to build the Merkle tree in the first place.  That
> makes sense for a persistent filesystem where a file can be written once and
> verified many times.  I don't see how it makes sense for tmpfs, where files have
> to be re-created on every boot.  You might as well just hash the whole file.

The point of adding fsverity support for tmpfs was to being able to do
integrity enforcement with just one mechanism, given that I was
planning to do integrity verification with reference values loaded
to the kernel with DIGLIM [1].

With an LSM such as IPE [2], integrity verification would consist in
querying the fsverity digest with DIGLIM and allowing the operation
if the digest was found. With fsverity support in tmpfs, this can be
done from the very beginning of the boot process.

Using regular file digests would be also possible but this requires
loading with DIGLIM both fsverity and non-fsverity reference values.
It would also require two separate mechanisms for calculating
the file digest depending on the filesystem. It could be done, but
I thought it was easier to add support for fsverity in tmpfs.

> Also, you didn't implement actually verifying the data (by calling
> fsverity_verify_page()), so this patch doesn't really do anything anyway.

Yes, at the end I didn't add it. Probably the only place where
calling fsverity_verify_page() would make sense is when a page
is swapped in (assuming that the swap device is untrusted).

I tried to add a call in shmem_swapin_page() but fsverity complained
due to the fact that the page was already up to date, and also
rejected the page. I will check it better.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

[1] https://lore.kernel.org/linux-integrity/20210914163401.864635-1-roberto.sassu@huawei.com/
[2] https://lore.kernel.org/linux-security-module/1634151995-16266-1-git-send-email-deven.desai@linux.microsoft.com/

RE: [RFC][PATCH 2/5] fsverity: Revalidate built-in signatures at file open

[Roberto Sassu]

> From: Eric Biggers [mailto:ebiggers@kernel.org]
> Sent: Friday, November 12, 2021 8:15 PM
> On Fri, Nov 12, 2021 at 01:44:08PM +0100, Roberto Sassu wrote:
> > Fsverity signatures are validated only upon request by the user by setting
> > the requirement through procfs or sysctl.
> >
> > However, signatures are validated only when the fsverity-related
> > initialization is performed on the file. If the initialization happened
> > while the signature requirement was disabled, the signature is not
> > validated again.
> 
> I'm not sure this really matters.  If someone has started using a verity file
> before the require_signatures sysctl was set, then there is already a race
> condition; this patch doesn't fix that.  Don't you need to set the
> require_signatures sysctl early enough anyway?

Yes, access to already opened files is not revoked. It will work
for a new open. Actually, the main reason for this patch was that
one of the tests in xfstests-dev fails (generic/577).

While persistent filesystems are unmounted and mounted before
the test, which causes the fsverity_info structure to be removed
from the inode, requiring a new verification, tmpfs is just remounted.
During remount, the fsverity_info structure of already instantiated
inodes is kept.

Since fsverity_verify_signature() is called only when the
fsverity_info structure is created, all files with that structure are
considered valid, even if signature verification was temporarily
disabled at the time the structure was created.

Requiring signature verification early could be a solution, but
it is still at discretion of root. Maybe it would be a good idea to
disable the interface with a kernel option, so that signatures
can be enforced in a mandatory way.

This patch probably helps more LSMs, by exposing the information
of whether the signature was validated, to make their decision
depending on their policy.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

Re: [RFC][PATCH 5/5] shmem: Add fsverity support

[Eric Biggers]

On Mon, Nov 15, 2021 at 08:49:41AM +0000, Roberto Sassu wrote:
> > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > Sent: Friday, November 12, 2021 8:12 PM
> > On Fri, Nov 12, 2021 at 01:44:11PM +0100, Roberto Sassu wrote:
> > > Make the necessary modifications to support fsverity in tmpfs.
> > >
> > > First, implement the fsverity operations (in a similar way of f2fs). These
> > > operations make use of shmem_read_mapping_page() instead of
> > > read_mapping_page() to handle the case where the page has been swapped
> > out.
> > > The fsverity descriptor is placed at the end of the file and its location
> > > is stored in an xattr.
> > >
> > > Second, implement the ioctl operations to enable, measure and read fsverity
> > > metadata.
> > >
> > > Lastly, add calls to fsverity functions, to ensure that fsverity-relevant
> > > operations are checked and handled by fsverity (file open, attr set, inode
> > > evict).
> > >
> > > Fsverity support can be enabled through the kernel configuration and
> > > remains enabled by default for every tmpfs filesystem instantiated (there
> > > should be no overhead, unless fsverity is enabled for a file).
> > >
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > I don't see how this makes sense at all.  The point of fs-verity is to avoid
> > having to hash the whole file when verifying it.  However, obviously the whole
> > file still has to be hashed to build the Merkle tree in the first place.  That
> > makes sense for a persistent filesystem where a file can be written once and
> > verified many times.  I don't see how it makes sense for tmpfs, where files have
> > to be re-created on every boot.  You might as well just hash the whole file.
> 
> The point of adding fsverity support for tmpfs was to being able to do
> integrity enforcement with just one mechanism, given that I was
> planning to do integrity verification with reference values loaded
> to the kernel with DIGLIM [1].
> 
> With an LSM such as IPE [2], integrity verification would consist in
> querying the fsverity digest with DIGLIM and allowing the operation
> if the digest was found. With fsverity support in tmpfs, this can be
> done from the very beginning of the boot process.
> 
> Using regular file digests would be also possible but this requires
> loading with DIGLIM both fsverity and non-fsverity reference values.
> It would also require two separate mechanisms for calculating
> the file digest depending on the filesystem. It could be done, but
> I thought it was easier to add support for fsverity in tmpfs.
> 
> > Also, you didn't implement actually verifying the data (by calling
> > fsverity_verify_page()), so this patch doesn't really do anything anyway.
> 
> Yes, at the end I didn't add it. Probably the only place where
> calling fsverity_verify_page() would make sense is when a page
> is swapped in (assuming that the swap device is untrusted).
> 
> I tried to add a call in shmem_swapin_page() but fsverity complained
> due to the fact that the page was already up to date, and also
> rejected the page. I will check it better.
> 

It sounds like you really only care about calculating fs-verity file digests.
That's just an algorithm for hashing a file, so it could just be implemented in
generic code that operates on any file on any filesystem, like how IMA
implemennts full file hashing for any file.  There isn't a need for any special
filesystem support to do this.

- Eric

RE: [RFC][PATCH 5/5] shmem: Add fsverity support

[Roberto Sassu]

> From: Eric Biggers [mailto:ebiggers@kernel.org]
> Sent: Monday, November 15, 2021 8:05 PM
> On Mon, Nov 15, 2021 at 08:49:41AM +0000, Roberto Sassu wrote:
> > > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > > Sent: Friday, November 12, 2021 8:12 PM
> > > On Fri, Nov 12, 2021 at 01:44:11PM +0100, Roberto Sassu wrote:
> > > > Make the necessary modifications to support fsverity in tmpfs.
> > > >
> > > > First, implement the fsverity operations (in a similar way of f2fs). These
> > > > operations make use of shmem_read_mapping_page() instead of
> > > > read_mapping_page() to handle the case where the page has been
> swapped
> > > out.
> > > > The fsverity descriptor is placed at the end of the file and its location
> > > > is stored in an xattr.
> > > >
> > > > Second, implement the ioctl operations to enable, measure and read
> fsverity
> > > > metadata.
> > > >
> > > > Lastly, add calls to fsverity functions, to ensure that fsverity-relevant
> > > > operations are checked and handled by fsverity (file open, attr set, inode
> > > > evict).
> > > >
> > > > Fsverity support can be enabled through the kernel configuration and
> > > > remains enabled by default for every tmpfs filesystem instantiated (there
> > > > should be no overhead, unless fsverity is enabled for a file).
> > > >
> > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > >
> > > I don't see how this makes sense at all.  The point of fs-verity is to avoid
> > > having to hash the whole file when verifying it.  However, obviously the
> whole
> > > file still has to be hashed to build the Merkle tree in the first place.  That
> > > makes sense for a persistent filesystem where a file can be written once and
> > > verified many times.  I don't see how it makes sense for tmpfs, where files
> have
> > > to be re-created on every boot.  You might as well just hash the whole file.
> >
> > The point of adding fsverity support for tmpfs was to being able to do
> > integrity enforcement with just one mechanism, given that I was
> > planning to do integrity verification with reference values loaded
> > to the kernel with DIGLIM [1].
> >
> > With an LSM such as IPE [2], integrity verification would consist in
> > querying the fsverity digest with DIGLIM and allowing the operation
> > if the digest was found. With fsverity support in tmpfs, this can be
> > done from the very beginning of the boot process.
> >
> > Using regular file digests would be also possible but this requires
> > loading with DIGLIM both fsverity and non-fsverity reference values.
> > It would also require two separate mechanisms for calculating
> > the file digest depending on the filesystem. It could be done, but
> > I thought it was easier to add support for fsverity in tmpfs.
> >
> > > Also, you didn't implement actually verifying the data (by calling
> > > fsverity_verify_page()), so this patch doesn't really do anything anyway.
> >
> > Yes, at the end I didn't add it. Probably the only place where
> > calling fsverity_verify_page() would make sense is when a page
> > is swapped in (assuming that the swap device is untrusted).
> >
> > I tried to add a call in shmem_swapin_page() but fsverity complained
> > due to the fact that the page was already up to date, and also
> > rejected the page. I will check it better.
> >
> 
> It sounds like you really only care about calculating fs-verity file digests.
> That's just an algorithm for hashing a file, so it could just be implemented in
> generic code that operates on any file on any filesystem, like how IMA
> implemennts full file hashing for any file.  There isn't a need for any special
> filesystem support to do this.

Initially I thought the same. Then, I realized that fsverity is much more
than that. Fsverity could be seen as a sort of property enforcer, it provides
a property associated to the file (the fsverity digest) and ensures that
the property remains the same while the system is running. In addition,
it takes advantage of the page cache to avoid remeasuring an up to date
page.

This remove some burden from LSMs. IPE would have just to compare
the fsverity digest with that in the policy (or just query it with DIGLIM).
Not taking into consideration the specific filesystem, not having to
fall back to the new fsverity measurement function, and avoiding to
preserve the fsverity property by itself, would make the LSM
implementation very simple.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua