Re: [PATCH 2/5] btrfs: initial fsverity support
On Fri, Feb 05, 2021 at 10:06:07AM +0200, Nikolay Borisov wrote:
>
>
> On 5.02.21 г. 1:21 ч., Boris Burkov wrote:
> > From: Chris Mason
> >
> > Add support for fsverity in btrfs. To support the generic interface in
> > fs/verity, we add two new item types in the fs tree for inodes with
> > verity enabled. One stores the per-file verity descriptor and the other
> > stores the Merkle tree data itself.
> >
> > Verity checking is done at the end of IOs to ensure each page is checked
> > before it is marked uptodate.
> >
> > Verity relies on PageChecked for the Merkle tree data itself to avoid
> > re-walking up shared paths in the tree. For this reason, we need to
> > cache the Merkle tree data. Since the file is immutable after verity is
> > turned on, we can cache it at an index past EOF.
> >
> > Use the new inode compat_flags to store verity on the inode item, so
> > that we can enable verity on a file, then rollback to an older kernel
> > and still mount the file system and read the file.
> >
> > Signed-off-by: Chris Mason
> > ---
> > fs/btrfs/Makefile | 1 +
> > fs/btrfs/btrfs_inode.h | 1 +
> > fs/btrfs/ctree.h| 12 +-
> > fs/btrfs/extent_io.c| 5 +-
> > fs/btrfs/file.c | 6 +
> > fs/btrfs/inode.c| 28 +-
> > fs/btrfs/ioctl.c| 14 +-
> > fs/btrfs/super.c| 1 +
> > fs/btrfs/verity.c | 527
> > include/uapi/linux/btrfs_tree.h | 8 +
> > 10 files changed, 587 insertions(+), 16 deletions(-)
> > create mode 100644 fs/btrfs/verity.c
> >
>
>
>
> > diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
> > new file mode 100644
> > index ..6f3dbaee81b7
> > --- /dev/null
> > +++ b/fs/btrfs/verity.c
> > @@ -0,0 +1,527 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 Facebook. All rights reserved.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include "ctree.h"
> > +#include "btrfs_inode.h"
> > +#include "transaction.h"
> > +#include "disk-io.h"
> > +#include "locking.h"
> > +
> > +/*
> > + * Just like ext4, we cache the merkle tree in pages after EOF in the page
> > + * cache. Unlike ext4, we're storing these in dedicated btree items and
> > + * not just shoving them after EOF in the file. This means we'll need to
> > + * do extra work to encrypt them once encryption is supported in btrfs,
> > + * but btrfs has a lot of careful code around i_size and it seems better
> > + * to make a new key type than try and adjust all of our expectations
> > + * for i_size.
> > + *
> > + * fs verity items are stored under two different key types on disk.
> > + *
> > + * The descriptor items:
> > + * [ inode objectid, BTRFS_VERITY_DESC_ITEM_KEY, offset ]
> > + *
> > + * These start at offset 0 and hold the fs verity descriptor. They are
> > opaque
> > + * to btrfs, we just read and write them as a blob for the higher level
> > + * verity code. The most common size for this is 256 bytes.
> > + *
> > + * The merkle tree items:
> > + * [ inode objectid, BTRFS_VERITY_MERKLE_ITEM_KEY, offset ]
> > + *
> > + * These also start at offset 0, and correspond to the merkle tree bytes.
> > + * So when fsverity asks for page 0 of the merkle tree, we pull up one page
> > + * starting at offset 0 for this key type. These are also opaque to btrfs,
> > + * we're blindly storing whatever fsverity sends down.
> > + *
> > + * This file is just reading and writing the various items whenever
> > + * fsverity needs us to.
> > + */
>
> The description of on-disk items should ideally be documented in
> https://github.com/btrfs/btrfs-dev-docs/blob/master/tree-items.txt
>
> > +
> > +/*
> > + * drop all the items for this inode with this key_type. Before
> > + * doing a verity enable we cleanup any existing verity items.
> > + *
> > + * This is also used to clean up if a verity enable failed half way
> > + * through
> > + */
> > +static int drop_verity_items(struct btrfs_inode *inode, u8 key_type)
> > +{
>
> You should ideally be using btrfs_truncate_inode_items as it also
> implements throttling policies and keeps everything in one place. If for
> any reason that interface is not sufficient I'd rather see it refactored
> and broken down in smaller pieces than just copying stuff around, this
> just increments the maintenance burden.
>
>
>
> > +
> > +/*
> > + * helper function to insert a single item. Returns zero if all went
> > + * well
> > + */
>
> Also given that we are aiming at improving the overall state of the code
> please document each parameter properly. Also the name is somewhat
> terse. For information about the the preferred style please refer to
>
> https://btrfs.wiki.kernel.org/index.php/Development_notes#Coding_style_preferences
> and search for "Comments:"
>
> > +static int wri
Re: [PATCH 2/5] btrfs: initial fsverity support
> On Feb 5, 2021, at 3:06 AM, Nikolay Borisov wrote:
>
>> +
>> +/*
>> + * drop all the items for this inode with this key_type. Before
>> + * doing a verity enable we cleanup any existing verity items.
>> + *
>> + * This is also used to clean up if a verity enable failed half way
>> + * through
>> + */
>> +static int drop_verity_items(struct btrfs_inode *inode, u8 key_type)
>> +{
>
> You should ideally be using btrfs_truncate_inode_items as it also
> implements throttling policies and keeps everything in one place. If for
> any reason that interface is not sufficient I'd rather see it refactored
> and broken down in smaller pieces than just copying stuff around, this
> just increments the maintenance burden.
>
btrfs_truncate_inode_items is already complex, and it’s designed for things
that deal with changes to i_size. It would have to be reworked to remove the
assumption that we’re zapping unconditionally from high offsets to low.
Maybe once we’ve finalized everything else about fsverity, it makes sense to
optimize drop_verity_items and fold it into the other truncate helper, but the
function as it stands is easy to review and has no risks of adding regressions
outside of fsverity. The important part now is getting the disk format nailed
down and basic functionality right.
>> +
>> +/* desc = NULL to just sum all the item lengths */
>
> nit: typo - dest instead of desc
>
Whoops, that came from the original ext4 code and I didn’t swap the comment.
>> +
>> +/*
>> + * fsverity calls this to ask us to setup the inode for enabling. We
>> + * drop any existing verity items and set the in progress bit
>> + */
>> +static int btrfs_begin_enable_verity(struct file *filp)
>> +{
>> +struct inode *inode = file_inode(filp);
>> +int ret;
>> +
>> +if (test_bit(BTRFS_INODE_VERITY_IN_PROGRESS,
>> &BTRFS_I(inode)->runtime_flags))
>> +return -EBUSY;
>> +
>> +/*
>> + * ext4 adds the inode to the orphan list here, presumably because the
>> + * truncate done at orphan processing time will delete partial
>> + * measurements. TODO: setup orphans
>> + */
>
> Any plans when orphan support is going to be implemented?
>
I was on the fence about ignoring them completely. The space isn’t leaked, and
orphans are bad enough already. It wouldn’t be hard to make the orphan code
check for incomplete fsverity enables though.
>> +set_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &BTRFS_I(inode)->runtime_flags);
>> +ret = drop_verity_items(BTRFS_I(inode), BTRFS_VERITY_DESC_ITEM_KEY);
>> +if (ret)
>> +goto err;
>> +
>> +ret = drop_verity_items(BTRFS_I(inode), BTRFS_VERITY_MERKLE_ITEM_KEY);
>> +if (ret)
>> +goto err;
>
> A slightly higher-level question:
>
> In drop_verity_items you are doing transaction-per-item, so what happens
> during a crash and only some of the items being deleted? Is this fine,
> presumably that'd make the file unreadable? I.e what should be the
> granule of atomicity when dealing with verity items - all or nothing or
> per-item is sufficient?
Just needs to rerun the verity enable after the crash. The file won’t be half
verity enabled because the bit isn’t set until after all of the verity items
are inserted.
>
>> +
>> +return 0;
>> +
>> +err:
>> +clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS,
>> &BTRFS_I(inode)->runtime_flags);
>> +return ret;
>> +
>> +}
>> +
>> +/*
>> + * fsverity calls this when it's done with all of the pages in the file
>> + * and all of the merkle items have been inserted. We write the
>> + * descriptor and update the inode in the btree to reflect it's new life
>> + * as a verity file
>> + */
>> +static int btrfs_end_enable_verity(struct file *filp, const void *desc,
>> + size_t desc_size, u64 merkle_tree_size)
>> +{
>> +struct btrfs_trans_handle *trans;
>> +struct inode *inode = file_inode(filp);
>> +struct btrfs_root *root = BTRFS_I(inode)->root;
>> +int ret;
>> +
>> +if (desc != NULL) {
>
> Redundant check as the descriptor can never be null as per enable_verity.
>
Take a look at the rollback goto:
rollback:
inode_lock(inode);
(void)vops->end_enable_verity(filp, NULL, 0, params.tree_size);
inode_unlock(inode);
goto out
>> +/* write out the descriptor */
>> +ret = write_key_bytes(BTRFS_I(inode),
>> + BTRFS_VERITY_DESC_ITEM_KEY, 0,
>> + desc, desc_size);
>> +if (ret)
>> +goto out;
>> +
>> +/* update our inode flags to include fs verity */
>> +trans = btrfs_start_transaction(root, 1);
>> +if (IS_ERR(trans)) {
>> +ret = PTR_ERR(trans);
>> +goto out;
>> +}
>> +BTRFS_I(inode)->compat_flags |= BTRFS_INODE_VERITY;
>> +btrfs_sync_inode_flags_to_i_flags(ino
Re: [PATCH 2/5] btrfs: initial fsverity support
> On Feb 5, 2021, at 1:39 AM, Eric Biggers wrote:
>
> On Thu, Feb 04, 2021 at 03:21:38PM -0800, Boris Burkov wrote:
>> +/*
>> + * drop all the items for this inode with this key_type. Before
>> + * doing a verity enable we cleanup any existing verity items.
>> + *
>> + * This is also used to clean up if a verity enable failed half way
>> + * through
>> + */
>> +static int drop_verity_items(struct btrfs_inode *inode, u8 key_type)
>
> I assume you checked whether there's already code in btrfs that does this?
Nikolay correctly called out btrfs_truncate_inode_items(), but I wanted to keep
my fingers out of that in v0 of the patches.
> This
> seems like a fairly generic thing that might be needed elsewhere in btrfs.
> Similarly for write_key_bytes() and read_key_bytes().
>
It might make sense to make read/write_key_bytes() our generic functions, but
unless I missed something I didn’t see great fits.
>> +/*
>> + * fsverity does a two pass setup for reading the descriptor, in the first
>> pass
>> + * it calls with buf_size = 0 to query the size of the descriptor,
>> + * and then in the second pass it actually reads the descriptor off
>> + * disk.
>> + */
>> +static int btrfs_get_verity_descriptor(struct inode *inode, void *buf,
>> + size_t buf_size)
>> +{
>> +ssize_t ret = 0;
>> +
>> +if (!buf_size) {
>> +return read_key_bytes(BTRFS_I(inode),
>> + BTRFS_VERITY_DESC_ITEM_KEY,
>> + 0, NULL, (u64)-1, NULL);
>> +}
>> +
>> +ret = read_key_bytes(BTRFS_I(inode),
>> + BTRFS_VERITY_DESC_ITEM_KEY, 0,
>> + buf, buf_size, NULL);
>> +if (ret < 0)
>> +return ret;
>> +
>> +if (ret != buf_size)
>> +return -EIO;
>> +
>> +return buf_size;
>> +}
>
> This doesn't return the right value when buf_size != 0 && buf_size !=
> desc_size.
> It's supposed to return the actual size or -ERANGE, like getxattr() does; see
> the comment above fsverity_operations::get_verity_descriptor.
>
> It doesn't matter much because that case doesn't happen currently, but it
> would
> be nice to keep things consistent.
>
Forgot all about this, but I was going to suggest optimizing this part a bit,
since btrfs is doing a tree search for each call. I’d love to have a way to do
it in one search-allocate-copy round.
>> +/*
>> + * reads and caches a merkle tree page. These are stored in the btree,
>> + * but we cache them in the inode's address space after EOF.
>> + */
>> +static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
>> + pgoff_t index,
>> + unsigned long num_ra_pages)
>> +{
>> +struct page *p;
>> +u64 start = index << PAGE_SHIFT;
>> +unsigned long mapping_index;
>> +ssize_t ret;
>> +int err;
>> +
>> +/*
>> + * the file is readonly, so i_size can't change here. We jump
>> + * some pages past the last page to cache our merkles. The goal
>> + * is just to jump past any hugepages that might be mapped in.
>> + */
>> +mapping_index = (i_size_read(inode) >> PAGE_SHIFT) + 2048 + index;
>
> btrfs allows files of up to the page cache limit of MAX_LFS_FILESIZE already.
> So if the Merkle tree pages are cached past EOF like this, it would be
> necessary
> to limit the size of files that verity can be enabled on, like what ext4 and
> f2fs do. See the -EFBIG case in pagecache_write() in fs/ext4/verity.c and
> fs/f2fs/verity.c.
>
> Note that this extra limit isn't likely to be encountered in practice, as it
> would only decrease a very large limit by about 1%, and fs-verity isn't likely
> to be used on terabyte-sized files.
>
> However maybe there's a way to avoid this weirdness entirely, e.g. by
> allocating
> a temporary in-memory inode and using its address_space?
This is a good point, I think maybe just do the EFBIG dance for now? It’s not
a factor for the disk format, and we can add the separate address space any
time. For the common case today, I would prefer avoiding the overhead of
allocating the temp inode/address_space.
-chris
Re: [PATCH 2/5] btrfs: initial fsverity support
On 5.02.21 г. 1:21 ч., Boris Burkov wrote:
> From: Chris Mason
>
> Add support for fsverity in btrfs. To support the generic interface in
> fs/verity, we add two new item types in the fs tree for inodes with
> verity enabled. One stores the per-file verity descriptor and the other
> stores the Merkle tree data itself.
>
> Verity checking is done at the end of IOs to ensure each page is checked
> before it is marked uptodate.
>
> Verity relies on PageChecked for the Merkle tree data itself to avoid
> re-walking up shared paths in the tree. For this reason, we need to
> cache the Merkle tree data. Since the file is immutable after verity is
> turned on, we can cache it at an index past EOF.
>
> Use the new inode compat_flags to store verity on the inode item, so
> that we can enable verity on a file, then rollback to an older kernel
> and still mount the file system and read the file.
>
> Signed-off-by: Chris Mason
> ---
> fs/btrfs/Makefile | 1 +
> fs/btrfs/btrfs_inode.h | 1 +
> fs/btrfs/ctree.h| 12 +-
> fs/btrfs/extent_io.c| 5 +-
> fs/btrfs/file.c | 6 +
> fs/btrfs/inode.c| 28 +-
> fs/btrfs/ioctl.c| 14 +-
> fs/btrfs/super.c| 1 +
> fs/btrfs/verity.c | 527
> include/uapi/linux/btrfs_tree.h | 8 +
> 10 files changed, 587 insertions(+), 16 deletions(-)
> create mode 100644 fs/btrfs/verity.c
>
> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
> new file mode 100644
> index ..6f3dbaee81b7
> --- /dev/null
> +++ b/fs/btrfs/verity.c
> @@ -0,0 +1,527 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Facebook. All rights reserved.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "ctree.h"
> +#include "btrfs_inode.h"
> +#include "transaction.h"
> +#include "disk-io.h"
> +#include "locking.h"
> +
> +/*
> + * Just like ext4, we cache the merkle tree in pages after EOF in the page
> + * cache. Unlike ext4, we're storing these in dedicated btree items and
> + * not just shoving them after EOF in the file. This means we'll need to
> + * do extra work to encrypt them once encryption is supported in btrfs,
> + * but btrfs has a lot of careful code around i_size and it seems better
> + * to make a new key type than try and adjust all of our expectations
> + * for i_size.
> + *
> + * fs verity items are stored under two different key types on disk.
> + *
> + * The descriptor items:
> + * [ inode objectid, BTRFS_VERITY_DESC_ITEM_KEY, offset ]
> + *
> + * These start at offset 0 and hold the fs verity descriptor. They are
> opaque
> + * to btrfs, we just read and write them as a blob for the higher level
> + * verity code. The most common size for this is 256 bytes.
> + *
> + * The merkle tree items:
> + * [ inode objectid, BTRFS_VERITY_MERKLE_ITEM_KEY, offset ]
> + *
> + * These also start at offset 0, and correspond to the merkle tree bytes.
> + * So when fsverity asks for page 0 of the merkle tree, we pull up one page
> + * starting at offset 0 for this key type. These are also opaque to btrfs,
> + * we're blindly storing whatever fsverity sends down.
> + *
> + * This file is just reading and writing the various items whenever
> + * fsverity needs us to.
> + */
The description of on-disk items should ideally be documented in
https://github.com/btrfs/btrfs-dev-docs/blob/master/tree-items.txt
> +
> +/*
> + * drop all the items for this inode with this key_type. Before
> + * doing a verity enable we cleanup any existing verity items.
> + *
> + * This is also used to clean up if a verity enable failed half way
> + * through
> + */
> +static int drop_verity_items(struct btrfs_inode *inode, u8 key_type)
> +{
You should ideally be using btrfs_truncate_inode_items as it also
implements throttling policies and keeps everything in one place. If for
any reason that interface is not sufficient I'd rather see it refactored
and broken down in smaller pieces than just copying stuff around, this
just increments the maintenance burden.
> +
> +/*
> + * helper function to insert a single item. Returns zero if all went
> + * well
> + */
Also given that we are aiming at improving the overall state of the code
please document each parameter properly. Also the name is somewhat
terse. For information about the the preferred style please refer to
https://btrfs.wiki.kernel.org/index.php/Development_notes#Coding_style_preferences
and search for "Comments:"
> +static int write_key_bytes(struct btrfs_inode *inode, u8 key_type, u64
> offset,
> +const char *src, u64 len)
This function should be moved to inode-item.c as it seems generic
enough. SOmething like write_inode_generic_bytes or something like that.
> +
> +/*
> + * helper function to read items from the btree. This
Re: [PATCH 2/5] btrfs: initial fsverity support
On Thu, Feb 04, 2021 at 03:21:38PM -0800, Boris Burkov wrote:
> +/*
> + * drop all the items for this inode with this key_type. Before
> + * doing a verity enable we cleanup any existing verity items.
> + *
> + * This is also used to clean up if a verity enable failed half way
> + * through
> + */
> +static int drop_verity_items(struct btrfs_inode *inode, u8 key_type)
I assume you checked whether there's already code in btrfs that does this? This
seems like a fairly generic thing that might be needed elsewhere in btrfs.
Similarly for write_key_bytes() and read_key_bytes().
> +/*
> + * fsverity does a two pass setup for reading the descriptor, in the first
> pass
> + * it calls with buf_size = 0 to query the size of the descriptor,
> + * and then in the second pass it actually reads the descriptor off
> + * disk.
> + */
> +static int btrfs_get_verity_descriptor(struct inode *inode, void *buf,
> +size_t buf_size)
> +{
> + ssize_t ret = 0;
> +
> + if (!buf_size) {
> + return read_key_bytes(BTRFS_I(inode),
> + BTRFS_VERITY_DESC_ITEM_KEY,
> + 0, NULL, (u64)-1, NULL);
> + }
> +
> + ret = read_key_bytes(BTRFS_I(inode),
> + BTRFS_VERITY_DESC_ITEM_KEY, 0,
> + buf, buf_size, NULL);
> + if (ret < 0)
> + return ret;
> +
> + if (ret != buf_size)
> + return -EIO;
> +
> + return buf_size;
> +}
This doesn't return the right value when buf_size != 0 && buf_size != desc_size.
It's supposed to return the actual size or -ERANGE, like getxattr() does; see
the comment above fsverity_operations::get_verity_descriptor.
It doesn't matter much because that case doesn't happen currently, but it would
be nice to keep things consistent.
> +/*
> + * reads and caches a merkle tree page. These are stored in the btree,
> + * but we cache them in the inode's address space after EOF.
> + */
> +static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
> +pgoff_t index,
> +unsigned long num_ra_pages)
> +{
> + struct page *p;
> + u64 start = index << PAGE_SHIFT;
> + unsigned long mapping_index;
> + ssize_t ret;
> + int err;
> +
> + /*
> + * the file is readonly, so i_size can't change here. We jump
> + * some pages past the last page to cache our merkles. The goal
> + * is just to jump past any hugepages that might be mapped in.
> + */
> + mapping_index = (i_size_read(inode) >> PAGE_SHIFT) + 2048 + index;
btrfs allows files of up to the page cache limit of MAX_LFS_FILESIZE already.
So if the Merkle tree pages are cached past EOF like this, it would be necessary
to limit the size of files that verity can be enabled on, like what ext4 and
f2fs do. See the -EFBIG case in pagecache_write() in fs/ext4/verity.c and
fs/f2fs/verity.c.
Note that this extra limit isn't likely to be encountered in practice, as it
would only decrease a very large limit by about 1%, and fs-verity isn't likely
to be used on terabyte-sized files.
However maybe there's a way to avoid this weirdness entirely, e.g. by allocating
a temporary in-memory inode and using its address_space?
- Eric
Re: [PATCH 2/5] btrfs: initial fsverity support
Hi Boris,
I love your patch! Perhaps something to improve:
[auto build test WARNING on v5.11-rc6]
[also build test WARNING on next-20210125]
[cannot apply to kdave/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Boris-Burkov/btrfs-support-fsverity/20210205-072745
base:1048ba83fb1c00cd24172e23e8263972f6b5d9ac
config: x86_64-randconfig-a002-20210204 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
c9439ca36342fb6013187d0a69aef92736951476)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
#
https://github.com/0day-ci/linux/commit/4fb68eb17c9ed350a759646451cba99a19ea7579
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Boris-Burkov/btrfs-support-fsverity/20210205-072745
git checkout 4fb68eb17c9ed350a759646451cba99a19ea7579
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> fs/btrfs/verity.c:370:6: warning: variable 'ret' is used uninitialized
>> whenever 'if' condition is false [-Wsometimes-uninitialized]
if (desc != NULL) {
^~~~
fs/btrfs/verity.c:397:9: note: uninitialized use occurs here
return ret;
^~~
fs/btrfs/verity.c:370:2: note: remove the 'if' if its condition is always
true
if (desc != NULL) {
^~
fs/btrfs/verity.c:368:9: note: initialize the variable 'ret' to silence this
warning
int ret;
^
= 0
1 warning generated.
vim +370 fs/btrfs/verity.c
355
356 /*
357 * fsverity calls this when it's done with all of the pages in the file
358 * and all of the merkle items have been inserted. We write the
359 * descriptor and update the inode in the btree to reflect it's new life
360 * as a verity file
361 */
362 static int btrfs_end_enable_verity(struct file *filp, const void *desc,
363size_t desc_size, u64
merkle_tree_size)
364 {
365 struct btrfs_trans_handle *trans;
366 struct inode *inode = file_inode(filp);
367 struct btrfs_root *root = BTRFS_I(inode)->root;
368 int ret;
369
> 370 if (desc != NULL) {
371 /* write out the descriptor */
372 ret = write_key_bytes(BTRFS_I(inode),
373BTRFS_VERITY_DESC_ITEM_KEY, 0,
374desc, desc_size);
375 if (ret)
376 goto out;
377
378 /* update our inode flags to include fs verity */
379 trans = btrfs_start_transaction(root, 1);
380 if (IS_ERR(trans)) {
381 ret = PTR_ERR(trans);
382 goto out;
383 }
384 BTRFS_I(inode)->compat_flags |= BTRFS_INODE_VERITY;
385 btrfs_sync_inode_flags_to_i_flags(inode);
386 ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
387 btrfs_end_transaction(trans);
388 }
389
390 out:
391 if (desc == NULL || ret) {
392 /* If we failed, drop all the verity items */
393 drop_verity_items(BTRFS_I(inode),
BTRFS_VERITY_DESC_ITEM_KEY);
394 drop_verity_items(BTRFS_I(inode),
BTRFS_VERITY_MERKLE_ITEM_KEY);
395 }
396 clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS,
&BTRFS_I(inode)->runtime_flags);
397 return ret;
398 }
399
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH 2/5] btrfs: initial fsverity support
Hi Boris,
I love your patch! Yet something to improve:
[auto build test ERROR on v5.11-rc6]
[also build test ERROR on next-20210125]
[cannot apply to kdave/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Boris-Burkov/btrfs-support-fsverity/20210205-072745
base:1048ba83fb1c00cd24172e23e8263972f6b5d9ac
config: arc-randconfig-r004-20210204 (attached as .config)
compiler: arc-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/4fb68eb17c9ed350a759646451cba99a19ea7579
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Boris-Burkov/btrfs-support-fsverity/20210205-072745
git checkout 4fb68eb17c9ed350a759646451cba99a19ea7579
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
fs/btrfs/super.c: In function 'btrfs_fill_super':
>> fs/btrfs/super.c:1343:6: error: 'struct super_block' has no member named
>> 's_vop'; did you mean 's_op'?
1343 | sb->s_vop = &btrfs_verityops;
| ^
| s_op
vim +1343 fs/btrfs/super.c
1329
1330 static int btrfs_fill_super(struct super_block *sb,
1331 struct btrfs_fs_devices *fs_devices,
1332 void *data)
1333 {
1334 struct inode *inode;
1335 struct btrfs_fs_info *fs_info = btrfs_sb(sb);
1336 int err;
1337
1338 sb->s_maxbytes = MAX_LFS_FILESIZE;
1339 sb->s_magic = BTRFS_SUPER_MAGIC;
1340 sb->s_op = &btrfs_super_ops;
1341 sb->s_d_op = &btrfs_dentry_operations;
1342 sb->s_export_op = &btrfs_export_ops;
> 1343 sb->s_vop = &btrfs_verityops;
1344 sb->s_xattr = btrfs_xattr_handlers;
1345 sb->s_time_gran = 1;
1346 #ifdef CONFIG_BTRFS_FS_POSIX_ACL
1347 sb->s_flags |= SB_POSIXACL;
1348 #endif
1349 sb->s_flags |= SB_I_VERSION;
1350 sb->s_iflags |= SB_I_CGROUPWB;
1351
1352 err = super_setup_bdi(sb);
1353 if (err) {
1354 btrfs_err(fs_info, "super_setup_bdi failed");
1355 return err;
1356 }
1357
1358 err = open_ctree(sb, fs_devices, (char *)data);
1359 if (err) {
1360 btrfs_err(fs_info, "open_ctree failed");
1361 return err;
1362 }
1363
1364 inode = btrfs_iget(sb, BTRFS_FIRST_FREE_OBJECTID,
fs_info->fs_root);
1365 if (IS_ERR(inode)) {
1366 err = PTR_ERR(inode);
1367 goto fail_close;
1368 }
1369
1370 sb->s_root = d_make_root(inode);
1371 if (!sb->s_root) {
1372 err = -ENOMEM;
1373 goto fail_close;
1374 }
1375
1376 cleancache_init_fs(sb);
1377 sb->s_flags |= SB_ACTIVE;
1378 return 0;
1379
1380 fail_close:
1381 close_ctree(fs_info);
1382 return err;
1383 }
1384
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH 2/5] btrfs: initial fsverity support
Hi Boris,
I love your patch! Yet something to improve:
[auto build test ERROR on v5.11-rc6]
[also build test ERROR on next-20210125]
[cannot apply to kdave/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Boris-Burkov/btrfs-support-fsverity/20210205-072745
base:1048ba83fb1c00cd24172e23e8263972f6b5d9ac
config: x86_64-randconfig-a006-20210204 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
c9439ca36342fb6013187d0a69aef92736951476)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
#
https://github.com/0day-ci/linux/commit/4fb68eb17c9ed350a759646451cba99a19ea7579
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Boris-Burkov/btrfs-support-fsverity/20210205-072745
git checkout 4fb68eb17c9ed350a759646451cba99a19ea7579
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> fs/btrfs/super.c:1343:6: error: no member named 's_vop' in 'struct
>> super_block'
sb->s_vop = &btrfs_verityops;
~~ ^
1 error generated.
vim +1343 fs/btrfs/super.c
1329
1330 static int btrfs_fill_super(struct super_block *sb,
1331 struct btrfs_fs_devices *fs_devices,
1332 void *data)
1333 {
1334 struct inode *inode;
1335 struct btrfs_fs_info *fs_info = btrfs_sb(sb);
1336 int err;
1337
1338 sb->s_maxbytes = MAX_LFS_FILESIZE;
1339 sb->s_magic = BTRFS_SUPER_MAGIC;
1340 sb->s_op = &btrfs_super_ops;
1341 sb->s_d_op = &btrfs_dentry_operations;
1342 sb->s_export_op = &btrfs_export_ops;
> 1343 sb->s_vop = &btrfs_verityops;
1344 sb->s_xattr = btrfs_xattr_handlers;
1345 sb->s_time_gran = 1;
1346 #ifdef CONFIG_BTRFS_FS_POSIX_ACL
1347 sb->s_flags |= SB_POSIXACL;
1348 #endif
1349 sb->s_flags |= SB_I_VERSION;
1350 sb->s_iflags |= SB_I_CGROUPWB;
1351
1352 err = super_setup_bdi(sb);
1353 if (err) {
1354 btrfs_err(fs_info, "super_setup_bdi failed");
1355 return err;
1356 }
1357
1358 err = open_ctree(sb, fs_devices, (char *)data);
1359 if (err) {
1360 btrfs_err(fs_info, "open_ctree failed");
1361 return err;
1362 }
1363
1364 inode = btrfs_iget(sb, BTRFS_FIRST_FREE_OBJECTID,
fs_info->fs_root);
1365 if (IS_ERR(inode)) {
1366 err = PTR_ERR(inode);
1367 goto fail_close;
1368 }
1369
1370 sb->s_root = d_make_root(inode);
1371 if (!sb->s_root) {
1372 err = -ENOMEM;
1373 goto fail_close;
1374 }
1375
1376 cleancache_init_fs(sb);
1377 sb->s_flags |= SB_ACTIVE;
1378 return 0;
1379
1380 fail_close:
1381 close_ctree(fs_info);
1382 return err;
1383 }
1384
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
