All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Masney <bmasney@redhat.com>
To: Alexander Larsson <alexl@redhat.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	gscrivan@redhat.com
Subject: Re: [PATCH 4/6] composefs: Add filesystem implementation
Date: Fri, 6 Jan 2023 07:18:41 -0500	[thread overview]
Message-ID: <Y7gRofheB9EaR4Mi@x1> (raw)
In-Reply-To: <1f0bd3e3a0c68ee19dd96ee0d573bb113428f1b6.1669631086.git.alexl@redhat.com>

On Mon, Nov 28, 2022 at 12:17:12PM +0100, Alexander Larsson wrote:
> This is the basic inode and filesystem implementation.
> 
> Signed-off-by: Alexander Larsson <alexl@redhat.com>
> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Note: I'm looking at this from the VFS viewpoint since I haven't done
anything in this subsystem. Just looking for some generic suggestions.

> ---
>  fs/composefs/cfs.c | 941 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 941 insertions(+)
>  create mode 100644 fs/composefs/cfs.c
> 
> diff --git a/fs/composefs/cfs.c b/fs/composefs/cfs.c
> new file mode 100644
> index 000000000000..4ed355ab079d
> --- /dev/null
> +++ b/fs/composefs/cfs.c
> @@ -0,0 +1,941 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * composefs
> + *
> + * Copyright (C) 2000 Linus Torvalds.
> + *               2000 Transmeta Corp.
> + * Copyright (C) 2021 Giuseppe Scrivano
> + * Copyright (C) 2022 Alexander Larsson
> + *
> + * This file is released under the GPL.
> + */
> +
> +#include <linux/exportfs.h>
> +#include <linux/fsverity.h>
> +#include <linux/fs_parser.h>
> +#include <linux/module.h>
> +#include <linux/namei.h>
> +#include <linux/seq_file.h>
> +#include <linux/version.h>
> +#include <linux/xattr.h>
> +#include <linux/statfs.h>
> +
> +#include "cfs-internals.h"
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Giuseppe Scrivano <gscrivan@redhat.com>");

An entry for composefs should be added to the MAINTAINERS file as well.

