All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Burkov <boris@bur.io>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: kbuild@lists.01.org, linux-btrfs@vger.kernel.org,
	linux-fscrypt@vger.kernel.org, kernel-team@fb.com, lkp@intel.com,
	kbuild-all@lists.01.org
Subject: Re: [PATCH v5 2/3] btrfs: initial fsverity support
Date: Wed, 30 Jun 2021 12:42:11 -0700	[thread overview]
Message-ID: <YNzJBAOomwEiNgDn@zen> (raw)
In-Reply-To: <202106260148.y7YIYV3M-lkp@intel.com>

On Mon, Jun 28, 2021 at 11:23:51AM +0300, Dan Carpenter wrote:
> Hi Boris,
> 
> url:    https://github.com/0day-ci/linux/commits/Boris-Burkov/btrfs-support-fsverity/20210625-064209
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
> config: parisc-randconfig-m031-20210625 (attached as .config)
> compiler: hppa-linux-gcc (GCC) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> New smatch warnings:
> fs/btrfs/verity.c:268 write_key_bytes() error: uninitialized symbol 'ret'.
> fs/btrfs/verity.c:745 btrfs_write_merkle_tree_block() warn: should '1 << log_blocksize' be a 64 bit type?

good catch :)

> 
> Old smatch warnings:
> fs/btrfs/verity.c:552 btrfs_begin_enable_verity() error: uninitialized symbol 'trans'.

This was a screw up from re-arranging the patches, will clean up.

