Skip to content

Commit

Permalink
fsverity: remove debug messages and CONFIG_FS_VERITY_DEBUG
Browse files Browse the repository at this point in the history
I've gotten very little use out of these debug messages, and I'm not
aware of anyone else having used them.

Indeed, sprinkling pr_debug around is not really a best practice these
days, especially for filesystem code.  Tracepoints are used instead.

Let's just remove these and start from a clean slate.

This change does not affect info, warning, and error messages.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Link: https://lore.kernel.org/r/20221215060420.60692-1-ebiggers@kernel.org
  • Loading branch information
Eric Biggers committed Jan 1, 2023
1 parent 72ea15f commit 86f6656
Show file tree
Hide file tree
Showing 7 changed files with 2 additions and 59 deletions.
8 changes: 0 additions & 8 deletions fs/verity/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,6 @@ config FS_VERITY

If unsure, say N.

config FS_VERITY_DEBUG
bool "FS Verity debugging"
depends on FS_VERITY
help
Enable debugging messages related to fs-verity by default.

Say N unless you are an fs-verity developer.

config FS_VERITY_BUILTIN_SIGNATURES
bool "FS Verity builtin signature support"
depends on FS_VERITY
Expand Down
11 changes: 0 additions & 11 deletions fs/verity/enable.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ static int build_merkle_tree_level(struct file *filp, unsigned int level,
for (i = 0; i < num_blocks_to_hash; i++) {
struct page *src_page;

if ((pgoff_t)i % 10000 == 0 || i + 1 == num_blocks_to_hash)
pr_debug("Hashing block %llu of %llu for level %u\n",
i + 1, num_blocks_to_hash, level);

if (level == 0) {
/* Leaf: hashing a data block */
src_page = read_file_data_page(filp, i, &ra,
Expand Down Expand Up @@ -263,15 +259,12 @@ static int enable_verity(struct file *filp,
* ->begin_enable_verity() and ->end_enable_verity() using the inode
* lock and only allow one process to be here at a time on a given file.
*/
pr_debug("Building Merkle tree...\n");
BUILD_BUG_ON(sizeof(desc->root_hash) < FS_VERITY_MAX_DIGEST_SIZE);
err = build_merkle_tree(filp, &params, desc->root_hash);
if (err) {
fsverity_err(inode, "Error %d building Merkle tree", err);
goto rollback;
}
pr_debug("Done building Merkle tree. Root hash is %s:%*phN\n",
params.hash_alg->name, params.digest_size, desc->root_hash);

/*
* Create the fsverity_info. Don't bother trying to save work by
Expand All @@ -286,10 +279,6 @@ static int enable_verity(struct file *filp,
goto rollback;
}

if (arg->sig_size)
pr_debug("Storing a %u-byte PKCS#7 signature alongside the file\n",
arg->sig_size);

/*
* Tell the filesystem to finish enabling verity on the file.
* Serialized with ->begin_enable_verity() by the inode lock.
Expand Down
4 changes: 0 additions & 4 deletions fs/verity/fsverity_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@
#ifndef _FSVERITY_PRIVATE_H
#define _FSVERITY_PRIVATE_H

#ifdef CONFIG_FS_VERITY_DEBUG
#define DEBUG
#endif

#define pr_fmt(fmt) "fs-verity: " fmt

#include <linux/fsverity.h>
Expand Down
1 change: 0 additions & 1 deletion fs/verity/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ static int __init fsverity_init(void)
if (err)
goto err_exit_workqueue;

pr_debug("Initialized fs-verity\n");
return 0;

err_exit_workqueue:
Expand Down
21 changes: 2 additions & 19 deletions fs/verity/open.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@ int fsverity_init_merkle_tree_params(struct merkle_tree_params *params,
params->log_arity = params->log_blocksize - ilog2(params->digest_size);
params->hashes_per_block = 1 << params->log_arity;

pr_debug("Merkle tree uses %s with %u-byte blocks (%u hashes/block), salt=%*phN\n",
hash_alg->name, params->block_size, params->hashes_per_block,
(int)salt_size, salt);

/*
* Compute the number of levels in the Merkle tree and create a map from
* level to the starting block of that level. Level 'num_levels - 1' is
Expand All @@ -90,7 +86,6 @@ int fsverity_init_merkle_tree_params(struct merkle_tree_params *params,

/* Compute number of levels and the number of blocks in each level */
blocks = ((u64)inode->i_size + params->block_size - 1) >> log_blocksize;
pr_debug("Data is %lld bytes (%llu blocks)\n", inode->i_size, blocks);
while (blocks > 1) {
if (params->num_levels >= FS_VERITY_MAX_LEVELS) {
fsverity_err(inode, "Too many levels in Merkle tree");
Expand All @@ -109,8 +104,6 @@ int fsverity_init_merkle_tree_params(struct merkle_tree_params *params,
for (level = (int)params->num_levels - 1; level >= 0; level--) {
blocks = params->level_start[level];
params->level_start[level] = offset;
pr_debug("Level %d is %llu blocks starting at index %llu\n",
level, blocks, offset);
offset += blocks;
}

Expand Down Expand Up @@ -176,9 +169,6 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
fsverity_err(inode, "Error %d computing file digest", err);
goto out;
}
pr_debug("Computed file digest: %s:%*phN\n",
vi->tree_params.hash_alg->name,
vi->tree_params.digest_size, vi->file_digest);

err = fsverity_verify_signature(vi, desc->signature,
le32_to_cpu(desc->sig_size));
Expand Down Expand Up @@ -327,23 +317,16 @@ static int ensure_verity_info(struct inode *inode)

int __fsverity_file_open(struct inode *inode, struct file *filp)
{
if (filp->f_mode & FMODE_WRITE) {
pr_debug("Denying opening verity file (ino %lu) for write\n",
inode->i_ino);
if (filp->f_mode & FMODE_WRITE)
return -EPERM;
}

return ensure_verity_info(inode);
}
EXPORT_SYMBOL_GPL(__fsverity_file_open);

int __fsverity_prepare_setattr(struct dentry *dentry, struct iattr *attr)
{
if (attr->ia_valid & ATTR_SIZE) {
pr_debug("Denying truncate of verity file (ino %lu)\n",
d_inode(dentry)->i_ino);
if (attr->ia_valid & ATTR_SIZE)
return -EPERM;
}
return 0;
}
EXPORT_SYMBOL_GPL(__fsverity_prepare_setattr);
Expand Down
2 changes: 0 additions & 2 deletions fs/verity/signature.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
return err;
}

pr_debug("Valid signature for file digest %s:%*phN\n",
hash_alg->name, hash_alg->digest_size, vi->file_digest);
return 0;
}

Expand Down
14 changes: 0 additions & 14 deletions fs/verity/verify.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#include <crypto/hash.h>
#include <linux/bio.h>
#include <linux/ratelimit.h>

static struct workqueue_struct *fsverity_read_workqueue;

Expand Down Expand Up @@ -91,8 +90,6 @@ static bool verify_page(struct inode *inode, const struct fsverity_info *vi,
if (WARN_ON_ONCE(!PageLocked(data_page) || PageUptodate(data_page)))
return false;

pr_debug_ratelimited("Verifying data page %lu...\n", index);

/*
* Starting at the leaf level, ascend the tree saving hash pages along
* the way until we find a verified hash page, indicated by PageChecked;
Expand All @@ -105,9 +102,6 @@ static bool verify_page(struct inode *inode, const struct fsverity_info *vi,

hash_at_level(params, index, level, &hindex, &hoffset);

pr_debug_ratelimited("Level %d: hindex=%lu, hoffset=%u\n",
level, hindex, hoffset);

hpage = inode->i_sb->s_vop->read_merkle_tree_page(inode, hindex,
level == 0 ? level0_ra_pages : 0);
if (IS_ERR(hpage)) {
Expand All @@ -122,19 +116,13 @@ static bool verify_page(struct inode *inode, const struct fsverity_info *vi,
memcpy_from_page(_want_hash, hpage, hoffset, hsize);
want_hash = _want_hash;
put_page(hpage);
pr_debug_ratelimited("Hash page already checked, want %s:%*phN\n",
params->hash_alg->name,
hsize, want_hash);
goto descend;
}
pr_debug_ratelimited("Hash page not yet checked\n");
hpages[level] = hpage;
hoffsets[level] = hoffset;
}

want_hash = vi->root_hash;
pr_debug("Want root hash: %s:%*phN\n",
params->hash_alg->name, hsize, want_hash);
descend:
/* Descend the tree verifying hash pages */
for (; level > 0; level--) {
Expand All @@ -151,8 +139,6 @@ static bool verify_page(struct inode *inode, const struct fsverity_info *vi,
memcpy_from_page(_want_hash, hpage, hoffset, hsize);
want_hash = _want_hash;
put_page(hpage);
pr_debug("Verified hash page at level %d, now want %s:%*phN\n",
level - 1, params->hash_alg->name, hsize, want_hash);
}

/* Finally, verify the data page */
Expand Down

0 comments on commit 86f6656

Please sign in to comment.