From 8b7d3fe96881e0f13c0176812b40432558882023 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 24 Feb 2023 15:25:30 -0800 Subject: [PATCH 1/5] fs/buffer.c: use b_folio for fsverity work Use b_folio now that it exists. This removes an unnecessary call to compound_head(). No actual change in behavior. Link: https://lore.kernel.org/r/20230224232530.98440-1-ebiggers@kernel.org Signed-off-by: Eric Biggers --- fs/buffer.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index 9e1e2add541e0..034bece271633 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -308,20 +308,19 @@ static void verify_bh(struct work_struct *work) struct buffer_head *bh = ctx->bh; bool valid; - valid = fsverity_verify_blocks(page_folio(bh->b_page), bh->b_size, - bh_offset(bh)); + valid = fsverity_verify_blocks(bh->b_folio, bh->b_size, bh_offset(bh)); end_buffer_async_read(bh, valid); kfree(ctx); } static bool need_fsverity(struct buffer_head *bh) { - struct page *page = bh->b_page; - struct inode *inode = page->mapping->host; + struct folio *folio = bh->b_folio; + struct inode *inode = folio->mapping->host; return fsverity_active(inode) && /* needed by ext4 */ - page->index < DIV_ROUND_UP(inode->i_size, PAGE_SIZE); + folio->index < DIV_ROUND_UP(inode->i_size, PAGE_SIZE); } static void decrypt_bh(struct work_struct *work) From 1238c8b91c5aca6dd13bccb1b4dc716718e7bfac Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Thu, 2 Mar 2023 12:28:24 -0800 Subject: [PATCH 2/5] fs-verity: simplify sysctls with register_sysctl() register_sysctl_paths() is only needed if your child (directories) have entries but this does not so just use register_sysctl() so to do away with the path specification. Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20230302202826.776286-10-mcgrof@kernel.org Signed-off-by: Eric Biggers --- fs/verity/signature.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/fs/verity/signature.c b/fs/verity/signature.c index e7d3ca919a1e0..b8c51ad40d3a3 100644 --- a/fs/verity/signature.c +++ b/fs/verity/signature.c @@ -88,12 +88,6 @@ int fsverity_verify_signature(const struct fsverity_info *vi, #ifdef CONFIG_SYSCTL static struct ctl_table_header *fsverity_sysctl_header; -static const struct ctl_path fsverity_sysctl_path[] = { - { .procname = "fs", }, - { .procname = "verity", }, - { } -}; - static struct ctl_table fsverity_sysctl_table[] = { { .procname = "require_signatures", @@ -109,8 +103,7 @@ static struct ctl_table fsverity_sysctl_table[] = { static int __init fsverity_sysctl_init(void) { - fsverity_sysctl_header = register_sysctl_paths(fsverity_sysctl_path, - fsverity_sysctl_table); + fsverity_sysctl_header = register_sysctl("fs/verity", fsverity_sysctl_table); if (!fsverity_sysctl_header) { pr_err("sysctl registration failed!\n"); return -ENOMEM; From 8eb8af4b3df5965dc65a24a32768043f39d82d59 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 27 Mar 2023 21:03:26 -0700 Subject: [PATCH 3/5] fsverity: use WARN_ON_ONCE instead of WARN_ON As per Linus's suggestion (https://lore.kernel.org/r/CAHk-=whefxRGyNGzCzG6BVeM=5vnvgb-XhSeFJVxJyAxAF8XRA@mail.gmail.com), use WARN_ON_ONCE instead of WARN_ON. This barely adds any extra overhead, and it makes it so that if any of these ever becomes reachable (they shouldn't, but that's the point), the logs can't be flooded. Link: https://lore.kernel.org/r/20230406181542.38894-1-ebiggers@kernel.org Reviewed-by: Chaitanya Kulkarni Reviewed-by: Christian Brauner Signed-off-by: Eric Biggers --- fs/verity/enable.c | 4 ++-- fs/verity/hash_algs.c | 4 ++-- fs/verity/open.c | 2 +- include/linux/fsverity.h | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/verity/enable.c b/fs/verity/enable.c index 7a0e3a84d370b..541c2a277c5c6 100644 --- a/fs/verity/enable.c +++ b/fs/verity/enable.c @@ -165,7 +165,7 @@ static int build_merkle_tree(struct file *filp, } } /* The root hash was filled by the last call to hash_one_block(). */ - if (WARN_ON(buffers[num_levels].filled != params->digest_size)) { + if (WARN_ON_ONCE(buffers[num_levels].filled != params->digest_size)) { err = -EINVAL; goto out; } @@ -277,7 +277,7 @@ static int enable_verity(struct file *filp, fsverity_err(inode, "%ps() failed with err %d", vops->end_enable_verity, err); fsverity_free_info(vi); - } else if (WARN_ON(!IS_VERITY(inode))) { + } else if (WARN_ON_ONCE(!IS_VERITY(inode))) { err = -EINVAL; fsverity_free_info(vi); } else { diff --git a/fs/verity/hash_algs.c b/fs/verity/hash_algs.c index 13fcf31be8441..ea00dbedf756b 100644 --- a/fs/verity/hash_algs.c +++ b/fs/verity/hash_algs.c @@ -84,9 +84,9 @@ struct fsverity_hash_alg *fsverity_get_hash_alg(const struct inode *inode, } err = -EINVAL; - if (WARN_ON(alg->digest_size != crypto_ahash_digestsize(tfm))) + if (WARN_ON_ONCE(alg->digest_size != crypto_ahash_digestsize(tfm))) goto err_free_tfm; - if (WARN_ON(alg->block_size != crypto_ahash_blocksize(tfm))) + if (WARN_ON_ONCE(alg->block_size != crypto_ahash_blocksize(tfm))) goto err_free_tfm; err = mempool_init_kmalloc_pool(&alg->req_pool, 1, diff --git a/fs/verity/open.c b/fs/verity/open.c index 9366b441d01ca..52048b7630dcc 100644 --- a/fs/verity/open.c +++ b/fs/verity/open.c @@ -83,7 +83,7 @@ int fsverity_init_merkle_tree_params(struct merkle_tree_params *params, params->log_blocks_per_page = PAGE_SHIFT - log_blocksize; params->blocks_per_page = 1 << params->log_blocks_per_page; - if (WARN_ON(!is_power_of_2(params->digest_size))) { + if (WARN_ON_ONCE(!is_power_of_2(params->digest_size))) { err = -EINVAL; goto out_err; } diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h index 119a3266791fd..e76605d5b36ee 100644 --- a/include/linux/fsverity.h +++ b/include/linux/fsverity.h @@ -233,18 +233,18 @@ static inline int fsverity_ioctl_read_metadata(struct file *filp, static inline bool fsverity_verify_blocks(struct folio *folio, size_t len, size_t offset) { - WARN_ON(1); + WARN_ON_ONCE(1); return false; } static inline void fsverity_verify_bio(struct bio *bio) { - WARN_ON(1); + WARN_ON_ONCE(1); } static inline void fsverity_enqueue_verify_work(struct work_struct *work) { - WARN_ON(1); + WARN_ON_ONCE(1); } #endif /* !CONFIG_FS_VERITY */ From 39049b69ec9fc125fa1f314165dcc86f72cb72ec Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 27 Mar 2023 21:15:05 -0700 Subject: [PATCH 4/5] fsverity: explicitly check for buffer overflow in build_merkle_tree() The new Merkle tree construction algorithm is a bit fragile in that it may overflow the 'root_hash' array if the tree actually generated does not match the calculated tree parameters. This should never happen unless there is a filesystem bug that allows the file size to change despite deny_write_access(), or a bug in the Merkle tree logic itself. Regardless, it's fairly easy to check for buffer overflow here, so let's do so. This is a robustness improvement only; this case is not currently known to be reachable. I've added a Fixes tag anyway, since I recommend that this be included in kernels that have the mentioned commit. Fixes: 56124d6c87fd ("fsverity: support enabling with tree block size < PAGE_SIZE") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20230328041505.110162-1-ebiggers@kernel.org Signed-off-by: Eric Biggers --- fs/verity/enable.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/fs/verity/enable.c b/fs/verity/enable.c index 541c2a277c5c6..bbec6f93172cf 100644 --- a/fs/verity/enable.c +++ b/fs/verity/enable.c @@ -13,6 +13,7 @@ struct block_buffer { u32 filled; + bool is_root_hash; u8 *data; }; @@ -24,6 +25,14 @@ static int hash_one_block(struct inode *inode, struct block_buffer *next = cur + 1; int err; + /* + * Safety check to prevent a buffer overflow in case of a filesystem bug + * that allows the file size to change despite deny_write_access(), or a + * bug in the Merkle tree logic itself + */ + if (WARN_ON_ONCE(next->is_root_hash && next->filled != 0)) + return -EINVAL; + /* Zero-pad the block if it's shorter than the block size. */ memset(&cur->data[cur->filled], 0, params->block_size - cur->filled); @@ -97,6 +106,7 @@ static int build_merkle_tree(struct file *filp, } } buffers[num_levels].data = root_hash; + buffers[num_levels].is_root_hash = true; BUILD_BUG_ON(sizeof(level_offset) != sizeof(params->level_start)); memcpy(level_offset, params->level_start, sizeof(level_offset)); From 04839139213cf60d4c5fc792214a08830e294ff8 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 6 Apr 2023 14:31:11 -0700 Subject: [PATCH 5/5] fsverity: reject FS_IOC_ENABLE_VERITY on mode 3 fds Commit 56124d6c87fd ("fsverity: support enabling with tree block size < PAGE_SIZE") changed FS_IOC_ENABLE_VERITY to use __kernel_read() to read the file's data, instead of direct pagecache accesses. An unintended consequence of this is that the 'WARN_ON_ONCE(!(file->f_mode & FMODE_READ))' in __kernel_read() became reachable by fuzz tests. This happens if FS_IOC_ENABLE_VERITY is called on a fd opened with access mode 3, which means "ioctl access only". Arguably, FS_IOC_ENABLE_VERITY should work on ioctl-only fds. But ioctl-only fds are a weird Linux extension that is rarely used and that few people even know about. (The documentation for FS_IOC_ENABLE_VERITY even specifically says it requires O_RDONLY.) It's probably not worthwhile to make the ioctl internally open a new fd just to handle this case. Thus, just reject the ioctl on such fds for now. Fixes: 56124d6c87fd ("fsverity: support enabling with tree block size < PAGE_SIZE") Reported-by: syzbot+51177e4144d764827c45@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?id=2281afcbbfa8fdb92f9887479cc0e4180f1c6b28 Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20230406215106.235829-1-ebiggers@kernel.org Reviewed-by: Christoph Hellwig Reviewed-by: Christian Brauner Signed-off-by: Eric Biggers --- fs/verity/enable.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fs/verity/enable.c b/fs/verity/enable.c index bbec6f93172cf..fc4c50e5219dc 100644 --- a/fs/verity/enable.c +++ b/fs/verity/enable.c @@ -357,6 +357,13 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *uarg) err = file_permission(filp, MAY_WRITE); if (err) return err; + /* + * __kernel_read() is used while building the Merkle tree. So, we can't + * allow file descriptors that were opened for ioctl access only, using + * the special nonstandard access mode 3. O_RDONLY only, please! + */ + if (!(filp->f_mode & FMODE_READ)) + return -EBADF; if (IS_APPEND(inode)) return -EPERM;