> 
> vim +/ret +268 fs/btrfs/verity.c
> 
> 24749321fc3abc Boris Burkov 2021-06-24  209  static int write_key_bytes(struct btrfs_inode *inode, u8 key_type, u64 offset,
> 24749321fc3abc Boris Burkov 2021-06-24  210  			   const char *src, u64 len)
> 24749321fc3abc Boris Burkov 2021-06-24  211  {
> 24749321fc3abc Boris Burkov 2021-06-24  212  	struct btrfs_trans_handle *trans;
> 24749321fc3abc Boris Burkov 2021-06-24  213  	struct btrfs_path *path;
> 24749321fc3abc Boris Burkov 2021-06-24  214  	struct btrfs_root *root = inode->root;
> 24749321fc3abc Boris Burkov 2021-06-24  215  	struct extent_buffer *leaf;
> 24749321fc3abc Boris Burkov 2021-06-24  216  	struct btrfs_key key;
> 24749321fc3abc Boris Burkov 2021-06-24  217  	u64 copied = 0;
> 24749321fc3abc Boris Burkov 2021-06-24  218  	unsigned long copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  219  	unsigned long src_offset = 0;
> 24749321fc3abc Boris Burkov 2021-06-24  220  	void *data;
> 24749321fc3abc Boris Burkov 2021-06-24  221  	int ret;
> 24749321fc3abc Boris Burkov 2021-06-24  222  
> 24749321fc3abc Boris Burkov 2021-06-24  223  	path = btrfs_alloc_path();
> 24749321fc3abc Boris Burkov 2021-06-24  224  	if (!path)
> 24749321fc3abc Boris Burkov 2021-06-24  225  		return -ENOMEM;
> 24749321fc3abc Boris Burkov 2021-06-24  226  
> 24749321fc3abc Boris Burkov 2021-06-24  227  	while (len > 0) {
> 
> Can we write zero bytes?  My test system has linux-next so I don't know.
> I don't think the kbuild bot uses the cross function DB so it doesn't
> know either.

Considering the three callsites and then the callsite of
end_enable_verity in fs/verity/enable.c, I don't think it's possible to
call write_key_bytes with len==0, but it doesn't hurt to fix it anyway,
just in case. It makes write_key_bytes more correct as documented, at
least.

> 
> 24749321fc3abc Boris Burkov 2021-06-24  228  		/*
> 24749321fc3abc Boris Burkov 2021-06-24  229  		 * 1 for the new item being inserted
> 24749321fc3abc Boris Burkov 2021-06-24  230  		 */
> 24749321fc3abc Boris Burkov 2021-06-24  231  		trans = btrfs_start_transaction(root, 1);
> 24749321fc3abc Boris Burkov 2021-06-24  232  		if (IS_ERR(trans)) {
> 24749321fc3abc Boris Burkov 2021-06-24  233  			ret = PTR_ERR(trans);
> 24749321fc3abc Boris Burkov 2021-06-24  234  			break;
> 24749321fc3abc Boris Burkov 2021-06-24  235  		}
> 24749321fc3abc Boris Burkov 2021-06-24  236  
> 24749321fc3abc Boris Burkov 2021-06-24  237  		key.objectid = btrfs_ino(inode);
> 24749321fc3abc Boris Burkov 2021-06-24  238  		key.type = key_type;
> 24749321fc3abc Boris Burkov 2021-06-24  239  		key.offset = offset;
> 24749321fc3abc Boris Burkov 2021-06-24  240  
> 24749321fc3abc Boris Burkov 2021-06-24  241  		/*
> 24749321fc3abc Boris Burkov 2021-06-24  242  		 * Insert 2K at a time mostly to be friendly for smaller
> 24749321fc3abc Boris Burkov 2021-06-24  243  		 * leaf size filesystems
> 24749321fc3abc Boris Burkov 2021-06-24  244  		 */
> 24749321fc3abc Boris Burkov 2021-06-24  245  		copy_bytes = min_t(u64, len, 2048);
> 24749321fc3abc Boris Burkov 2021-06-24  246  
> 24749321fc3abc Boris Burkov 2021-06-24  247  		ret = btrfs_insert_empty_item(trans, root, path, &key, copy_bytes);
> 24749321fc3abc Boris Burkov 2021-06-24  248  		if (ret) {
> 24749321fc3abc Boris Burkov 2021-06-24  249  			btrfs_end_transaction(trans);
> 24749321fc3abc Boris Burkov 2021-06-24  250  			break;
> 24749321fc3abc Boris Burkov 2021-06-24  251  		}
> 24749321fc3abc Boris Burkov 2021-06-24  252  
> 24749321fc3abc Boris Burkov 2021-06-24  253  		leaf = path->nodes[0];
> 24749321fc3abc Boris Burkov 2021-06-24  254  
> 24749321fc3abc Boris Burkov 2021-06-24  255  		data = btrfs_item_ptr(leaf, path->slots[0], void);
> 24749321fc3abc Boris Burkov 2021-06-24  256  		write_extent_buffer(leaf, src + src_offset,
> 24749321fc3abc Boris Burkov 2021-06-24  257  				    (unsigned long)data, copy_bytes);
> 24749321fc3abc Boris Burkov 2021-06-24  258  		offset += copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  259  		src_offset += copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  260  		len -= copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  261  		copied += copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  262  
> 24749321fc3abc Boris Burkov 2021-06-24  263  		btrfs_release_path(path);
> 24749321fc3abc Boris Burkov 2021-06-24  264  		btrfs_end_transaction(trans);
> 24749321fc3abc Boris Burkov 2021-06-24  265  	}
> 24749321fc3abc Boris Burkov 2021-06-24  266  
> 24749321fc3abc Boris Burkov 2021-06-24  267  	btrfs_free_path(path);
> 24749321fc3abc Boris Burkov 2021-06-24 @268  	return ret;
> 24749321fc3abc Boris Burkov 2021-06-24  269  }
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 

WARNING: multiple messages have this Message-ID (diff)
From: Boris Burkov <boris@bur.io>
To: kbuild-all@lists.01.org
Subject: Re: [PATCH v5 2/3] btrfs: initial fsverity support
Date: Wed, 30 Jun 2021 12:42:11 -0700	[thread overview]
Message-ID: <YNzJBAOomwEiNgDn@zen> (raw)
In-Reply-To: <202106260148.y7YIYV3M-lkp@intel.com>