> +
> +#define CFS_MAX_STACK 500
> +
> +struct cfs_info {
> +	struct cfs_context_s cfs_ctx;
> +
> +	char *base_path;
> +
> +	size_t n_bases;
> +	struct vfsmount **bases;
> +
> +	u32 verity_check; /* 0 == none, 1 == if specified in image, 2 == require in image */
> +	bool has_digest;
> +	u8 digest[SHA256_DIGEST_SIZE]; /* fs-verity digest */
> +};
> +
> +struct cfs_inode {
> +	/* must be first for clear in cfs_alloc_inode to work */
> +	struct inode vfs_inode;

[ snip ]

> +static struct inode *cfs_alloc_inode(struct super_block *sb)
> +{
> +	struct cfs_inode *cino =
> +		alloc_inode_sb(sb, cfs_inode_cachep, GFP_KERNEL);
> +
> +	if (!cino)
> +		return NULL;
> +
> +	memset((u8 *)cino + sizeof(struct inode), 0,
> +	       sizeof(struct cfs_inode) - sizeof(struct inode));

Why not use container_of() to look up the vfs_inode and then you can get
rid of the restriction of this being first. This may also break with
structure randomization turned on.

> +
> +	struct cfs_inode_data_s inode_data;
> +};
> +
> +static inline struct cfs_inode *CFS_I(struct inode *inode)

CFS_I in upper case doesn't match naming conventions in the rest of the
kernel.

> +{
> +	return container_of(inode, struct cfs_inode, vfs_inode);
> +}
> +
> +static struct file empty_file;
> +
> +static const struct file_operations cfs_file_operations;
> +static const struct vm_operations_struct generic_file_vm_ops;
> +
> +static const struct super_operations cfs_ops;
> +static const struct file_operations cfs_dir_operations;
> +static const struct inode_operations cfs_dir_inode_operations;
> +static const struct inode_operations cfs_file_inode_operations;
> +static const struct inode_operations cfs_link_inode_operations;
> +
> +static const struct xattr_handler *cfs_xattr_handlers[];
> +static const struct export_operations cfs_export_operations;
> +
> +static const struct address_space_operations cfs_aops = {
> +	.direct_IO = noop_direct_IO,
> +};
> +
> +static ssize_t cfs_listxattr(struct dentry *dentry, char *names, size_t size);
> +
> +/* copied from overlayfs.  */
> +static unsigned int cfs_split_basedirs(char *str)
> +{
> +	unsigned int ctr = 1;
> +	char *s, *d;
> +
> +	for (s = d = str;; s++, d++) {
> +		if (*s == '\\') {
> +			s++;
> +		} else if (*s == ':') {
> +			*d = '\0';
> +			ctr++;
> +			continue;
> +		}
> +		*d = *s;
> +		if (!*s)
> +			break;
> +	}
> +	return ctr;
> +}

To expand on the comment, this is ovl_split_lowerdirs in
fs/overlayfs/super.c. It'd be nice if there was a common place where
this could go.

> +
> +static struct inode *cfs_make_inode(struct cfs_context_s *ctx,
> +				    struct super_block *sb, ino_t ino_num,
> +				    struct cfs_inode_s *ino,
> +				    const struct inode *dir)
> +{
> +	struct cfs_xattr_header_s *xattrs = NULL;
> +	struct cfs_inode *cino;
> +	struct inode *inode = NULL;
> +	struct cfs_inode_data_s inode_data = { 0 };
> +	int ret, res;

Put in reverse Christmas tree order.

> +
> +	res = cfs_init_inode_data(ctx, ino, ino_num, &inode_data);
> +	if (res < 0) {
> +		ret = res;
> +		goto fail;

Just return ERR_PTR(res) here. xattrs is set to NULL at this point and
the fail path attempts to free it. Yes, there's a check in kfree for
this. The code in cfs_inode_data_put appears to handle this NULL case
fine as well.

> +	}
> +
> +	inode = new_inode(sb);
> +	if (inode) {
> +		inode_init_owner(&init_user_ns, inode, dir, ino->st_mode);
> +		inode->i_mapping->a_ops = &cfs_aops;
> +
> +		cino = CFS_I(inode);
> +		cino->inode_data = inode_data;
> +
> +		inode->i_ino = ino_num;
> +		set_nlink(inode, ino->st_nlink);
> +		inode->i_rdev = ino->st_rdev;
> +		inode->i_uid = make_kuid(current_user_ns(), ino->st_uid);
> +		inode->i_gid = make_kgid(current_user_ns(), ino->st_gid);
> +		inode->i_mode = ino->st_mode;
> +		inode->i_atime = ino->st_mtim;
> +		inode->i_mtime = ino->st_mtim;
> +		inode->i_ctime = ino->st_ctim;
> +
> +		switch (ino->st_mode & S_IFMT) {
> +		case S_IFREG:
> +			inode->i_op = &cfs_file_inode_operations;
> +			inode->i_fop = &cfs_file_operations;
> +			inode->i_size = ino->st_size;
> +			break;
> +		case S_IFLNK:
> +			inode->i_link = cino->inode_data.path_payload;
> +			inode->i_op = &cfs_link_inode_operations;
> +			inode->i_fop = &cfs_file_operations;
> +			break;
> +		case S_IFDIR:
> +			inode->i_op = &cfs_dir_inode_operations;
> +			inode->i_fop = &cfs_dir_operations;
> +			inode->i_size = 4096;
> +			break;
> +		case S_IFCHR:
> +		case S_IFBLK:
> +			if (current_user_ns() != &init_user_ns) {
> +				ret = -EPERM;
> +				goto fail;
> +			}
> +			fallthrough;
> +		default:
> +			inode->i_op = &cfs_file_inode_operations;
> +			init_special_inode(inode, ino->st_mode, ino->st_rdev);
> +			break;
> +		}
> +	}
> +	return inode;
> +
> +fail:
> +	if (inode)
> +		iput(inode);
> +	kfree(xattrs);
> +	cfs_inode_data_put(&inode_data);
> +	return ERR_PTR(ret);
> +}
> +
> +static struct inode *cfs_get_root_inode(struct super_block *sb)
> +{
> +	struct cfs_info *fsi = sb->s_fs_info;
> +	struct cfs_inode_s ino_buf;
> +	struct cfs_inode_s *ino;
> +	u64 index;
> +
> +	ino = cfs_get_root_ino(&fsi->cfs_ctx, &ino_buf, &index);
> +	if (IS_ERR(ino))
> +		return ERR_CAST(ino);
> +
> +	return cfs_make_inode(&fsi->cfs_ctx, sb, index, ino, NULL);
> +}
> +
> +static bool cfs_iterate_cb(void *private, const char *name, int name_len,
> +			   u64 ino, unsigned int dtype)
> +{
> +	struct dir_context *ctx = private;
> +
> +	if (!dir_emit(ctx, name, name_len, ino, dtype))
> +		return 0;
> +
> +	ctx->pos++;
> +	return 1;
> +}
> +
> +static int cfs_iterate(struct file *file, struct dir_context *ctx)
> +{
> +	struct inode *inode = file->f_inode;
> +	struct cfs_info *fsi = inode->i_sb->s_fs_info;
> +	struct cfs_inode *cino = CFS_I(inode);
> +
> +	if (!dir_emit_dots(file, ctx))
> +		return 0;
> +
> +	return cfs_dir_iterate(&fsi->cfs_ctx, inode->i_ino, &cino->inode_data,
> +			       ctx->pos - 2, cfs_iterate_cb, ctx);
> +}
> +
> +static struct dentry *cfs_lookup(struct inode *dir, struct dentry *dentry,
> +				 unsigned int flags)
> +{
> +	struct cfs_info *fsi = dir->i_sb->s_fs_info;
> +	struct cfs_inode *cino = CFS_I(dir);
> +	struct cfs_inode_s ino_buf;
> +	struct inode *inode;
> +	struct cfs_inode_s *ino_s;
> +	u64 index;
> +	int ret;
> +
> +	if (dentry->d_name.len > NAME_MAX)
> +		return ERR_PTR(-ENAMETOOLONG);
> +
> +	ret = cfs_dir_lookup(&fsi->cfs_ctx, dir->i_ino, &cino->inode_data,
> +			     dentry->d_name.name, dentry->d_name.len, &index);
> +	if (ret < 0)
> +		return ERR_PTR(ret);
> +	if (ret == 0)
> +		goto return_negative;
> +
> +	ino_s = cfs_get_ino_index(&fsi->cfs_ctx, index, &ino_buf);
> +	if (IS_ERR(ino_s))
> +		return ERR_CAST(ino_s);
> +
> +	inode = cfs_make_inode(&fsi->cfs_ctx, dir->i_sb, index, ino_s, dir);
> +	if (IS_ERR(inode))
> +		return ERR_CAST(inode);
> +
> +	return d_splice_alias(inode, dentry);
> +
> +return_negative:
> +	d_add(dentry, NULL);
> +	return NULL;
> +}
> +
> +static const struct file_operations cfs_dir_operations = {
> +	.llseek = generic_file_llseek,
> +	.read = generic_read_dir,
> +	.iterate_shared = cfs_iterate,
> +};
> +
> +static const struct inode_operations cfs_dir_inode_operations = {
> +	.lookup = cfs_lookup,
> +	.listxattr = cfs_listxattr,
> +};
> +
> +static const struct inode_operations cfs_link_inode_operations = {
> +	.get_link = simple_get_link,
> +	.listxattr = cfs_listxattr,
> +};
> +
> +static void digest_to_string(const u8 *digest, char *buf)
> +{
> +	static const char hexchars[] = "0123456789abcdef";
> +	u32 i, j;
> +
> +	for (i = 0, j = 0; i < SHA256_DIGEST_SIZE; i++, j += 2) {
> +		u8 byte = digest[i];
> +
> +		buf[j] = hexchars[byte >> 4];
> +		buf[j + 1] = hexchars[byte & 0xF];
> +	}
> +	buf[j] = '\0';
> +}

Use %*phN printk() format with snprintf(). See the section 'Raw buffer
as a hex string' in Documentation/core-api/printk-formats.rst.

> +
> +static int digest_from_string(const char *digest_str, u8 *digest)
> +{
> +	size_t i, j;
> +
> +	for (i = 0, j = 0; i < SHA256_DIGEST_SIZE; i += 1, j += 2) {
> +		int big, little;
> +
> +		if (digest_str[j] == 0 || digest_str[j + 1] == 0)
> +			return -EINVAL; /* Too short string */
> +
> +		big = cfs_xdigit_value(digest_str[j]);
> +		little = cfs_xdigit_value(digest_str[j + 1]);
> +
> +		if (big == -1 || little == -1)
> +			return -EINVAL; /* Not hex digit */
> +
> +		digest[i] = (big << 4) | little;
> +	}
> +
> +	if (digest_str[j] != 0)
> +		return -EINVAL; /* Too long string */
> +
> +	return 0;
> +}

I mentioned this an another patch in this series but replace with
hex2bin() from lib/hexdump.c.

> +
> +/*
> + * Display the mount options in /proc/mounts.
> + */
> +static int cfs_show_options(struct seq_file *m, struct dentry *root)
> +{
> +	struct cfs_info *fsi = root->d_sb->s_fs_info;
> +
> +	if (fsi->base_path)
> +		seq_show_option(m, "basedir", fsi->base_path);
> +	if (fsi->has_digest) {
> +		char buf[SHA256_DIGEST_SIZE * 2 + 1];
> +
> +		digest_to_string(fsi->digest, buf);
> +		seq_show_option(m, "digest", buf);
> +	}
> +	if (fsi->verity_check != 0)
> +		seq_printf(m, ",verity_check=%u", fsi->verity_check);
> +
> +	return 0;
> +}
> +
> +static struct kmem_cache *cfs_inode_cachep;
> +

> +static void cfs_destroy_inode(struct inode *inode)
> +{
> +	struct cfs_inode *cino = CFS_I(inode);
> +
> +	cfs_inode_data_put(&cino->inode_data);
> +}
> +
> +static void cfs_free_inode(struct inode *inode)
> +{
> +	struct cfs_inode *cino = CFS_I(inode);
> +
> +	kmem_cache_free(cfs_inode_cachep, cino);
> +}
> +
> +static void cfs_put_super(struct super_block *sb)
> +{
> +	struct cfs_info *fsi = sb->s_fs_info;
> +
> +	cfs_ctx_put(&fsi->cfs_ctx);
> +	if (fsi->bases) {
> +		kern_unmount_array(fsi->bases, fsi->n_bases);
> +		kfree(fsi->bases);
> +	}
> +	kfree(fsi->base_path);
> +
> +	kfree(fsi);
> +}
> +
> +static int cfs_statfs(struct dentry *dentry, struct kstatfs *buf)
> +{
> +	struct cfs_info *fsi = dentry->d_sb->s_fs_info;
> +	int err = 0;
> +
> +	/* We return the free space, etc from the first base dir. */
> +	if (fsi->n_bases > 0) {
> +		struct path root = { .mnt = fsi->bases[0],
> +				     .dentry = fsi->bases[0]->mnt_root };
> +		err = vfs_statfs(&root, buf);
> +	}
> +
> +	if (!err) {
> +		buf->f_namelen = NAME_MAX;
> +		buf->f_type = dentry->d_sb->s_magic;
> +	}
> +
> +	return err;
> +}
> +
> +static const struct super_operations cfs_ops = {
> +	.statfs = cfs_statfs,
> +	.drop_inode = generic_delete_inode,
> +	.show_options = cfs_show_options,
> +	.put_super = cfs_put_super,
> +	.destroy_inode = cfs_destroy_inode,
> +	.alloc_inode = cfs_alloc_inode,
> +	.free_inode = cfs_free_inode,
> +};
> +
> +enum cfs_param {
> +	Opt_base_path,
> +	Opt_digest,
> +	Opt_verity_check,
> +};
> +
> +const struct fs_parameter_spec cfs_parameters[] = {
> +	fsparam_string("basedir", Opt_base_path),
> +	fsparam_string("digest", Opt_digest),
> +	fsparam_u32("verity_check", Opt_verity_check),
> +	{}
> +};
> +
> +static int cfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
> +{
> +	struct fs_parse_result result;
> +	struct cfs_info *fsi = fc->s_fs_info;
> +	int opt, r;
> +
> +	opt = fs_parse(fc, cfs_parameters, param, &result);
> +	if (opt == -ENOPARAM)
> +		return vfs_parse_fs_param_source(fc, param);
> +	if (opt < 0)
> +		return opt;
> +
> +	switch (opt) {
> +	case Opt_base_path:
> +		kfree(fsi->base_path);
> +		/* Take ownership.  */
> +		fsi->base_path = param->string;
> +		param->string = NULL;
> +		break;
> +	case Opt_digest:
> +		r = digest_from_string(param->string, fsi->digest);
> +		if (r < 0)
> +			return r;
> +		fsi->has_digest = true;
> +		fsi->verity_check = 2; /* Default to full verity check */
> +		break;
> +	case Opt_verity_check:
> +		if (result.uint_32 > 2)
> +			return invalfc(fc, "Invalid verity_check mode");
> +		fsi->verity_check = result.uint_32;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct vfsmount *resolve_basedir(const char *name)
> +{
> +	struct path path = {};
> +	struct vfsmount *mnt;
> +	int err = -EINVAL;
> +
> +	if (!*name) {
> +		pr_err("empty basedir\n");
> +		goto out;
> +	}
> +	err = kern_path(name, LOOKUP_FOLLOW, &path);
> +	if (err) {
> +		pr_err("failed to resolve '%s': %i\n", name, err);
> +		goto out;
> +	}
> +
> +	mnt = clone_private_mount(&path);
> +	err = PTR_ERR(mnt);
> +	if (IS_ERR(mnt)) {
> +		pr_err("failed to clone basedir\n");
> +		goto out_put;
> +	}
> +
> +	path_put(&path);
> +
> +	/* Don't inherit atime flags */
> +	mnt->mnt_flags &= ~(MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME);
> +
> +	return mnt;
> +
> +out_put:
> +	path_put(&path);
> +out:
> +	return ERR_PTR(err);
> +}
> +
> +static int cfs_fill_super(struct super_block *sb, struct fs_context *fc)
> +{
> +	struct cfs_info *fsi = sb->s_fs_info;
> +	struct vfsmount **bases = NULL;
> +	size_t numbasedirs = 0;
> +	struct inode *inode;
> +	struct vfsmount *mnt;
> +	int ret;
> +
> +	if (sb->s_root)
> +		return -EINVAL;
> +
> +	/* Set up the inode allocator early */
> +	sb->s_op = &cfs_ops;
> +	sb->s_flags |= SB_RDONLY;
> +	sb->s_magic = CFS_MAGIC;
> +	sb->s_xattr = cfs_xattr_handlers;
> +	sb->s_export_op = &cfs_export_operations;
> +
> +	if (fsi->base_path) {
> +		char *lower, *splitlower = NULL;
> +		size_t i;
> +
> +		ret = -ENOMEM;
> +		splitlower = kstrdup(fsi->base_path, GFP_KERNEL);
> +		if (!splitlower)
> +			goto fail;
> +
> +		ret = -EINVAL;
> +		numbasedirs = cfs_split_basedirs(splitlower);
> +		if (numbasedirs > CFS_MAX_STACK) {
> +			pr_err("too many lower directories, limit is %d\n",
> +			       CFS_MAX_STACK);
> +			kfree(splitlower);
> +			goto fail;
> +		}
> +
> +		ret = -ENOMEM;
> +		bases = kcalloc(numbasedirs, sizeof(struct vfsmount *),
> +				GFP_KERNEL);
> +		if (!bases) {
> +			kfree(splitlower);
> +			goto fail;
> +		}
> +
> +		lower = splitlower;
> +		for (i = 0; i < numbasedirs; i++) {
> +			mnt = resolve_basedir(lower);
> +			if (IS_ERR(mnt)) {
> +				ret = PTR_ERR(mnt);
> +				kfree(splitlower);
> +				goto fail;
> +			}
> +			bases[i] = mnt;
> +
> +			lower = strchr(lower, '\0') + 1;
> +		}
> +		kfree(splitlower);
> +	}
> +
> +	/* Must be inited before calling cfs_get_inode.  */
> +	ret = cfs_init_ctx(fc->source, fsi->has_digest ? fsi->digest : NULL,
> +			   &fsi->cfs_ctx);
> +	if (ret < 0)
> +		goto fail;
> +
> +	inode = cfs_get_root_inode(sb);
> +	if (IS_ERR(inode)) {
> +		ret = PTR_ERR(inode);
> +		goto fail;
> +	}
> +	sb->s_root = d_make_root(inode);
> +
> +	ret = -ENOMEM;
> +	if (!sb->s_root)
> +		goto fail;
> +
> +	sb->s_maxbytes = MAX_LFS_FILESIZE;
> +	sb->s_blocksize = PAGE_SIZE;
> +	sb->s_blocksize_bits = PAGE_SHIFT;
> +
> +	sb->s_time_gran = 1;
> +
> +	fsi->bases = bases;
> +	fsi->n_bases = numbasedirs;
> +	return 0;
> +fail:
> +	if (bases) {
> +		size_t i;
> +
> +		for (i = 0; i < numbasedirs; i++) {

You can now do 'for (size_t i = 0; ...'

> +			if (bases[i])
> +				kern_unmount(bases[i]);
> +		}
> +		kfree(bases);
> +	}
> +	cfs_ctx_put(&fsi->cfs_ctx);
> +	return ret;
> +}
> +
> +static int cfs_get_tree(struct fs_context *fc)
> +{
> +	return get_tree_nodev(fc, cfs_fill_super);
> +}
> +
> +static const struct fs_context_operations cfs_context_ops = {
> +	.parse_param = cfs_parse_param,
> +	.get_tree = cfs_get_tree,
> +};
> +
> +static struct file *open_base_file(struct cfs_info *fsi, struct inode *inode,
> +				   struct file *file)
> +{
> +	struct cfs_inode *cino = CFS_I(inode);
> +	struct file *real_file;
> +	char *real_path = cino->inode_data.path_payload;
> +	size_t i;
> +
> +	for (i = 0; i < fsi->n_bases; i++) {
> +		real_file = file_open_root_mnt(fsi->bases[i], real_path,
> +					       file->f_flags, 0);
> +		if (!IS_ERR(real_file) || PTR_ERR(real_file) != -ENOENT)
> +			return real_file;
> +	}
> +
> +	return ERR_PTR(-ENOENT);
> +}
> +
> +static int cfs_open_file(struct inode *inode, struct file *file)
> +{
> +	struct cfs_inode *cino = CFS_I(inode);
> +	struct cfs_info *fsi = inode->i_sb->s_fs_info;
> +	struct file *real_file;
> +	struct file *faked_file;
> +	char *real_path = cino->inode_data.path_payload;
> +
> +	if (WARN_ON(!file))
> +		return -EIO;
> +
> +	if (file->f_flags & (O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_TRUNC))
> +		return -EROFS;
> +
> +	if (!real_path) {
> +		file->private_data = &empty_file;
> +		return 0;
> +	}
> +
> +	if (fsi->verity_check >= 2 && !cino->inode_data.has_digest) {
> +		pr_warn("WARNING: composefs image file '%pd' specified no fs-verity digest\n",
> +			file->f_path.dentry);
> +		return -EIO;
> +	}
> +
> +	real_file = open_base_file(fsi, inode, file);
> +
> +	if (IS_ERR(real_file))
> +		return PTR_ERR(real_file);
> +
> +	/* If metadata records a digest for the file, ensure it is there
> +	 * and correct before using the contents.
> +	 */
> +	if (cino->inode_data.has_digest && fsi->verity_check >= 1) {
> +		u8 verity_digest[FS_VERITY_MAX_DIGEST_SIZE];
> +		enum hash_algo verity_algo;
> +		int res;
> +
> +		res = fsverity_get_digest(d_inode(real_file->f_path.dentry),
> +					  verity_digest, &verity_algo);
> +		if (res < 0) {
> +			pr_warn("WARNING: composefs backing file '%pd' has no fs-verity digest\n",
> +				real_file->f_path.dentry);
> +			fput(real_file);
> +			return -EIO;
> +		}
> +		if (verity_algo != HASH_ALGO_SHA256 ||
> +		    memcmp(cino->inode_data.digest, verity_digest,
> +			   SHA256_DIGEST_SIZE) != 0) {
> +			pr_warn("WARNING: composefs backing file '%pd' has the wrong fs-verity digest\n",
> +				real_file->f_path.dentry);
> +			fput(real_file);
> +			return -EIO;
> +		}
> +	}
> +
> +	faked_file = open_with_fake_path(&file->f_path, file->f_flags,
> +					 real_file->f_inode, current_cred());
> +	fput(real_file);
> +
> +	if (IS_ERR(faked_file))
> +		return PTR_ERR(faked_file);
> +
> +	file->private_data = faked_file;
> +	return 0;
> +}
> +
> +static unsigned long cfs_mmu_get_unmapped_area(struct file *file,
> +					       unsigned long addr,
> +					       unsigned long len,
> +					       unsigned long pgoff,
> +					       unsigned long flags)
> +{
> +	struct file *realfile = file->private_data;
> +
> +	if (realfile == &empty_file)
> +		return 0;
> +
> +	return current->mm->get_unmapped_area(file, addr, len, pgoff, flags);
> +}
> +
> +static int cfs_release_file(struct inode *inode, struct file *file)
> +{
> +	struct file *realfile = file->private_data;
> +
> +	if (WARN_ON(!realfile))
> +		return -EIO;
> +
> +	if (realfile == &empty_file)
> +		return 0;
> +
> +	fput(file->private_data);
> +
> +	return 0;
> +}
> +
> +static int cfs_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	struct file *realfile = file->private_data;
> +	int ret;
> +
> +	if (realfile == &empty_file)
> +		return 0;
> +
> +	if (!realfile->f_op->mmap)
> +		return -ENODEV;
> +
> +	if (WARN_ON(file != vma->vm_file))
> +		return -EIO;
> +
> +	vma_set_file(vma, realfile);
> +
> +	ret = call_mmap(vma->vm_file, vma);
> +
> +	return ret;
> +}
> +
> +static ssize_t cfs_read_iter(struct kiocb *iocb, struct iov_iter *iter)
> +{
> +	struct file *file = iocb->ki_filp;
> +	struct file *realfile = file->private_data;
> +	int ret;
> +
> +	if (realfile == &empty_file)
> +		return 0;
> +
> +	if (!realfile->f_op->read_iter)
> +		return -ENODEV;
> +
> +	iocb->ki_filp = realfile;
> +	ret = call_read_iter(realfile, iocb, iter);
> +	iocb->ki_filp = file;
> +
> +	return ret;
> +}
> +
> +static int cfs_fadvise(struct file *file, loff_t offset, loff_t len, int advice)
> +{
> +	struct file *realfile = file->private_data;
> +
> +	if (realfile == &empty_file)
> +		return 0;
> +
> +	return vfs_fadvise(realfile, offset, len, advice);
> +}
> +
> +static int cfs_encode_fh(struct inode *inode, u32 *fh, int *max_len,
> +			 struct inode *parent)
> +{
> +	int len = 3;
> +	u64 nodeid;
> +	u32 generation;
> +
> +	if (*max_len < len) {
> +		*max_len = len;
> +		return FILEID_INVALID;
> +	}
> +
> +	nodeid = inode->i_ino;
> +	generation = inode->i_generation;
> +
> +	fh[0] = (u32)(nodeid >> 32);
> +	fh[1] = (u32)(nodeid & 0xffffffff);
> +	fh[2] = generation;
> +
> +	*max_len = len;
> +
> +	return 0x91;
> +}
> +
> +static struct dentry *cfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
> +				       int fh_len, int fh_type)
> +{
> +	struct cfs_info *fsi = sb->s_fs_info;
> +	struct inode *ino;
> +	u64 inode_index;
> +	u32 generation;
> +
> +	if (fh_type != 0x91 || fh_len < 3)

0x91 is referenced above as well. Should this be a #define?

Brian


  reply	other threads:[~2023-01-06 12:19 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28 11:13 [PATCH RFC 0/6] Composefs: an opportunistically sharing verified image filesystem Alexander Larsson
2022-11-28 11:13 ` [PATCH 1/6] fsverity: Export fsverity_get_digest Alexander Larsson
2022-11-28 11:16 ` [PATCH 2/6] composefs: Add on-disk layout Alexander Larsson
2023-01-05 15:55   ` Brian Masney
2023-01-10 16:19     ` Alexander Larsson
2022-11-28 11:16 ` [PATCH 3/6] composefs: Add descriptor parsing code Alexander Larsson
2023-01-05 20:02   ` Brian Masney
2023-01-10 16:33     ` Alexander Larsson
2022-11-28 11:17 ` [PATCH 4/6] composefs: Add filesystem implementation Alexander Larsson
2023-01-06 12:18   ` Brian Masney [this message]
2023-01-10 16:41     ` Alexander Larsson
2022-11-28 11:17 ` [PATCH 5/6] composefs: Add documentation Alexander Larsson
2022-11-29 14:08   ` Bagas Sanjaya
2022-11-28 11:17 ` [PATCH 6/6] composefs: Add kconfig and build support Alexander Larsson
2022-11-28 14:16   ` kernel test robot
2022-11-28 17:28   ` kernel test robot
2022-11-29  7:08   ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y7gRofheB9EaR4Mi@x1 \
    --to=bmasney@redhat.com \
    --cc=alexl@redhat.com \
    --cc=gscrivan@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.