[-- Attachment #1: Type: text/plain, Size: 5852 bytes --]

On Mon, Jun 28, 2021 at 11:23:51AM +0300, Dan Carpenter wrote:
> Hi Boris,
> 
> url:    https://github.com/0day-ci/linux/commits/Boris-Burkov/btrfs-support-fsverity/20210625-064209
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
> config: parisc-randconfig-m031-20210625 (attached as .config)
> compiler: hppa-linux-gcc (GCC) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> New smatch warnings:
> fs/btrfs/verity.c:268 write_key_bytes() error: uninitialized symbol 'ret'.
> fs/btrfs/verity.c:745 btrfs_write_merkle_tree_block() warn: should '1 << log_blocksize' be a 64 bit type?

good catch :)

> 
> Old smatch warnings:
> fs/btrfs/verity.c:552 btrfs_begin_enable_verity() error: uninitialized symbol 'trans'.

This was a screw up from re-arranging the patches, will clean up.

> 
> vim +/ret +268 fs/btrfs/verity.c
> 
> 24749321fc3abc Boris Burkov 2021-06-24  209  static int write_key_bytes(struct btrfs_inode *inode, u8 key_type, u64 offset,
> 24749321fc3abc Boris Burkov 2021-06-24  210  			   const char *src, u64 len)
> 24749321fc3abc Boris Burkov 2021-06-24  211  {
> 24749321fc3abc Boris Burkov 2021-06-24  212  	struct btrfs_trans_handle *trans;
> 24749321fc3abc Boris Burkov 2021-06-24  213  	struct btrfs_path *path;
> 24749321fc3abc Boris Burkov 2021-06-24  214  	struct btrfs_root *root = inode->root;
> 24749321fc3abc Boris Burkov 2021-06-24  215  	struct extent_buffer *leaf;
> 24749321fc3abc Boris Burkov 2021-06-24  216  	struct btrfs_key key;
> 24749321fc3abc Boris Burkov 2021-06-24  217  	u64 copied = 0;
> 24749321fc3abc Boris Burkov 2021-06-24  218  	unsigned long copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  219  	unsigned long src_offset = 0;
> 24749321fc3abc Boris Burkov 2021-06-24  220  	void *data;
> 24749321fc3abc Boris Burkov 2021-06-24  221  	int ret;
> 24749321fc3abc Boris Burkov 2021-06-24  222  
> 24749321fc3abc Boris Burkov 2021-06-24  223  	path = btrfs_alloc_path();
> 24749321fc3abc Boris Burkov 2021-06-24  224  	if (!path)
> 24749321fc3abc Boris Burkov 2021-06-24  225  		return -ENOMEM;
> 24749321fc3abc Boris Burkov 2021-06-24  226  
> 24749321fc3abc Boris Burkov 2021-06-24  227  	while (len > 0) {
> 
> Can we write zero bytes?  My test system has linux-next so I don't know.
> I don't think the kbuild bot uses the cross function DB so it doesn't
> know either.

Considering the three callsites and then the callsite of
end_enable_verity in fs/verity/enable.c, I don't think it's possible to
call write_key_bytes with len==0, but it doesn't hurt to fix it anyway,
just in case. It makes write_key_bytes more correct as documented, at
least.

> 
> 24749321fc3abc Boris Burkov 2021-06-24  228  		/*
> 24749321fc3abc Boris Burkov 2021-06-24  229  		 * 1 for the new item being inserted
> 24749321fc3abc Boris Burkov 2021-06-24  230  		 */
> 24749321fc3abc Boris Burkov 2021-06-24  231  		trans = btrfs_start_transaction(root, 1);
> 24749321fc3abc Boris Burkov 2021-06-24  232  		if (IS_ERR(trans)) {
> 24749321fc3abc Boris Burkov 2021-06-24  233  			ret = PTR_ERR(trans);
> 24749321fc3abc Boris Burkov 2021-06-24  234  			break;
> 24749321fc3abc Boris Burkov 2021-06-24  235  		}
> 24749321fc3abc Boris Burkov 2021-06-24  236  
> 24749321fc3abc Boris Burkov 2021-06-24  237  		key.objectid = btrfs_ino(inode);
> 24749321fc3abc Boris Burkov 2021-06-24  238  		key.type = key_type;
> 24749321fc3abc Boris Burkov 2021-06-24  239  		key.offset = offset;
> 24749321fc3abc Boris Burkov 2021-06-24  240  
> 24749321fc3abc Boris Burkov 2021-06-24  241  		/*
> 24749321fc3abc Boris Burkov 2021-06-24  242  		 * Insert 2K at a time mostly to be friendly for smaller
> 24749321fc3abc Boris Burkov 2021-06-24  243  		 * leaf size filesystems
> 24749321fc3abc Boris Burkov 2021-06-24  244  		 */
> 24749321fc3abc Boris Burkov 2021-06-24  245  		copy_bytes = min_t(u64, len, 2048);
> 24749321fc3abc Boris Burkov 2021-06-24  246  
> 24749321fc3abc Boris Burkov 2021-06-24  247  		ret = btrfs_insert_empty_item(trans, root, path, &key, copy_bytes);
> 24749321fc3abc Boris Burkov 2021-06-24  248  		if (ret) {
> 24749321fc3abc Boris Burkov 2021-06-24  249  			btrfs_end_transaction(trans);
> 24749321fc3abc Boris Burkov 2021-06-24  250  			break;
> 24749321fc3abc Boris Burkov 2021-06-24  251  		}
> 24749321fc3abc Boris Burkov 2021-06-24  252  
> 24749321fc3abc Boris Burkov 2021-06-24  253  		leaf = path->nodes[0];
> 24749321fc3abc Boris Burkov 2021-06-24  254  
> 24749321fc3abc Boris Burkov 2021-06-24  255  		data = btrfs_item_ptr(leaf, path->slots[0], void);
> 24749321fc3abc Boris Burkov 2021-06-24  256  		write_extent_buffer(leaf, src + src_offset,
> 24749321fc3abc Boris Burkov 2021-06-24  257  				    (unsigned long)data, copy_bytes);
> 24749321fc3abc Boris Burkov 2021-06-24  258  		offset += copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  259  		src_offset += copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  260  		len -= copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  261  		copied += copy_bytes;
> 24749321fc3abc Boris Burkov 2021-06-24  262  
> 24749321fc3abc Boris Burkov 2021-06-24  263  		btrfs_release_path(path);
> 24749321fc3abc Boris Burkov 2021-06-24  264  		btrfs_end_transaction(trans);
> 24749321fc3abc Boris Burkov 2021-06-24  265  	}
> 24749321fc3abc Boris Burkov 2021-06-24  266  
> 24749321fc3abc Boris Burkov 2021-06-24  267  	btrfs_free_path(path);
> 24749321fc3abc Boris Burkov 2021-06-24 @268  	return ret;
> 24749321fc3abc Boris Burkov 2021-06-24  269  }
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
> 

  reply	other threads:[~2021-06-30 19:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24 22:41 [PATCH v5 0/3] btrfs: support fsverity Boris Burkov
2021-06-24 22:41 ` [PATCH v5 1/3] btrfs: add ro compat flags to inodes Boris Burkov
2021-06-24 22:41 ` [PATCH v5 2/3] btrfs: initial fsverity support Boris Burkov
2021-06-25  4:50   ` kernel test robot
2021-06-25  4:50     ` kernel test robot
2021-06-25 17:59   ` kernel test robot
2021-06-28  8:23     ` Dan Carpenter
2021-06-28  8:23     ` Dan Carpenter
2021-06-30 19:42     ` Boris Burkov [this message]
2021-06-30 19:42       ` Boris Burkov
2021-06-24 22:41 ` [PATCH v5 3/3] btrfs: verity metadata orphan items Boris Burkov

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=YNzJBAOomwEiNgDn@zen \
    --to=boris@bur.io \
    --cc=dan.carpenter@oracle.com \
    --cc=kbuild-all@lists.01.org \
    --cc=kbuild@lists.01.org \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=lkp@intel.com \
    /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.