All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/13] Add support for other checksums
@ 2019-05-22  8:18 Johannes Thumshirn
  2019-05-22  8:18 ` [PATCH v3 01/13] btrfs: use btrfs_csum_data() instead of directly calling crc32c Johannes Thumshirn
                   ` (13 more replies)
  0 siblings, 14 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:18 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

This patchset add support for adding new checksum types in BTRFS.

Currently BTRFS only supports CRC32C as data and metadata checksum, which is
good if you only want to detect errors due to data corruption in hardware.

But CRC32C isn't able cover other use-cases like de-duplication or
cryptographically save data integrity guarantees.

The following properties made SHA-256 interesting for these use-cases:
- Still considered cryptographically sound
- Reasonably well understood by the security industry
- Result fits into the 32Byte/256Bit we have for the checksum in the on-disk
  format
- Small enough collision space to make it feasible for data de-duplication
- Fast enough to calculate and offloadable to crypto hardware via the kernel's
  crypto_shash framework.

The patchset also provides mechanisms for plumbing in different hash
algorithms relatively easy.

Unfortunately this patchset also partially reverts commit: 
9678c54388b6 ("btrfs: Remove custom crc32c init code")

This is an intermediate submission, as a) mkfs.btrfs support is still missing
and b) David requested to have three hash algorithms, where 1 is crc32c, one
cryptographically secure and one in between.

A changelog can be found directly in the patches. The branch is also available
on a gitweb at
https://git.kernel.org/pub/scm/linux/kernel/git/jth/linux.git/log/?h=btrfs-csum-rework.v3

Johannes Thumshirn (13):
  btrfs: use btrfs_csum_data() instead of directly calling crc32c
  btrfs: resurrect btrfs_crc32c()
  btrfs: use btrfs_crc32c{,_final}() in for free space cache
  btrfs: don't assume ordered sums to be 4 bytes
  btrfs: dont assume compressed_bio sums to be 4 bytes
  btrfs: format checksums according to type for printing
  btrfs: add common checksum type validation
  btrfs: check for supported superblock checksum type before checksum
    validation
  btrfs: Simplify btrfs_check_super_csum() and get rid of size
    assumptions
  btrfs: add boilerplate code for directly including the crypto
    framework
  btrfs: directly call into crypto framework for checsumming
  btrfs: remove assumption about csum type form
    btrfs_print_data_csum_error()
  btrfs: add sha256 as another checksum algorithm

 fs/btrfs/Kconfig                |   4 +-
 fs/btrfs/btrfs_inode.h          |  33 +++++++--
 fs/btrfs/check-integrity.c      |  12 ++--
 fs/btrfs/compression.c          |  40 +++++++----
 fs/btrfs/compression.h          |   2 +-
 fs/btrfs/ctree.h                |  27 +++++++-
 fs/btrfs/disk-io.c              | 146 ++++++++++++++++++++++++++--------------
 fs/btrfs/disk-io.h              |   2 -
 fs/btrfs/extent-tree.c          |   6 +-
 fs/btrfs/file-item.c            |  44 +++++++-----
 fs/btrfs/free-space-cache.c     |  10 ++-
 fs/btrfs/inode.c                |  20 ++++--
 fs/btrfs/ordered-data.c         |  10 +--
 fs/btrfs/ordered-data.h         |   4 +-
 fs/btrfs/scrub.c                |  39 ++++++++---
 fs/btrfs/send.c                 |   2 +-
 fs/btrfs/super.c                |   2 +
 include/uapi/linux/btrfs_tree.h |   6 +-
 18 files changed, 280 insertions(+), 129 deletions(-)

-- 
2.16.4


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 01/13] btrfs: use btrfs_csum_data() instead of directly calling crc32c
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
@ 2019-05-22  8:18 ` Johannes Thumshirn
  2019-05-22  8:18 ` [PATCH v3 02/13] btrfs: resurrect btrfs_crc32c() Johannes Thumshirn
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:18 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

btrfsic_test_for_metadata() directly calls the crc32c() library function
for calculating the CRC32C checksum, but then uses btrfs_csum_final() to
invert the result.

To ease further refactoring and development around checksumming in BTRFS
convert to calling btrfs_csum_data(), which is a wrapper around crc32c().

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/check-integrity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index b0c8094528d1..85774e2fa3e5 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -1728,7 +1728,7 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
 		size_t sublen = i ? PAGE_SIZE :
 				    (PAGE_SIZE - BTRFS_CSUM_SIZE);
 
-		crc = crc32c(crc, data, sublen);
+		crc = btrfs_csum_data(data, crc, sublen);
 	}
 	btrfs_csum_final(crc, csum);
 	if (memcmp(csum, h->csum, state->csum_size))
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 02/13] btrfs: resurrect btrfs_crc32c()
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
  2019-05-22  8:18 ` [PATCH v3 01/13] btrfs: use btrfs_csum_data() instead of directly calling crc32c Johannes Thumshirn
@ 2019-05-22  8:18 ` Johannes Thumshirn
  2019-05-22  8:19 ` [PATCH v3 03/13] btrfs: use btrfs_crc32c{,_final}() in for free space cache Johannes Thumshirn
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:18 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

Commit 9678c54388b6 ("btrfs: Remove custom crc32c init code") removed the
btrfs_crc32c() function, because it was a duplicate of the crc32c() library
function we already have in the kernel.

Resurrect it as a shim wrapper over crc32c() to make following
transformations of the checksumming code in btrfs easier.

Also provide a btrfs_crc32_final() to ease following transformations.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h       | 12 ++++++++++++
 fs/btrfs/extent-tree.c |  6 +++---
 fs/btrfs/send.c        |  2 +-
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index b81c331b28fa..d85541f13f65 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -19,6 +19,7 @@
 #include <linux/kobject.h>
 #include <trace/events/btrfs.h>
 #include <asm/kmap_types.h>
+#include <asm/unaligned.h>
 #include <linux/pagemap.h>
 #include <linux/btrfs.h>
 #include <linux/btrfs_tree.h>
@@ -2642,6 +2643,17 @@ BTRFS_SETGET_STACK_FUNCS(stack_dev_replace_cursor_right,
 	((unsigned long)(BTRFS_LEAF_DATA_OFFSET + \
 	btrfs_item_offset_nr(leaf, slot)))
 
+
+static inline u32 btrfs_crc32c(u32 crc, const void *address, unsigned length)
+{
+	return crc32c(crc, address, length);
+}
+
+static inline void btrfs_crc32c_final(u32 crc, u8 *result)
+{
+	put_unaligned_le32(~crc, result);
+}
+
 static inline u64 btrfs_name_hash(const char *name, int len)
 {
        return crc32c((u32)~1, name, len);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index f79e477a378e..06a30f2cd2e0 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1119,11 +1119,11 @@ static u64 hash_extent_data_ref(u64 root_objectid, u64 owner, u64 offset)
 	__le64 lenum;
 
 	lenum = cpu_to_le64(root_objectid);
-	high_crc = crc32c(high_crc, &lenum, sizeof(lenum));
+	high_crc = btrfs_crc32c(high_crc, &lenum, sizeof(lenum));
 	lenum = cpu_to_le64(owner);
-	low_crc = crc32c(low_crc, &lenum, sizeof(lenum));
+	low_crc = btrfs_crc32c(low_crc, &lenum, sizeof(lenum));
 	lenum = cpu_to_le64(offset);
-	low_crc = crc32c(low_crc, &lenum, sizeof(lenum));
+	low_crc = btrfs_crc32c(low_crc, &lenum, sizeof(lenum));
 
 	return ((u64)high_crc << 31) ^ (u64)low_crc;
 }
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index dd38dfe174df..c029ca6d5eba 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -686,7 +686,7 @@ static int send_cmd(struct send_ctx *sctx)
 	hdr->len = cpu_to_le32(sctx->send_size - sizeof(*hdr));
 	hdr->crc = 0;
 
-	crc = crc32c(0, (unsigned char *)sctx->send_buf, sctx->send_size);
+	crc = btrfs_crc32c(0, (unsigned char *)sctx->send_buf, sctx->send_size);
 	hdr->crc = cpu_to_le32(crc);
 
 	ret = write_buf(sctx->send_filp, sctx->send_buf, sctx->send_size,
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 03/13] btrfs: use btrfs_crc32c{,_final}() in for free space cache
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
  2019-05-22  8:18 ` [PATCH v3 01/13] btrfs: use btrfs_csum_data() instead of directly calling crc32c Johannes Thumshirn
  2019-05-22  8:18 ` [PATCH v3 02/13] btrfs: resurrect btrfs_crc32c() Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-22  8:19 ` [PATCH v3 04/13] btrfs: don't assume ordered sums to be 4 bytes Johannes Thumshirn
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

The CRC checksum in the free space cache is not dependant on the super
block's csum_type field but always a CRC32C.

So use btrfs_crc32c() and btrfs_crc32c_final() instead of btrfs_csum_data()
and btrfs_csum_final() for computing these checksums.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/free-space-cache.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index f74dc259307b..26ed8ed60722 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -465,9 +465,8 @@ static void io_ctl_set_crc(struct btrfs_io_ctl *io_ctl, int index)
 	if (index == 0)
 		offset = sizeof(u32) * io_ctl->num_pages;
 
-	crc = btrfs_csum_data(io_ctl->orig + offset, crc,
-			      PAGE_SIZE - offset);
-	btrfs_csum_final(crc, (u8 *)&crc);
+	crc = btrfs_crc32c(crc, io_ctl->orig + offset, PAGE_SIZE - offset);
+	btrfs_crc32c_final(crc, (u8 *)&crc);
 	io_ctl_unmap_page(io_ctl);
 	tmp = page_address(io_ctl->pages[0]);
 	tmp += index;
@@ -493,9 +492,8 @@ static int io_ctl_check_crc(struct btrfs_io_ctl *io_ctl, int index)
 	val = *tmp;
 
 	io_ctl_map_page(io_ctl, 0);
-	crc = btrfs_csum_data(io_ctl->orig + offset, crc,
-			      PAGE_SIZE - offset);
-	btrfs_csum_final(crc, (u8 *)&crc);
+	crc = btrfs_crc32c(crc, io_ctl->orig + offset, PAGE_SIZE - offset);
+	btrfs_crc32c_final(crc, (u8 *)&crc);
 	if (val != crc) {
 		btrfs_err_rl(io_ctl->fs_info,
 			"csum mismatch on free space cache");
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 04/13] btrfs: don't assume ordered sums to be 4 bytes
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (2 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 03/13] btrfs: use btrfs_crc32c{,_final}() in for free space cache Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-22  8:19 ` [PATCH v3 05/13] btrfs: dont assume compressed_bio " Johannes Thumshirn
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

BTRFS has the implicit assumption that a checksum in btrfs_orderd_sums is 4
bytes. While this is true for CRC32C, it is not for any other checksum.

Change the data type to be a byte array and adjust loop index calculation
accordingly.

This includes moving the adjustment of 'index' by 'ins_size' in
btrfs_csum_file_blocks() before dividing 'ins_size' by the checksum size,
because before this patch the 'sums' member of 'struct btrfs_ordered_sum'
was 4 Bytes in size and afterwards it is only one byte.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>

---
Changes since v2:
- Change MAX_ORDERED_SUM_BYTES() marco into a function (Nik)
---
 fs/btrfs/compression.c  |  4 ++--
 fs/btrfs/ctree.h        |  3 ++-
 fs/btrfs/file-item.c    | 34 ++++++++++++++++++++--------------
 fs/btrfs/ordered-data.c | 10 ++++++----
 fs/btrfs/ordered-data.h |  4 ++--
 fs/btrfs/scrub.c        |  2 +-
 6 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 4ec1df369e47..98d8c2ed367f 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -632,7 +632,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 
 			if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
 				ret = btrfs_lookup_bio_sums(inode, comp_bio,
-							    sums);
+							    (u8 *)sums);
 				BUG_ON(ret); /* -ENOMEM */
 			}
 			sums += DIV_ROUND_UP(comp_bio->bi_iter.bi_size,
@@ -658,7 +658,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	BUG_ON(ret); /* -ENOMEM */
 
 	if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
-		ret = btrfs_lookup_bio_sums(inode, comp_bio, sums);
+		ret = btrfs_lookup_bio_sums(inode, comp_bio, (u8 *) sums);
 		BUG_ON(ret); /* -ENOMEM */
 	}
 
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d85541f13f65..2ec742db2001 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3198,7 +3198,8 @@ int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
 struct btrfs_dio_private;
 int btrfs_del_csums(struct btrfs_trans_handle *trans,
 		    struct btrfs_fs_info *fs_info, u64 bytenr, u64 len);
-blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u32 *dst);
+blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
+				   u8 *dst);
 blk_status_t btrfs_lookup_bio_sums_dio(struct inode *inode, struct bio *bio,
 			      u64 logical_offset);
 int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index d431ea8198e4..39fc8da701fe 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -22,9 +22,13 @@
 #define MAX_CSUM_ITEMS(r, size) (min_t(u32, __MAX_CSUM_ITEMS(r, size), \
 				       PAGE_SIZE))
 
-#define MAX_ORDERED_SUM_BYTES(fs_info) ((PAGE_SIZE - \
-				   sizeof(struct btrfs_ordered_sum)) / \
-				   sizeof(u32) * (fs_info)->sectorsize)
+static inline size_t max_ordered_sum_bytes(struct btrfs_fs_info *fs_info,
+					   u16 csum_size)
+{
+	u32 ncsums = (PAGE_SIZE - sizeof(struct btrfs_ordered_sum)) / csum_size;
+
+	return ncsums * fs_info->sectorsize;
+}
 
 int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root,
@@ -144,7 +148,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
 }
 
 static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
-				   u64 logical_offset, u32 *dst, int dio)
+				   u64 logical_offset, u8 *dst, int dio)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	struct bio_vec bvec;
@@ -211,7 +215,7 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
 		if (!dio)
 			offset = page_offset(bvec.bv_page) + bvec.bv_offset;
 		count = btrfs_find_ordered_sum(inode, offset, disk_bytenr,
-					       (u32 *)csum, nblocks);
+					       csum, nblocks);
 		if (count)
 			goto found;
 
@@ -283,7 +287,8 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
 	return 0;
 }
 
-blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u32 *dst)
+blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
+				   u8 *dst)
 {
 	return __btrfs_lookup_bio_sums(inode, bio, 0, dst, 0);
 }
@@ -374,7 +379,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
 				      struct btrfs_csum_item);
 		while (start < csum_end) {
 			size = min_t(size_t, csum_end - start,
-				     MAX_ORDERED_SUM_BYTES(fs_info));
+				     max_ordered_sum_bytes(fs_info, csum_size));
 			sums = kzalloc(btrfs_ordered_sum_size(fs_info, size),
 				       GFP_NOFS);
 			if (!sums) {
@@ -439,6 +444,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 	int i;
 	u64 offset;
 	unsigned nofs_flag;
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 
 	nofs_flag = memalloc_nofs_save();
 	sums = kvzalloc(btrfs_ordered_sum_size(fs_info, bio->bi_iter.bi_size),
@@ -473,6 +479,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 						 - 1);
 
 		for (i = 0; i < nr_sectors; i++) {
+			u32 tmp;
 			if (offset >= ordered->file_offset + ordered->len ||
 				offset < ordered->file_offset) {
 				unsigned long bytes_left;
@@ -498,17 +505,16 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 				index = 0;
 			}
 
-			sums->sums[index] = ~(u32)0;
+			memset(&sums->sums[index], 0xff, csum_size);
 			data = kmap_atomic(bvec.bv_page);
-			sums->sums[index]
-				= btrfs_csum_data(data + bvec.bv_offset
+			tmp = btrfs_csum_data(data + bvec.bv_offset
 						+ (i * fs_info->sectorsize),
-						sums->sums[index],
+						*(u32 *)&sums->sums[index],
 						fs_info->sectorsize);
 			kunmap_atomic(data);
-			btrfs_csum_final(sums->sums[index],
+			btrfs_csum_final(tmp,
 					(char *)(sums->sums + index));
-			index++;
+			index += csum_size;
 			offset += fs_info->sectorsize;
 			this_sum_bytes += fs_info->sectorsize;
 			total_bytes += fs_info->sectorsize;
@@ -904,9 +910,9 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 	write_extent_buffer(leaf, sums->sums + index, (unsigned long)item,
 			    ins_size);
 
+	index += ins_size;
 	ins_size /= csum_size;
 	total_bytes += ins_size * fs_info->sectorsize;
-	index += ins_size;
 
 	btrfs_mark_buffer_dirty(path->nodes[0]);
 	if (total_bytes < sums->len) {
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 52889da69113..6f7a18148dcb 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -924,14 +924,16 @@ int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
  * be reclaimed before their checksum is actually put into the btree
  */
 int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
-			   u32 *sum, int len)
+			   u8 *sum, int len)
 {
+	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	struct btrfs_ordered_sum *ordered_sum;
 	struct btrfs_ordered_extent *ordered;
 	struct btrfs_ordered_inode_tree *tree = &BTRFS_I(inode)->ordered_tree;
 	unsigned long num_sectors;
 	unsigned long i;
 	u32 sectorsize = btrfs_inode_sectorsize(inode);
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	int index = 0;
 
 	ordered = btrfs_lookup_ordered_extent(inode, offset);
@@ -947,10 +949,10 @@ int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
 			num_sectors = ordered_sum->len >>
 				      inode->i_sb->s_blocksize_bits;
 			num_sectors = min_t(int, len - index, num_sectors - i);
-			memcpy(sum + index, ordered_sum->sums + i,
-			       num_sectors);
+			memcpy(sum + index, ordered_sum->sums + i * csum_size,
+			       num_sectors * csum_size);
 
-			index += (int)num_sectors;
+			index += (int)num_sectors * csum_size;
 			if (index == len)
 				goto out;
 			disk_bytenr += num_sectors * sectorsize;
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index 4c5991c3de14..9a9884966343 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -23,7 +23,7 @@ struct btrfs_ordered_sum {
 	int len;
 	struct list_head list;
 	/* last field is a variable length array of csums */
-	u32 sums[];
+	u8 sums[];
 };
 
 /*
@@ -183,7 +183,7 @@ struct btrfs_ordered_extent *btrfs_lookup_ordered_range(
 int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
 				struct btrfs_ordered_extent *ordered);
 int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
-			   u32 *sum, int len);
+			   u8 *sum, int len);
 u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
 			       const u64 range_start, const u64 range_len);
 u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index f7b29f9db5e2..2cf3cf9e9c9b 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2448,7 +2448,7 @@ static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
 	ASSERT(index < UINT_MAX);
 
 	num_sectors = sum->len / sctx->fs_info->sectorsize;
-	memcpy(csum, sum->sums + index, sctx->csum_size);
+	memcpy(csum, sum->sums + index * sctx->csum_size, sctx->csum_size);
 	if (index == num_sectors - 1) {
 		list_del(&sum->list);
 		kfree(sum);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 05/13] btrfs: dont assume compressed_bio sums to be 4 bytes
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (3 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 04/13] btrfs: don't assume ordered sums to be 4 bytes Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-22  8:19 ` [PATCH v3 06/13] btrfs: format checksums according to type for printing Johannes Thumshirn
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

BTRFS has the implicit assumption that a checksum in compressed_bio is 4
bytes. While this is true for CRC32C, it is not for any other checksum.

Change the data type to be a byte array and adjust loop index calculation
accordingly.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
Changes to v2:
- Remove stray hunk in btrfs_find_ordered_sum() (Nik)
---
 fs/btrfs/compression.c | 27 +++++++++++++++++----------
 fs/btrfs/compression.h |  2 +-
 fs/btrfs/file-item.c   |  2 +-
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 98d8c2ed367f..d5642f3b5c44 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -57,12 +57,14 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 				 struct compressed_bio *cb,
 				 u64 disk_start)
 {
+	struct btrfs_fs_info *fs_info = inode->root->fs_info;
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	int ret;
 	struct page *page;
 	unsigned long i;
 	char *kaddr;
 	u32 csum;
-	u32 *cb_sum = &cb->sums;
+	u8 *cb_sum = cb->sums;
 
 	if (inode->flags & BTRFS_INODE_NODATASUM)
 		return 0;
@@ -76,13 +78,13 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 		btrfs_csum_final(csum, (u8 *)&csum);
 		kunmap_atomic(kaddr);
 
-		if (csum != *cb_sum) {
+		if (memcmp(&csum, cb_sum, csum_size)) {
 			btrfs_print_data_csum_error(inode, disk_start, csum,
-					*cb_sum, cb->mirror_num);
+					*(u32 *)cb_sum, cb->mirror_num);
 			ret = -EIO;
 			goto fail;
 		}
-		cb_sum++;
+		cb_sum += csum_size;
 
 	}
 	ret = 0;
@@ -537,7 +539,8 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	struct extent_map *em;
 	blk_status_t ret = BLK_STS_RESOURCE;
 	int faili = 0;
-	u32 *sums;
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	u8 *sums;
 
 	em_tree = &BTRFS_I(inode)->extent_tree;
 
@@ -559,7 +562,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	cb->errors = 0;
 	cb->inode = inode;
 	cb->mirror_num = mirror_num;
-	sums = &cb->sums;
+	sums = cb->sums;
 
 	cb->start = em->orig_start;
 	em_len = em->len;
@@ -618,6 +621,8 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 		page->mapping = NULL;
 		if (submit || bio_add_page(comp_bio, page, PAGE_SIZE, 0) <
 		    PAGE_SIZE) {
+			unsigned int nr_sectors;
+
 			ret = btrfs_bio_wq_end_io(fs_info, comp_bio,
 						  BTRFS_WQ_ENDIO_DATA);
 			BUG_ON(ret); /* -ENOMEM */
@@ -632,11 +637,13 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 
 			if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
 				ret = btrfs_lookup_bio_sums(inode, comp_bio,
-							    (u8 *)sums);
+							    sums);
 				BUG_ON(ret); /* -ENOMEM */
 			}
-			sums += DIV_ROUND_UP(comp_bio->bi_iter.bi_size,
-					     fs_info->sectorsize);
+
+			nr_sectors = DIV_ROUND_UP(comp_bio->bi_iter.bi_size,
+						  fs_info->sectorsize);
+			sums += csum_size * nr_sectors;
 
 			ret = btrfs_map_bio(fs_info, comp_bio, mirror_num, 0);
 			if (ret) {
@@ -658,7 +665,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	BUG_ON(ret); /* -ENOMEM */
 
 	if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
-		ret = btrfs_lookup_bio_sums(inode, comp_bio, (u8 *) sums);
+		ret = btrfs_lookup_bio_sums(inode, comp_bio, sums);
 		BUG_ON(ret); /* -ENOMEM */
 	}
 
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 9976fe0f7526..191e5f4e3523 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -61,7 +61,7 @@ struct compressed_bio {
 	 * the start of a variable length array of checksums only
 	 * used by reads
 	 */
-	u32 sums;
+	u8 sums[];
 };
 
 static inline unsigned int btrfs_compress_type(unsigned int type_level)
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 39fc8da701fe..0bb77392ec08 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -186,7 +186,7 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
 		}
 		csum = btrfs_bio->csum;
 	} else {
-		csum = (u8 *)dst;
+		csum = dst;
 	}
 
 	if (bio->bi_iter.bi_size > PAGE_SIZE * 8)
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 06/13] btrfs: format checksums according to type for printing
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (4 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 05/13] btrfs: dont assume compressed_bio " Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-27 16:57   ` David Sterba
  2019-05-22  8:19 ` [PATCH v3 07/13] btrfs: add common checksum type validation Johannes Thumshirn
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

Add a small helper for btrfs_print_data_csum_error() which formats the
checksum according to it's type for pretty printing.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/btrfs_inode.h | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index d5b438706b77..f0a757eb5744 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -337,22 +337,42 @@ static inline void btrfs_inode_resume_unlocked_dio(struct btrfs_inode *inode)
 	clear_bit(BTRFS_INODE_READDIO_NEED_LOCK, &inode->runtime_flags);
 }
 
+static inline void btrfs_csum_format(struct btrfs_super_block *sb,
+				     u32 csum, u8 *cbuf)
+{
+	size_t size = btrfs_super_csum_size(sb) * 8;
+
+	switch (btrfs_super_csum_type(sb)) {
+	case BTRFS_CSUM_TYPE_CRC32:
+		snprintf(cbuf, size, "0x%08x", csum);
+		break;
+	default: /* can't happen -  csum type is validated at mount time */
+		break;
+	}
+}
+
 static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
 		u64 logical_start, u32 csum, u32 csum_expected, int mirror_num)
 {
 	struct btrfs_root *root = inode->root;
+	struct btrfs_super_block *sb = root->fs_info->super_copy;
+	u8 cbuf[BTRFS_CSUM_SIZE];
+	u8 ecbuf[BTRFS_CSUM_SIZE];
+
+	btrfs_csum_format(sb, csum, cbuf);
+	btrfs_csum_format(sb, csum_expected, ecbuf);
 
 	/* Output minus objectid, which is more meaningful */
 	if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID)
 		btrfs_warn_rl(root->fs_info,
-	"csum failed root %lld ino %lld off %llu csum 0x%08x expected csum 0x%08x mirror %d",
+	"csum failed root %lld ino %lld off %llu csum %s expected csum %s mirror %d",
 			root->root_key.objectid, btrfs_ino(inode),
-			logical_start, csum, csum_expected, mirror_num);
+			logical_start, cbuf, ecbuf, mirror_num);
 	else
 		btrfs_warn_rl(root->fs_info,
-	"csum failed root %llu ino %llu off %llu csum 0x%08x expected csum 0x%08x mirror %d",
+	"csum failed root %llu ino %llu off %llu csum %s expected csum %s mirror %d",
 			root->root_key.objectid, btrfs_ino(inode),
-			logical_start, csum, csum_expected, mirror_num);
+			logical_start, cbuf, ecbuf, mirror_num);
 }
 
 #endif
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 07/13] btrfs: add common checksum type validation
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (5 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 06/13] btrfs: format checksums according to type for printing Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-30 15:47   ` David Sterba
  2019-05-22  8:19 ` [PATCH v3 08/13] btrfs: check for supported superblock checksum type before checksum validation Johannes Thumshirn
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

Currently btrfs is only supporting CRC32C as checksumming algorithm. As
this is about to change provide a function to validate the checksum type in
the superblock against all possible algorithms.

This makes adding new algorithms easier as there are fewer places to adjust
when adding new algorithms.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>

---
Changes to v2:
- Only pass csum_type into btrfs_supported_csum() (David)
- Directly return in btrfs_check_super_csum() (David)
---
 fs/btrfs/disk-io.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 663efce22d98..594583273782 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -356,6 +356,16 @@ static int verify_parent_transid(struct extent_io_tree *io_tree,
 	return ret;
 }
 
+static bool btrfs_supported_super_csum(u16 csum_type)
+{
+	switch (csum_type) {
+	case BTRFS_CSUM_TYPE_CRC32:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /*
  * Return 0 if the superblock checksum type matches the checksum value of that
  * algorithm. Pass the raw disk superblock data.
@@ -366,7 +376,12 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 	struct btrfs_super_block *disk_sb =
 		(struct btrfs_super_block *)raw_disk_sb;
 	u16 csum_type = btrfs_super_csum_type(disk_sb);
-	int ret = 0;
+
+	if (!btrfs_supported_super_csum(csum_type)) {
+		btrfs_err(fs_info, "unsupported checksum algorithm %u",
+			  csum_type);
+		return 1;
+	}
 
 	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
 		u32 crc = ~(u32)0;
@@ -382,16 +397,10 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 		btrfs_csum_final(crc, result);
 
 		if (memcmp(raw_disk_sb, result, sizeof(result)))
-			ret = 1;
+			return 1;
 	}
 
-	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
-		btrfs_err(fs_info, "unsupported checksum algorithm %u",
-				csum_type);
-		ret = 1;
-	}
-
-	return ret;
+	return 0;
 }
 
 int btrfs_verify_level_key(struct extent_buffer *eb, int level,
@@ -2577,7 +2586,7 @@ static int btrfs_validate_write_super(struct btrfs_fs_info *fs_info,
 	ret = validate_super(fs_info, sb, -1);
 	if (ret < 0)
 		goto out;
-	if (btrfs_super_csum_type(sb) != BTRFS_CSUM_TYPE_CRC32) {
+	if (!btrfs_supported_super_csum(sb)) {
 		ret = -EUCLEAN;
 		btrfs_err(fs_info, "invalid csum type, has %u want %u",
 			  btrfs_super_csum_type(sb), BTRFS_CSUM_TYPE_CRC32);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 08/13] btrfs: check for supported superblock checksum type before checksum validation
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (6 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 07/13] btrfs: add common checksum type validation Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-30 15:49   ` David Sterba
  2019-05-22  8:19 ` [PATCH v3 09/13] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions Johannes Thumshirn
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

Now that we have factorerd out the superblock checksum type validation, we
can check for supported superblock checksum types before doing the actual
validation of the superblock read from disk.

This leads the path to further simplifications of btrfs_check_super_csum()
later on.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>

---
Changes to v2:
- Print on-disk checksum type if we encounter an unsupported type (David)
---
 fs/btrfs/disk-io.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 594583273782..f541d3c15d99 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2821,6 +2821,16 @@ int open_ctree(struct super_block *sb,
 		goto fail_alloc;
 	}
 
+	if (!btrfs_supported_super_csum((struct btrfs_super_block *)
+					bh->b_data)) {
+		btrfs_err(fs_info, "unsupported checksum algorithm: %d",
+			  btrfs_super_csum_type((struct btrfs_super_block *)
+						bh->b_data));
+		err = -EINVAL;
+		brelse(bh);
+		goto fail_alloc;
+	}
+
 	/*
 	 * We want to check superblock checksum, the type is stored inside.
 	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 09/13] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (7 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 08/13] btrfs: check for supported superblock checksum type before checksum validation Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-22  8:19 ` [PATCH v3 10/13] btrfs: add boilerplate code for directly including the crypto framework Johannes Thumshirn
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

Now that we have already checked for a valid checksum type before calling
btrfs_check_super_csum(), it can be simplified even further.

While at it get rid of the implicit size assumption of the resulting
checksum as well.

This is a preparation for changing all checksum functionality to use the
crypto layer later.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>

---
Changes to v1:
- Check for disk_sb->csum instead of raw buffer (Nikolay)
---
 fs/btrfs/disk-io.c | 45 ++++++++++++++++++---------------------------
 1 file changed, 18 insertions(+), 27 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index f541d3c15d99..b1c30a5cef9d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -375,30 +375,20 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 {
 	struct btrfs_super_block *disk_sb =
 		(struct btrfs_super_block *)raw_disk_sb;
-	u16 csum_type = btrfs_super_csum_type(disk_sb);
-
-	if (!btrfs_supported_super_csum(csum_type)) {
-		btrfs_err(fs_info, "unsupported checksum algorithm %u",
-			  csum_type);
-		return 1;
-	}
-
-	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
-		u32 crc = ~(u32)0;
-		char result[sizeof(crc)];
+	u32 crc = ~(u32)0;
+	char result[BTRFS_CSUM_SIZE];
 
-		/*
-		 * The super_block structure does not span the whole
-		 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
-		 * is filled with zeros and is included in the checksum.
-		 */
-		crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
-				crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-		btrfs_csum_final(crc, result);
+	/*
+	 * The super_block structure does not span the whole
+	 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
+	 * is filled with zeros and is included in the checksum.
+	 */
+	crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
+			      crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+	btrfs_csum_final(crc, result);
 
-		if (memcmp(raw_disk_sb, result, sizeof(result)))
-			return 1;
-	}
+	if (memcmp(disk_sb->csum, result, btrfs_super_csum_size(disk_sb)))
+		return 1;
 
 	return 0;
 }
@@ -2586,7 +2576,7 @@ static int btrfs_validate_write_super(struct btrfs_fs_info *fs_info,
 	ret = validate_super(fs_info, sb, -1);
 	if (ret < 0)
 		goto out;
-	if (!btrfs_supported_super_csum(sb)) {
+	if (!btrfs_supported_super_csum(btrfs_super_csum_type(sb))) {
 		ret = -EUCLEAN;
 		btrfs_err(fs_info, "invalid csum type, has %u want %u",
 			  btrfs_super_csum_type(sb), BTRFS_CSUM_TYPE_CRC32);
@@ -2616,6 +2606,7 @@ int open_ctree(struct super_block *sb,
 	u32 stripesize;
 	u64 generation;
 	u64 features;
+	u16 csum_type;
 	struct btrfs_key location;
 	struct buffer_head *bh;
 	struct btrfs_super_block *disk_super;
@@ -2821,11 +2812,11 @@ int open_ctree(struct super_block *sb,
 		goto fail_alloc;
 	}
 
-	if (!btrfs_supported_super_csum((struct btrfs_super_block *)
-					bh->b_data)) {
+	csum_type = btrfs_super_csum_type((struct btrfs_super_block *)
+					  bh->b_data);
+	if (!btrfs_supported_super_csum(csum_type)) {
 		btrfs_err(fs_info, "unsupported checksum algorithm: %d",
-			  btrfs_super_csum_type((struct btrfs_super_block *)
-						bh->b_data));
+			  csum_type);
 		err = -EINVAL;
 		brelse(bh);
 		goto fail_alloc;
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 10/13] btrfs: add boilerplate code for directly including the crypto framework
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (8 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 09/13] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-22  8:19 ` [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

Add boilerplate code for directly including the crypto framework.

This helps us flipping the switch for new algorithms.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>

---
- Remove stray newline (David)
- Directly pass in csum_type to btrfs_init_csum_hash() (David)
- Don't use single letter variables (David)
---
 fs/btrfs/ctree.h   | 10 ++++++++++
 fs/btrfs/disk-io.c | 46 +++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 2ec742db2001..8b635ca370f5 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -73,6 +73,7 @@ struct btrfs_ref;
 
 /* four bytes for CRC32 */
 static const int btrfs_csum_sizes[] = { 4 };
+static char *btrfs_csum_names[] = { "crc32c" };
 
 #define BTRFS_EMPTY_DIR_SIZE 0
 
@@ -1165,6 +1166,8 @@ struct btrfs_fs_info {
 	spinlock_t ref_verify_lock;
 	struct rb_root block_tree;
 #endif
+
+	struct crypto_shash *csum_shash;
 };
 
 static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
@@ -2452,6 +2455,13 @@ static inline int btrfs_super_csum_size(const struct btrfs_super_block *s)
 	return btrfs_csum_sizes[t];
 }
 
+static inline char *btrfs_super_csum_name(u16 csum_type)
+{
+	/*
+	 * csum type is validated at mount time
+	 */
+	return btrfs_csum_names[csum_type];
+}
 
 /*
  * The leaf data grows from end-to-front in the node.
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index b1c30a5cef9d..f73660e63522 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -19,6 +19,7 @@
 #include <linux/crc32c.h>
 #include <linux/sched/mm.h>
 #include <asm/unaligned.h>
+#include <crypto/hash.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"
@@ -2261,6 +2262,29 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info,
 	return 0;
 }
 
+static int btrfs_init_csum_hash(struct btrfs_fs_info *fs_info, u16 csum_type)
+{
+	struct crypto_shash *csum_shash;
+	const char *csum_name = btrfs_super_csum_name(csum_type);
+
+	csum_shash = crypto_alloc_shash(csum_name, 0, 0);
+
+	if (IS_ERR(csum_shash)) {
+		btrfs_err(fs_info, "error allocating %s hash for checksum",
+			  csum_name);
+		return PTR_ERR(csum_shash);
+	}
+
+	fs_info->csum_shash = csum_shash;
+
+	return 0;
+}
+
+static void btrfs_free_csum_hash(struct btrfs_fs_info *fs_info)
+{
+	crypto_free_shash(fs_info->csum_shash);
+}
+
 static int btrfs_replay_log(struct btrfs_fs_info *fs_info,
 			    struct btrfs_fs_devices *fs_devices)
 {
@@ -2822,6 +2846,12 @@ int open_ctree(struct super_block *sb,
 		goto fail_alloc;
 	}
 
+	ret = btrfs_init_csum_hash(fs_info, csum_type);
+	if (ret) {
+		err = ret;
+		goto fail_alloc;
+	}
+
 	/*
 	 * We want to check superblock checksum, the type is stored inside.
 	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
@@ -2830,7 +2860,7 @@ int open_ctree(struct super_block *sb,
 		btrfs_err(fs_info, "superblock checksum mismatch");
 		err = -EINVAL;
 		brelse(bh);
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	/*
@@ -2867,11 +2897,11 @@ int open_ctree(struct super_block *sb,
 	if (ret) {
 		btrfs_err(fs_info, "superblock contains fatal errors");
 		err = -EINVAL;
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	if (!btrfs_super_root(disk_super))
-		goto fail_alloc;
+		goto fail_csum;
 
 	/* check FS state, whether FS is broken. */
 	if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_ERROR)
@@ -2893,7 +2923,7 @@ int open_ctree(struct super_block *sb,
 	ret = btrfs_parse_options(fs_info, options, sb->s_flags);
 	if (ret) {
 		err = ret;
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	features = btrfs_super_incompat_flags(disk_super) &
@@ -2903,7 +2933,7 @@ int open_ctree(struct super_block *sb,
 		    "cannot mount because of unsupported optional features (%llx)",
 		    features);
 		err = -EINVAL;
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	features = btrfs_super_incompat_flags(disk_super);
@@ -2947,7 +2977,7 @@ int open_ctree(struct super_block *sb,
 		btrfs_err(fs_info,
 "unequal nodesize/sectorsize (%u != %u) are not allowed for mixed block groups",
 			nodesize, sectorsize);
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	/*
@@ -2963,7 +2993,7 @@ int open_ctree(struct super_block *sb,
 	"cannot mount read-write because of unsupported optional features (%llx)",
 		       features);
 		err = -EINVAL;
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	ret = btrfs_init_workqueues(fs_info, fs_devices);
@@ -3341,6 +3371,8 @@ int open_ctree(struct super_block *sb,
 fail_sb_buffer:
 	btrfs_stop_all_workers(fs_info);
 	btrfs_free_block_groups(fs_info);
+fail_csum:
+	btrfs_free_csum_hash(fs_info);
 fail_alloc:
 fail_iput:
 	btrfs_mapping_tree_free(&fs_info->mapping_tree);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (9 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 10/13] btrfs: add boilerplate code for directly including the crypto framework Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-22  8:33   ` Nikolay Borisov
  2019-05-29 19:32   ` David Gstir
  2019-05-22  8:19 ` [PATCH v3 12/13] btrfs: remove assumption about csum type form btrfs_print_data_csum_error() Johannes Thumshirn
                   ` (2 subsequent siblings)
  13 siblings, 2 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

Currently btrfs_csum_data() relied on the crc32c() wrapper around the crypto
framework for calculating the CRCs.

As we have our own crypto_shash structure in the fs_info now, we can
directly call into the crypto framework without going trough the wrapper.

This way we can even remove the btrfs_csum_data() and btrfs_csum_final()
wrappers.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

---
Changes to v2:
- Also select CRYPTO in Kconfig
- Add pre dependency on crc32c
Changes to v1:
- merge with 'btrfs: pass in an fs_info to btrfs_csum_{data,final}()'
- Remove btrfs_csum_data() and btrfs_csum_final() alltogether
- don't use LIBCRC32C but CRYPTO_CRC32C in KConfig
---
 fs/btrfs/Kconfig           |  3 ++-
 fs/btrfs/check-integrity.c | 12 +++++++----
 fs/btrfs/compression.c     | 19 +++++++++++------
 fs/btrfs/disk-io.c         | 51 +++++++++++++++++++++++++---------------------
 fs/btrfs/disk-io.h         |  2 --
 fs/btrfs/file-item.c       | 18 ++++++++--------
 fs/btrfs/inode.c           | 24 ++++++++++++++--------
 fs/btrfs/scrub.c           | 37 +++++++++++++++++++++++++--------
 fs/btrfs/super.c           |  1 +
 9 files changed, 106 insertions(+), 61 deletions(-)

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index 23537bc8c827..212b4a854f2c 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -2,7 +2,8 @@
 
 config BTRFS_FS
 	tristate "Btrfs filesystem support"
-	select LIBCRC32C
+	select CRYPTO
+	select CRYPTO_CRC32C
 	select ZLIB_INFLATE
 	select ZLIB_DEFLATE
 	select LZO_COMPRESS
diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 85774e2fa3e5..6adee8fe82e2 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -83,7 +83,7 @@
 #include <linux/blkdev.h>
 #include <linux/mm.h>
 #include <linux/string.h>
-#include <linux/crc32c.h>
+#include <crypto/hash.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"
@@ -1710,9 +1710,9 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
 				     char **datav, unsigned int num_pages)
 {
 	struct btrfs_fs_info *fs_info = state->fs_info;
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	struct btrfs_header *h;
 	u8 csum[BTRFS_CSUM_SIZE];
-	u32 crc = ~(u32)0;
 	unsigned int i;
 
 	if (num_pages * PAGE_SIZE < state->metablock_size)
@@ -1723,14 +1723,18 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
 	if (memcmp(h->fsid, fs_info->fs_devices->fsid, BTRFS_FSID_SIZE))
 		return 1;
 
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+	crypto_shash_init(shash);
+
 	for (i = 0; i < num_pages; i++) {
 		u8 *data = i ? datav[i] : (datav[i] + BTRFS_CSUM_SIZE);
 		size_t sublen = i ? PAGE_SIZE :
 				    (PAGE_SIZE - BTRFS_CSUM_SIZE);
 
-		crc = btrfs_csum_data(data, crc, sublen);
+		crypto_shash_update(shash, data, sublen);
 	}
-	btrfs_csum_final(crc, csum);
+	crypto_shash_final(shash, csum);
 	if (memcmp(csum, h->csum, state->csum_size))
 		return 1;
 
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index d5642f3b5c44..e027e58358be 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -17,6 +17,7 @@
 #include <linux/slab.h>
 #include <linux/sched/mm.h>
 #include <linux/log2.h>
+#include <crypto/hash.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"
@@ -58,29 +59,35 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 				 u64 disk_start)
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	int ret;
 	struct page *page;
 	unsigned long i;
 	char *kaddr;
-	u32 csum;
+	u8 csum[BTRFS_CSUM_SIZE];
 	u8 *cb_sum = cb->sums;
 
 	if (inode->flags & BTRFS_INODE_NODATASUM)
 		return 0;
 
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+
 	for (i = 0; i < cb->nr_pages; i++) {
 		page = cb->compressed_pages[i];
-		csum = ~(u32)0;
+
+		crypto_shash_init(shash);
 
 		kaddr = kmap_atomic(page);
-		csum = btrfs_csum_data(kaddr, csum, PAGE_SIZE);
-		btrfs_csum_final(csum, (u8 *)&csum);
+		crypto_shash_update(shash, kaddr, PAGE_SIZE);
+		crypto_shash_final(shash, (u8 *)&csum);
 		kunmap_atomic(kaddr);
 
 		if (memcmp(&csum, cb_sum, csum_size)) {
-			btrfs_print_data_csum_error(inode, disk_start, csum,
-					*(u32 *)cb_sum, cb->mirror_num);
+			btrfs_print_data_csum_error(inode, disk_start,
+						    *(u32 *)csum, *(u32 *)cb_sum,
+						    cb->mirror_num);
 			ret = -EIO;
 			goto fail;
 		}
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index f73660e63522..68cdee36b470 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -250,16 +250,6 @@ struct extent_map *btree_get_extent(struct btrfs_inode *inode,
 	return em;
 }
 
-u32 btrfs_csum_data(const char *data, u32 seed, size_t len)
-{
-	return crc32c(seed, data, len);
-}
-
-void btrfs_csum_final(u32 crc, u8 *result)
-{
-	put_unaligned_le32(~crc, result);
-}
-
 /*
  * Compute the csum of a btree block and store the result to provided buffer.
  *
@@ -267,6 +257,8 @@ void btrfs_csum_final(u32 crc, u8 *result)
  */
 static int csum_tree_block(struct extent_buffer *buf, u8 *result)
 {
+	struct btrfs_fs_info *fs_info = buf->fs_info;
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	unsigned long len;
 	unsigned long cur_len;
 	unsigned long offset = BTRFS_CSUM_SIZE;
@@ -274,9 +266,14 @@ static int csum_tree_block(struct extent_buffer *buf, u8 *result)
 	unsigned long map_start;
 	unsigned long map_len;
 	int err;
-	u32 crc = ~(u32)0;
+
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+
+	crypto_shash_init(shash);
 
 	len = buf->len - offset;
+
 	while (len > 0) {
 		/*
 		 * Note: we don't need to check for the err == 1 case here, as
@@ -289,14 +286,13 @@ static int csum_tree_block(struct extent_buffer *buf, u8 *result)
 		if (WARN_ON(err))
 			return err;
 		cur_len = min(len, map_len - (offset - map_start));
-		crc = btrfs_csum_data(kaddr + offset - map_start,
-				      crc, cur_len);
+		crypto_shash_update(shash, kaddr + offset - map_start, cur_len);
 		len -= cur_len;
 		offset += cur_len;
 	}
 	memset(result, 0, BTRFS_CSUM_SIZE);
 
-	btrfs_csum_final(crc, result);
+	crypto_shash_final(shash, result);
 
 	return 0;
 }
@@ -376,17 +372,22 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 {
 	struct btrfs_super_block *disk_sb =
 		(struct btrfs_super_block *)raw_disk_sb;
-	u32 crc = ~(u32)0;
 	char result[BTRFS_CSUM_SIZE];
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
+
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+
+	crypto_shash_init(shash);
 
 	/*
 	 * The super_block structure does not span the whole
 	 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
 	 * is filled with zeros and is included in the checksum.
 	 */
-	crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
-			      crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-	btrfs_csum_final(crc, result);
+	crypto_shash_update(shash, raw_disk_sb + BTRFS_CSUM_SIZE,
+			    BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+	crypto_shash_final(shash, result);
 
 	if (memcmp(disk_sb->csum, result, btrfs_super_csum_size(disk_sb)))
 		return 1;
@@ -3514,17 +3515,21 @@ struct buffer_head *btrfs_read_dev_super(struct block_device *bdev)
 static int write_dev_supers(struct btrfs_device *device,
 			    struct btrfs_super_block *sb, int max_mirrors)
 {
+	struct btrfs_fs_info *fs_info = device->fs_info;
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	struct buffer_head *bh;
 	int i;
 	int ret;
 	int errors = 0;
-	u32 crc;
 	u64 bytenr;
 	int op_flags;
 
 	if (max_mirrors == 0)
 		max_mirrors = BTRFS_SUPER_MIRROR_MAX;
 
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+
 	for (i = 0; i < max_mirrors; i++) {
 		bytenr = btrfs_sb_offset(i);
 		if (bytenr + BTRFS_SUPER_INFO_SIZE >=
@@ -3533,10 +3538,10 @@ static int write_dev_supers(struct btrfs_device *device,
 
 		btrfs_set_super_bytenr(sb, bytenr);
 
-		crc = ~(u32)0;
-		crc = btrfs_csum_data((const char *)sb + BTRFS_CSUM_SIZE, crc,
-				      BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-		btrfs_csum_final(crc, sb->csum);
+		crypto_shash_init(shash);
+		crypto_shash_update(shash, (const char *)sb + BTRFS_CSUM_SIZE,
+				    BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+		crypto_shash_final(shash, sb->csum);
 
 		/* One reference for us, and we leave it for the caller */
 		bh = __getblk(device->bdev, bytenr / BTRFS_BDEV_BLOCKSIZE,
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index a0161aa1ea0b..e80f7c45a307 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -115,8 +115,6 @@ int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,
 			  int atomic);
 int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid, int level,
 		      struct btrfs_key *first_key);
-u32 btrfs_csum_data(const char *data, u32 seed, size_t len);
-void btrfs_csum_final(u32 crc, u8 *result);
 blk_status_t btrfs_bio_wq_end_io(struct btrfs_fs_info *info, struct bio *bio,
 			enum btrfs_wq_endio_type metadata);
 blk_status_t btrfs_wq_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 0bb77392ec08..ba2405b4931b 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -8,6 +8,7 @@
 #include <linux/pagemap.h>
 #include <linux/highmem.h>
 #include <linux/sched/mm.h>
+#include <crypto/hash.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"
@@ -432,6 +433,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 		       u64 file_start, int contig)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	struct btrfs_ordered_sum *sums;
 	struct btrfs_ordered_extent *ordered = NULL;
 	char *data;
@@ -465,6 +467,9 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 	sums->bytenr = (u64)bio->bi_iter.bi_sector << 9;
 	index = 0;
 
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+
 	bio_for_each_segment(bvec, bio, iter) {
 		if (!contig)
 			offset = page_offset(bvec.bv_page) + bvec.bv_offset;
@@ -479,7 +484,6 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 						 - 1);
 
 		for (i = 0; i < nr_sectors; i++) {
-			u32 tmp;
 			if (offset >= ordered->file_offset + ordered->len ||
 				offset < ordered->file_offset) {
 				unsigned long bytes_left;
@@ -505,15 +509,13 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 				index = 0;
 			}
 
-			memset(&sums->sums[index], 0xff, csum_size);
+			crypto_shash_init(shash);
 			data = kmap_atomic(bvec.bv_page);
-			tmp = btrfs_csum_data(data + bvec.bv_offset
-						+ (i * fs_info->sectorsize),
-						*(u32 *)&sums->sums[index],
-						fs_info->sectorsize);
+			crypto_shash_update(shash, data + bvec.bv_offset
+					    + (i * fs_info->sectorsize),
+					    fs_info->sectorsize);
 			kunmap_atomic(data);
-			btrfs_csum_final(tmp,
-					(char *)(sums->sums + index));
+			crypto_shash_final(shash, (char *)(sums->sums + index));
 			index += csum_size;
 			offset += fs_info->sectorsize;
 			this_sum_bytes += fs_info->sectorsize;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b6d549c993f6..402c9ea8239d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3203,23 +3203,31 @@ static int __readpage_endio_check(struct inode *inode,
 				  int icsum, struct page *page,
 				  int pgoff, u64 start, size_t len)
 {
+	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	char *kaddr;
-	u32 csum_expected;
-	u32 csum = ~(u32)0;
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	u8 *csum_expected;
+	u8 csum[BTRFS_CSUM_SIZE];
 
-	csum_expected = *(((u32 *)io_bio->csum) + icsum);
+	csum_expected = ((u8 *)io_bio->csum) + icsum * csum_size;
 
 	kaddr = kmap_atomic(page);
-	csum = btrfs_csum_data(kaddr + pgoff, csum,  len);
-	btrfs_csum_final(csum, (u8 *)&csum);
-	if (csum != csum_expected)
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+
+	crypto_shash_init(shash);
+	crypto_shash_update(shash, kaddr + pgoff, len);
+	crypto_shash_final(shash, csum);
+
+	if (memcmp(csum, csum_expected, csum_size))
 		goto zeroit;
 
 	kunmap_atomic(kaddr);
 	return 0;
 zeroit:
-	btrfs_print_data_csum_error(BTRFS_I(inode), start, csum, csum_expected,
-				    io_bio->mirror_num);
+	btrfs_print_data_csum_error(BTRFS_I(inode), start, *(u32 *)csum,
+				    *(u32 *)csum_expected, io_bio->mirror_num);
 	memset(kaddr + pgoff, 1, len);
 	flush_dcache_page(page);
 	kunmap_atomic(kaddr);
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 2cf3cf9e9c9b..65bd4999c7ad 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -6,6 +6,7 @@
 #include <linux/blkdev.h>
 #include <linux/ratelimit.h>
 #include <linux/sched/mm.h>
+#include <crypto/hash.h>
 #include "ctree.h"
 #include "volumes.h"
 #include "disk-io.h"
@@ -1787,11 +1788,12 @@ static int scrub_checksum(struct scrub_block *sblock)
 static int scrub_checksum_data(struct scrub_block *sblock)
 {
 	struct scrub_ctx *sctx = sblock->sctx;
+	struct btrfs_fs_info *fs_info = sctx->fs_info;
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	u8 csum[BTRFS_CSUM_SIZE];
 	u8 *on_disk_csum;
 	struct page *page;
 	void *buffer;
-	u32 crc = ~(u32)0;
 	u64 len;
 	int index;
 
@@ -1799,16 +1801,22 @@ static int scrub_checksum_data(struct scrub_block *sblock)
 	if (!sblock->pagev[0]->have_csum)
 		return 0;
 
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+
+	crypto_shash_init(shash);
+
 	on_disk_csum = sblock->pagev[0]->csum;
 	page = sblock->pagev[0]->page;
 	buffer = kmap_atomic(page);
 
+	memset(csum, 0xff, btrfs_super_csum_size(sctx->fs_info->super_copy));
 	len = sctx->fs_info->sectorsize;
 	index = 0;
 	for (;;) {
 		u64 l = min_t(u64, len, PAGE_SIZE);
 
-		crc = btrfs_csum_data(buffer, crc, l);
+		crypto_shash_update(shash, buffer, l);
 		kunmap_atomic(buffer);
 		len -= l;
 		if (len == 0)
@@ -1820,7 +1828,7 @@ static int scrub_checksum_data(struct scrub_block *sblock)
 		buffer = kmap_atomic(page);
 	}
 
-	btrfs_csum_final(crc, csum);
+	crypto_shash_final(shash, csum);
 	if (memcmp(csum, on_disk_csum, sctx->csum_size))
 		sblock->checksum_error = 1;
 
@@ -1832,16 +1840,21 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 	struct scrub_ctx *sctx = sblock->sctx;
 	struct btrfs_header *h;
 	struct btrfs_fs_info *fs_info = sctx->fs_info;
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	u8 calculated_csum[BTRFS_CSUM_SIZE];
 	u8 on_disk_csum[BTRFS_CSUM_SIZE];
 	struct page *page;
 	void *mapped_buffer;
 	u64 mapped_size;
 	void *p;
-	u32 crc = ~(u32)0;
 	u64 len;
 	int index;
 
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+
+	crypto_shash_init(shash);
+
 	BUG_ON(sblock->page_count < 1);
 	page = sblock->pagev[0]->page;
 	mapped_buffer = kmap_atomic(page);
@@ -1875,7 +1888,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 	for (;;) {
 		u64 l = min_t(u64, len, mapped_size);
 
-		crc = btrfs_csum_data(p, crc, l);
+		crypto_shash_update(shash, p, l);
 		kunmap_atomic(mapped_buffer);
 		len -= l;
 		if (len == 0)
@@ -1889,7 +1902,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 		p = mapped_buffer;
 	}
 
-	btrfs_csum_final(crc, calculated_csum);
+	crypto_shash_final(shash, calculated_csum);
 	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
 		sblock->checksum_error = 1;
 
@@ -1900,18 +1913,24 @@ static int scrub_checksum_super(struct scrub_block *sblock)
 {
 	struct btrfs_super_block *s;
 	struct scrub_ctx *sctx = sblock->sctx;
+	struct btrfs_fs_info *fs_info = sctx->fs_info;
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	u8 calculated_csum[BTRFS_CSUM_SIZE];
 	u8 on_disk_csum[BTRFS_CSUM_SIZE];
 	struct page *page;
 	void *mapped_buffer;
 	u64 mapped_size;
 	void *p;
-	u32 crc = ~(u32)0;
 	int fail_gen = 0;
 	int fail_cor = 0;
 	u64 len;
 	int index;
 
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+
+	crypto_shash_init(shash);
+
 	BUG_ON(sblock->page_count < 1);
 	page = sblock->pagev[0]->page;
 	mapped_buffer = kmap_atomic(page);
@@ -1934,7 +1953,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
 	for (;;) {
 		u64 l = min_t(u64, len, mapped_size);
 
-		crc = btrfs_csum_data(p, crc, l);
+		crypto_shash_update(shash, p, l);
 		kunmap_atomic(mapped_buffer);
 		len -= l;
 		if (len == 0)
@@ -1948,7 +1967,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
 		p = mapped_buffer;
 	}
 
-	btrfs_csum_final(crc, calculated_csum);
+	crypto_shash_final(shash, calculated_csum);
 	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
 		++fail_cor;
 
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 2c66d9ea6a3b..f40516ca5963 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2465,3 +2465,4 @@ late_initcall(init_btrfs_fs);
 module_exit(exit_btrfs_fs)
 
 MODULE_LICENSE("GPL");
+MODULE_SOFTDEP("pre: crc32c");
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 12/13] btrfs: remove assumption about csum type form btrfs_print_data_csum_error()
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (10 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-22  8:19 ` [RFC PATCH v3 13/13] btrfs: add sha256 as another checksum algorithm Johannes Thumshirn
  2019-05-27 17:19 ` [PATCH v3 00/13] Add support for other checksums David Sterba
  13 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

btrfs_print_data_csum_error() still assumed checksums to be 32 bit in size.

Make it size agnostic.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/btrfs_inode.h | 6 +++---
 fs/btrfs/compression.c | 2 +-
 fs/btrfs/inode.c       | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index f0a757eb5744..e79fd9129075 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -338,13 +338,13 @@ static inline void btrfs_inode_resume_unlocked_dio(struct btrfs_inode *inode)
 }
 
 static inline void btrfs_csum_format(struct btrfs_super_block *sb,
-				     u32 csum, u8 *cbuf)
+				     u8 *csum, u8 *cbuf)
 {
 	size_t size = btrfs_super_csum_size(sb) * 8;
 
 	switch (btrfs_super_csum_type(sb)) {
 	case BTRFS_CSUM_TYPE_CRC32:
-		snprintf(cbuf, size, "0x%08x", csum);
+		snprintf(cbuf, size, "0x%08x", *(u32 *)csum);
 		break;
 	default: /* can't happen -  csum type is validated at mount time */
 		break;
@@ -352,7 +352,7 @@ static inline void btrfs_csum_format(struct btrfs_super_block *sb,
 }
 
 static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
-		u64 logical_start, u32 csum, u32 csum_expected, int mirror_num)
+		u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num)
 {
 	struct btrfs_root *root = inode->root;
 	struct btrfs_super_block *sb = root->fs_info->super_copy;
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index e027e58358be..358a85d77108 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -86,7 +86,7 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 
 		if (memcmp(&csum, cb_sum, csum_size)) {
 			btrfs_print_data_csum_error(inode, disk_start,
-						    *(u32 *)csum, *(u32 *)cb_sum,
+						    csum, cb_sum,
 						    cb->mirror_num);
 			ret = -EIO;
 			goto fail;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 402c9ea8239d..af27dddcb05f 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3226,8 +3226,8 @@ static int __readpage_endio_check(struct inode *inode,
 	kunmap_atomic(kaddr);
 	return 0;
 zeroit:
-	btrfs_print_data_csum_error(BTRFS_I(inode), start, *(u32 *)csum,
-				    *(u32 *)csum_expected, io_bio->mirror_num);
+	btrfs_print_data_csum_error(BTRFS_I(inode), start, csum, csum_expected,
+				    io_bio->mirror_num);
 	memset(kaddr + pgoff, 1, len);
 	flush_dcache_page(page);
 	kunmap_atomic(kaddr);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH v3 13/13] btrfs: add sha256 as another checksum algorithm
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (11 preceding siblings ...)
  2019-05-22  8:19 ` [PATCH v3 12/13] btrfs: remove assumption about csum type form btrfs_print_data_csum_error() Johannes Thumshirn
@ 2019-05-22  8:19 ` Johannes Thumshirn
  2019-05-27 17:10   ` David Sterba
  2019-05-27 17:19 ` [PATCH v3 00/13] Add support for other checksums David Sterba
  13 siblings, 1 reply; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:19 UTC (permalink / raw)
  To: David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger,
	David Gstir, Nikolay Borisov, Johannes Thumshirn

Now that we everything in place, we can add SHA-256 as another checksum
algorithm.

SHA-256 was taken as it was the cryptographically strongest algorithm that
can fit into the 32 Bytes we have left.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

---
changes to v2:
- Add pre dependency on sha256
Changes to v1:
- Select SHA-256 in KConfig
- Minimalize switch() in btrfs_supported_super_csum() (Nikolay)
- Use enum for new on-disk checksum type (Nikolay/David)
- Format SHA256 using sprintf()'s hexdump mode
---
 fs/btrfs/Kconfig                | 1 +
 fs/btrfs/btrfs_inode.h          | 3 +++
 fs/btrfs/ctree.h                | 4 ++--
 fs/btrfs/disk-io.c              | 1 +
 fs/btrfs/super.c                | 1 +
 include/uapi/linux/btrfs_tree.h | 6 ++++--
 6 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index 212b4a854f2c..2521a24f74be 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -4,6 +4,7 @@ config BTRFS_FS
 	tristate "Btrfs filesystem support"
 	select CRYPTO
 	select CRYPTO_CRC32C
+	select CRYPTO_SHA256
 	select ZLIB_INFLATE
 	select ZLIB_DEFLATE
 	select LZO_COMPRESS
diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index e79fd9129075..125bc7f3b871 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -346,6 +346,9 @@ static inline void btrfs_csum_format(struct btrfs_super_block *sb,
 	case BTRFS_CSUM_TYPE_CRC32:
 		snprintf(cbuf, size, "0x%08x", *(u32 *)csum);
 		break;
+	case BTRFS_CSUM_TYPE_SHA256:
+		snprintf(cbuf, size, "%*phN", (int)size / 8, csum);
+		break;
 	default: /* can't happen -  csum type is validated at mount time */
 		break;
 	}
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8b635ca370f5..f2246aa2596a 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -72,8 +72,8 @@ struct btrfs_ref;
 #define BTRFS_LINK_MAX 65535U
 
 /* four bytes for CRC32 */
-static const int btrfs_csum_sizes[] = { 4 };
-static char *btrfs_csum_names[] = { "crc32c" };
+static const int btrfs_csum_sizes[] = { 4, 32 };
+static char *btrfs_csum_names[] = { "crc32c", "sha256" };
 
 #define BTRFS_EMPTY_DIR_SIZE 0
 
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 68cdee36b470..0c7ea5c7f0db 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -357,6 +357,7 @@ static bool btrfs_supported_super_csum(u16 csum_type)
 {
 	switch (csum_type) {
 	case BTRFS_CSUM_TYPE_CRC32:
+	case BTRFS_CSUM_TYPE_SHA256:
 		return true;
 	default:
 		return false;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index f40516ca5963..5d3687354fc7 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2466,3 +2466,4 @@ module_exit(exit_btrfs_fs)
 
 MODULE_LICENSE("GPL");
 MODULE_SOFTDEP("pre: crc32c");
+MODULE_SOFTDEP("pre: sha256");
diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index 421239b98db2..8edb624410cb 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -300,8 +300,10 @@
 #define BTRFS_CSUM_SIZE 32
 
 /* csum types */
-#define BTRFS_CSUM_TYPE_CRC32	0
-
+enum {
+	BTRFS_CSUM_TYPE_CRC32	= 0,
+	BTRFS_CSUM_TYPE_SHA256	= 1,
+};
 /*
  * flags definitions for directory entry item type
  *
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming
  2019-05-22  8:19 ` [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
@ 2019-05-22  8:33   ` Nikolay Borisov
  2019-05-22  8:35     ` Johannes Thumshirn
  2019-05-22  9:04     ` Johannes Thumshirn
  2019-05-29 19:32   ` David Gstir
  1 sibling, 2 replies; 28+ messages in thread
From: Nikolay Borisov @ 2019-05-22  8:33 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba
  Cc: Linux BTRFS Mailinglist, Chris Mason, Richard Weinberger, David Gstir



On 22.05.19 г. 11:19 ч., Johannes Thumshirn wrote:
> Currently btrfs_csum_data() relied on the crc32c() wrapper around the crypto
> framework for calculating the CRCs.
> 
> As we have our own crypto_shash structure in the fs_info now, we can
> directly call into the crypto framework without going trough the wrapper.
> 
> This way we can even remove the btrfs_csum_data() and btrfs_csum_final()
> wrappers.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> 
> ---
> Changes to v2:
> - Also select CRYPTO in Kconfig
> - Add pre dependency on crc32c
> Changes to v1:
> - merge with 'btrfs: pass in an fs_info to btrfs_csum_{data,final}()'
> - Remove btrfs_csum_data() and btrfs_csum_final() alltogether
> - don't use LIBCRC32C but CRYPTO_CRC32C in KConfig
> ---
>  fs/btrfs/Kconfig           |  3 ++-
>  fs/btrfs/check-integrity.c | 12 +++++++----
>  fs/btrfs/compression.c     | 19 +++++++++++------
>  fs/btrfs/disk-io.c         | 51 +++++++++++++++++++++++++---------------------
>  fs/btrfs/disk-io.h         |  2 --
>  fs/btrfs/file-item.c       | 18 ++++++++--------
>  fs/btrfs/inode.c           | 24 ++++++++++++++--------
>  fs/btrfs/scrub.c           | 37 +++++++++++++++++++++++++--------
>  fs/btrfs/super.c           |  1 +
>  9 files changed, 106 insertions(+), 61 deletions(-)
> 

<snip>

> @@ -1799,16 +1801,22 @@ static int scrub_checksum_data(struct scrub_block *sblock)
>  	if (!sblock->pagev[0]->have_csum)
>  		return 0;
>  
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> +	crypto_shash_init(shash);
> +
>  	on_disk_csum = sblock->pagev[0]->csum;
>  	page = sblock->pagev[0]->page;
>  	buffer = kmap_atomic(page);
>  
> +	memset(csum, 0xff, btrfs_super_csum_size(sctx->fs_info->super_copy));

Is this required? You don't do it in other place like
scrub_checksum_tree_block/scrub_checksum_super/__readpage_endio_check.
If it's not strictly require just drop it.

>  	len = sctx->fs_info->sectorsize;
>  	index = 0;
>  	for (;;) {
>  		u64 l = min_t(u64, len, PAGE_SIZE);
>  
> -		crc = btrfs_csum_data(buffer, crc, l);
> +		crypto_shash_update(shash, buffer, l);
>  		kunmap_atomic(buffer);
>  		len -= l;
>  		if (len == 0)
> @@ -1820,7 +1828,7 @@ static int scrub_checksum_data(struct scrub_block *sblock)
>  		buffer = kmap_atomic(page);
>  	}
>  
> -	btrfs_csum_final(crc, csum);
> +	crypto_shash_final(shash, csum);
>  	if (memcmp(csum, on_disk_csum, sctx->csum_size))
>  		sblock->checksum_error = 1;
>  
> @@ -1832,16 +1840,21 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
>  	struct scrub_ctx *sctx = sblock->sctx;
>  	struct btrfs_header *h;
>  	struct btrfs_fs_info *fs_info = sctx->fs_info;
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>  	u8 calculated_csum[BTRFS_CSUM_SIZE];
>  	u8 on_disk_csum[BTRFS_CSUM_SIZE];
>  	struct page *page;
>  	void *mapped_buffer;
>  	u64 mapped_size;
>  	void *p;
> -	u32 crc = ~(u32)0;
>  	u64 len;
>  	int index;
>  
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> +	crypto_shash_init(shash);
> +
>  	BUG_ON(sblock->page_count < 1);
>  	page = sblock->pagev[0]->page;
>  	mapped_buffer = kmap_atomic(page);
> @@ -1875,7 +1888,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
>  	for (;;) {
>  		u64 l = min_t(u64, len, mapped_size);
>  
> -		crc = btrfs_csum_data(p, crc, l);
> +		crypto_shash_update(shash, p, l);
>  		kunmap_atomic(mapped_buffer);
>  		len -= l;
>  		if (len == 0)
> @@ -1889,7 +1902,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
>  		p = mapped_buffer;
>  	}
>  
> -	btrfs_csum_final(crc, calculated_csum);
> +	crypto_shash_final(shash, calculated_csum);
>  	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
>  		sblock->checksum_error = 1;
>  
> @@ -1900,18 +1913,24 @@ static int scrub_checksum_super(struct scrub_block *sblock)
>  {
>  	struct btrfs_super_block *s;
>  	struct scrub_ctx *sctx = sblock->sctx;
> +	struct btrfs_fs_info *fs_info = sctx->fs_info;
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>  	u8 calculated_csum[BTRFS_CSUM_SIZE];
>  	u8 on_disk_csum[BTRFS_CSUM_SIZE];
>  	struct page *page;
>  	void *mapped_buffer;
>  	u64 mapped_size;
>  	void *p;
> -	u32 crc = ~(u32)0;
>  	int fail_gen = 0;
>  	int fail_cor = 0;
>  	u64 len;
>  	int index;
>  
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> +	crypto_shash_init(shash);
> +
>  	BUG_ON(sblock->page_count < 1);
>  	page = sblock->pagev[0]->page;
>  	mapped_buffer = kmap_atomic(page);
> @@ -1934,7 +1953,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
>  	for (;;) {
>  		u64 l = min_t(u64, len, mapped_size);
>  
> -		crc = btrfs_csum_data(p, crc, l);
> +		crypto_shash_update(shash, p, l);
>  		kunmap_atomic(mapped_buffer);
>  		len -= l;
>  		if (len == 0)
> @@ -1948,7 +1967,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
>  		p = mapped_buffer;
>  	}
>  
> -	btrfs_csum_final(crc, calculated_csum);
> +	crypto_shash_final(shash, calculated_csum);
>  	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
>  		++fail_cor;
>  
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 2c66d9ea6a3b..f40516ca5963 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2465,3 +2465,4 @@ late_initcall(init_btrfs_fs);
>  module_exit(exit_btrfs_fs)
>  
>  MODULE_LICENSE("GPL");
> +MODULE_SOFTDEP("pre: crc32c");
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming
  2019-05-22  8:33   ` Nikolay Borisov
@ 2019-05-22  8:35     ` Johannes Thumshirn
  2019-05-22  9:04     ` Johannes Thumshirn
  1 sibling, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  8:35 UTC (permalink / raw)
  To: Nikolay Borisov
  Cc: David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir

On Wed, May 22, 2019 at 11:33:39AM +0300, Nikolay Borisov wrote:
> 
> > @@ -1799,16 +1801,22 @@ static int scrub_checksum_data(struct scrub_block *sblock)
> >  	if (!sblock->pagev[0]->have_csum)
> >  		return 0;
> >  
> > +	shash->tfm = fs_info->csum_shash;
> > +	shash->flags = 0;
> > +
> > +	crypto_shash_init(shash);
> > +
> >  	on_disk_csum = sblock->pagev[0]->csum;
> >  	page = sblock->pagev[0]->page;
> >  	buffer = kmap_atomic(page);
> >  
> > +	memset(csum, 0xff, btrfs_super_csum_size(sctx->fs_info->super_copy));
> 
> Is this required? You don't do it in other place like
> scrub_checksum_tree_block/scrub_checksum_super/__readpage_endio_check.
> If it's not strictly require just drop it.

I guess this is a leftover, thanks for spotting it.

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming
  2019-05-22  8:33   ` Nikolay Borisov
  2019-05-22  8:35     ` Johannes Thumshirn
@ 2019-05-22  9:04     ` Johannes Thumshirn
  1 sibling, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-05-22  9:04 UTC (permalink / raw)
  To: Nikolay Borisov
  Cc: David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir

On Wed, May 22, 2019 at 11:33:39AM +0300, Nikolay Borisov wrote:
> > +	memset(csum, 0xff, btrfs_super_csum_size(sctx->fs_info->super_copy));

David can you fold this in?

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 65bd4999c7ad..d97039338952 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -1810,7 +1810,6 @@ static int scrub_checksum_data(struct scrub_block *sblock)
 	page = sblock->pagev[0]->page;
 	buffer = kmap_atomic(page);
 
-	memset(csum, 0xff, btrfs_super_csum_size(sctx->fs_info->super_copy));
 	len = sctx->fs_info->sectorsize;
 	index = 0;
 	for (;;) {
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 06/13] btrfs: format checksums according to type for printing
  2019-05-22  8:19 ` [PATCH v3 06/13] btrfs: format checksums according to type for printing Johannes Thumshirn
@ 2019-05-27 16:57   ` David Sterba
  2019-06-03  9:33     ` Johannes Thumshirn
  0 siblings, 1 reply; 28+ messages in thread
From: David Sterba @ 2019-05-27 16:57 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir, Nikolay Borisov

On Wed, May 22, 2019 at 10:19:03AM +0200, Johannes Thumshirn wrote:
> Add a small helper for btrfs_print_data_csum_error() which formats the
> checksum according to it's type for pretty printing.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/btrfs_inode.h | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index d5b438706b77..f0a757eb5744 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -337,22 +337,42 @@ static inline void btrfs_inode_resume_unlocked_dio(struct btrfs_inode *inode)
>  	clear_bit(BTRFS_INODE_READDIO_NEED_LOCK, &inode->runtime_flags);
>  }
>  
> +static inline void btrfs_csum_format(struct btrfs_super_block *sb,
> +				     u32 csum, u8 *cbuf)
> +{
> +	size_t size = btrfs_super_csum_size(sb) * 8;
> +
> +	switch (btrfs_super_csum_type(sb)) {
> +	case BTRFS_CSUM_TYPE_CRC32:
> +		snprintf(cbuf, size, "0x%08x", csum);
> +		break;
> +	default: /* can't happen -  csum type is validated at mount time */
> +		break;
> +	}
> +}
> +
>  static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
>  		u64 logical_start, u32 csum, u32 csum_expected, int mirror_num)
>  {
>  	struct btrfs_root *root = inode->root;
> +	struct btrfs_super_block *sb = root->fs_info->super_copy;
> +	u8 cbuf[BTRFS_CSUM_SIZE];
> +	u8 ecbuf[BTRFS_CSUM_SIZE];
> +
> +	btrfs_csum_format(sb, csum, cbuf);
> +	btrfs_csum_format(sb, csum_expected, ecbuf);

cbuf is 32 bytes (BTRFS_CSUM_SIZE) for the binary representation, while
you want reserve the space for the string representation, which is 2x
the size in bytes + "0x" prefix but that can be moved to the printk
format string.

So, the above is fine for crc32c the string will fit there, but with
sha256 this will cause buffer overflow, by 32 * 2 + 2 - 32 = 30 bytes.
Gotcha.

I think the helper is not needed at all, the format "%*phN" can be used
for crc32c too without the intermediate buffers. For better readability,
some macros can be added like

	"this is wrong csum " CSUM_FORMAT " end of string",
	CSUM_FORMAT_VALUE(csum_size, csum_bytes)

with CSUM_FORMAT "0x%*phN" and
CSUM_FORMAT_VALUE(size, bytes)	size, bytes

ie. just for the explict requirement of the variable length required by
"*".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH v3 13/13] btrfs: add sha256 as another checksum algorithm
  2019-05-22  8:19 ` [RFC PATCH v3 13/13] btrfs: add sha256 as another checksum algorithm Johannes Thumshirn
@ 2019-05-27 17:10   ` David Sterba
  0 siblings, 0 replies; 28+ messages in thread
From: David Sterba @ 2019-05-27 17:10 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir, Nikolay Borisov

On Wed, May 22, 2019 at 10:19:10AM +0200, Johannes Thumshirn wrote:
> Now that we everything in place, we can add SHA-256 as another checksum
> algorithm.
> 
> SHA-256 was taken as it was the cryptographically strongest algorithm that
> can fit into the 32 Bytes we have left.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> 
> ---
> changes to v2:
> - Add pre dependency on sha256
> Changes to v1:
> - Select SHA-256 in KConfig
> - Minimalize switch() in btrfs_supported_super_csum() (Nikolay)
> - Use enum for new on-disk checksum type (Nikolay/David)
> - Format SHA256 using sprintf()'s hexdump mode
> ---
>  fs/btrfs/Kconfig                | 1 +
>  fs/btrfs/btrfs_inode.h          | 3 +++
>  fs/btrfs/ctree.h                | 4 ++--
>  fs/btrfs/disk-io.c              | 1 +
>  fs/btrfs/super.c                | 1 +
>  include/uapi/linux/btrfs_tree.h | 6 ++++--
>  6 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
> index 212b4a854f2c..2521a24f74be 100644
> --- a/fs/btrfs/Kconfig
> +++ b/fs/btrfs/Kconfig
> @@ -4,6 +4,7 @@ config BTRFS_FS
>  	tristate "Btrfs filesystem support"
>  	select CRYPTO
>  	select CRYPTO_CRC32C
> +	select CRYPTO_SHA256
>  	select ZLIB_INFLATE
>  	select ZLIB_DEFLATE
>  	select LZO_COMPRESS
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index e79fd9129075..125bc7f3b871 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -346,6 +346,9 @@ static inline void btrfs_csum_format(struct btrfs_super_block *sb,
>  	case BTRFS_CSUM_TYPE_CRC32:
>  		snprintf(cbuf, size, "0x%08x", *(u32 *)csum);
>  		break;
> +	case BTRFS_CSUM_TYPE_SHA256:
> +		snprintf(cbuf, size, "%*phN", (int)size / 8, csum);

This seems to print the buffer in the host order ('h', other formats use
that). I wonder if there needs to be some caution about endianity here.
Ideally we won't need to care and just use the right format that will
printk handle transparently for us.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 00/13] Add support for other checksums
  2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
                   ` (12 preceding siblings ...)
  2019-05-22  8:19 ` [RFC PATCH v3 13/13] btrfs: add sha256 as another checksum algorithm Johannes Thumshirn
@ 2019-05-27 17:19 ` David Sterba
  2019-06-03  9:38   ` Johannes Thumshirn
  13 siblings, 1 reply; 28+ messages in thread
From: David Sterba @ 2019-05-27 17:19 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir, Nikolay Borisov

On Wed, May 22, 2019 at 10:18:57AM +0200, Johannes Thumshirn wrote:
> This patchset add support for adding new checksum types in BTRFS.
> 
> Currently BTRFS only supports CRC32C as data and metadata checksum, which is
> good if you only want to detect errors due to data corruption in hardware.
> 
> But CRC32C isn't able cover other use-cases like de-duplication or
> cryptographically save data integrity guarantees.
> 
> The following properties made SHA-256 interesting for these use-cases:
> - Still considered cryptographically sound
> - Reasonably well understood by the security industry
> - Result fits into the 32Byte/256Bit we have for the checksum in the on-disk
>   format
> - Small enough collision space to make it feasible for data de-duplication
> - Fast enough to calculate and offloadable to crypto hardware via the kernel's
>   crypto_shash framework.
> 
> The patchset also provides mechanisms for plumbing in different hash
> algorithms relatively easy.
> 
> Unfortunately this patchset also partially reverts commit: 
> 9678c54388b6 ("btrfs: Remove custom crc32c init code")
> 
> This is an intermediate submission, as a) mkfs.btrfs support is still missing
> and b) David requested to have three hash algorithms, where 1 is crc32c, one
> cryptographically secure and one in between.
> 
> A changelog can be found directly in the patches. The branch is also available
> on a gitweb at
> https://git.kernel.org/pub/scm/linux/kernel/git/jth/linux.git/log/?h=btrfs-csum-rework.v3
> 
> Johannes Thumshirn (13):
>   btrfs: use btrfs_csum_data() instead of directly calling crc32c
>   btrfs: resurrect btrfs_crc32c()
>   btrfs: use btrfs_crc32c{,_final}() in for free space cache
>   btrfs: don't assume ordered sums to be 4 bytes
>   btrfs: dont assume compressed_bio sums to be 4 bytes
>   btrfs: format checksums according to type for printing
>   btrfs: add common checksum type validation
>   btrfs: check for supported superblock checksum type before checksum
>     validation
>   btrfs: Simplify btrfs_check_super_csum() and get rid of size
>     assumptions
>   btrfs: add boilerplate code for directly including the crypto
>     framework
>   btrfs: directly call into crypto framework for checsumming
>   btrfs: remove assumption about csum type form
>     btrfs_print_data_csum_error()
>   btrfs: add sha256 as another checksum algorithm

1-5 are reviewed and ok, 6 and 13 should be reworked, 7-12 is ok. I
can't put the branch to next yet due to the csum formatting "issues" but
will do once you resend. Should be ok just 6 and 13 as they're
independent.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming
  2019-05-22  8:19 ` [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
  2019-05-22  8:33   ` Nikolay Borisov
@ 2019-05-29 19:32   ` David Gstir
  2019-05-30 10:14     ` David Sterba
  1 sibling, 1 reply; 28+ messages in thread
From: David Gstir @ 2019-05-29 19:32 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, Nikolay Borisov

Hi Johannes!

> On 22.05.2019, at 10:19, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> 
> Currently btrfs_csum_data() relied on the crc32c() wrapper around the crypto
> framework for calculating the CRCs.
> 
> As we have our own crypto_shash structure in the fs_info now, we can
> directly call into the crypto framework without going trough the wrapper.
> 
> This way we can even remove the btrfs_csum_data() and btrfs_csum_final()
> wrappers.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> 
> ---
> Changes to v2:
> - Also select CRYPTO in Kconfig
> - Add pre dependency on crc32c
> Changes to v1:
> - merge with 'btrfs: pass in an fs_info to btrfs_csum_{data,final}()'
> - Remove btrfs_csum_data() and btrfs_csum_final() alltogether
> - don't use LIBCRC32C but CRYPTO_CRC32C in KConfig
> ---
> fs/btrfs/Kconfig           |  3 ++-
> fs/btrfs/check-integrity.c | 12 +++++++----
> fs/btrfs/compression.c     | 19 +++++++++++------
> fs/btrfs/disk-io.c         | 51 +++++++++++++++++++++++++---------------------
> fs/btrfs/disk-io.h         |  2 --
> fs/btrfs/file-item.c       | 18 ++++++++--------
> fs/btrfs/inode.c           | 24 ++++++++++++++--------
> fs/btrfs/scrub.c           | 37 +++++++++++++++++++++++++--------
> fs/btrfs/super.c           |  1 +
> 9 files changed, 106 insertions(+), 61 deletions(-)
> 
> diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
> index 23537bc8c827..212b4a854f2c 100644
> --- a/fs/btrfs/Kconfig
> +++ b/fs/btrfs/Kconfig
> @@ -2,7 +2,8 @@
> 
> config BTRFS_FS
> 	tristate "Btrfs filesystem support"
> -	select LIBCRC32C
> +	select CRYPTO
> +	select CRYPTO_CRC32C
> 	select ZLIB_INFLATE
> 	select ZLIB_DEFLATE
> 	select LZO_COMPRESS
> diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
> index 85774e2fa3e5..6adee8fe82e2 100644
> --- a/fs/btrfs/check-integrity.c
> +++ b/fs/btrfs/check-integrity.c
> @@ -83,7 +83,7 @@
> #include <linux/blkdev.h>
> #include <linux/mm.h>
> #include <linux/string.h>
> -#include <linux/crc32c.h>
> +#include <crypto/hash.h>
> #include "ctree.h"
> #include "disk-io.h"
> #include "transaction.h"
> @@ -1710,9 +1710,9 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
> 				     char **datav, unsigned int num_pages)
> {
> 	struct btrfs_fs_info *fs_info = state->fs_info;
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> 	struct btrfs_header *h;
> 	u8 csum[BTRFS_CSUM_SIZE];
> -	u32 crc = ~(u32)0;
> 	unsigned int i;
> 
> 	if (num_pages * PAGE_SIZE < state->metablock_size)
> @@ -1723,14 +1723,18 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
> 	if (memcmp(h->fsid, fs_info->fs_devices->fsid, BTRFS_FSID_SIZE))
> 		return 1;
> 
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +	crypto_shash_init(shash);
> +
> 	for (i = 0; i < num_pages; i++) {
> 		u8 *data = i ? datav[i] : (datav[i] + BTRFS_CSUM_SIZE);
> 		size_t sublen = i ? PAGE_SIZE :
> 				    (PAGE_SIZE - BTRFS_CSUM_SIZE);
> 
> -		crc = btrfs_csum_data(data, crc, sublen);
> +		crypto_shash_update(shash, data, sublen);
> 	}
> -	btrfs_csum_final(crc, csum);
> +	crypto_shash_final(shash, csum);
> 	if (memcmp(csum, h->csum, state->csum_size))
> 		return 1;
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index d5642f3b5c44..e027e58358be 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -17,6 +17,7 @@
> #include <linux/slab.h>
> #include <linux/sched/mm.h>
> #include <linux/log2.h>
> +#include <crypto/hash.h>
> #include "ctree.h"
> #include "disk-io.h"
> #include "transaction.h"
> @@ -58,29 +59,35 @@ static int check_compressed_csum(struct btrfs_inode *inode,
> 				 u64 disk_start)
> {
> 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> 	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> 	int ret;
> 	struct page *page;
> 	unsigned long i;
> 	char *kaddr;
> -	u32 csum;
> +	u8 csum[BTRFS_CSUM_SIZE];
> 	u8 *cb_sum = cb->sums;
> 
> 	if (inode->flags & BTRFS_INODE_NODATASUM)
> 		return 0;
> 
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> 	for (i = 0; i < cb->nr_pages; i++) {
> 		page = cb->compressed_pages[i];
> -		csum = ~(u32)0;
> +
> +		crypto_shash_init(shash);
> 
> 		kaddr = kmap_atomic(page);
> -		csum = btrfs_csum_data(kaddr, csum, PAGE_SIZE);
> -		btrfs_csum_final(csum, (u8 *)&csum);
> +		crypto_shash_update(shash, kaddr, PAGE_SIZE);
> +		crypto_shash_final(shash, (u8 *)&csum);
> 		kunmap_atomic(kaddr);
> 
> 		if (memcmp(&csum, cb_sum, csum_size)) {
> -			btrfs_print_data_csum_error(inode, disk_start, csum,
> -					*(u32 *)cb_sum, cb->mirror_num);
> +			btrfs_print_data_csum_error(inode, disk_start,
> +						    *(u32 *)csum, *(u32 *)cb_sum,
> +						    cb->mirror_num);
> 			ret = -EIO;
> 			goto fail;
> 		}
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index f73660e63522..68cdee36b470 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -250,16 +250,6 @@ struct extent_map *btree_get_extent(struct btrfs_inode *inode,
> 	return em;
> }
> 
> -u32 btrfs_csum_data(const char *data, u32 seed, size_t len)
> -{
> -	return crc32c(seed, data, len);
> -}
> -
> -void btrfs_csum_final(u32 crc, u8 *result)
> -{
> -	put_unaligned_le32(~crc, result);
> -}
> -
> /*
>  * Compute the csum of a btree block and store the result to provided buffer.
>  *
> @@ -267,6 +257,8 @@ void btrfs_csum_final(u32 crc, u8 *result)
>  */
> static int csum_tree_block(struct extent_buffer *buf, u8 *result)
> {
> +	struct btrfs_fs_info *fs_info = buf->fs_info;
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> 	unsigned long len;
> 	unsigned long cur_len;
> 	unsigned long offset = BTRFS_CSUM_SIZE;
> @@ -274,9 +266,14 @@ static int csum_tree_block(struct extent_buffer *buf, u8 *result)
> 	unsigned long map_start;
> 	unsigned long map_len;
> 	int err;
> -	u32 crc = ~(u32)0;
> +
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> +	crypto_shash_init(shash);
> 
> 	len = buf->len - offset;
> +
> 	while (len > 0) {
> 		/*
> 		 * Note: we don't need to check for the err == 1 case here, as
> @@ -289,14 +286,13 @@ static int csum_tree_block(struct extent_buffer *buf, u8 *result)
> 		if (WARN_ON(err))
> 			return err;
> 		cur_len = min(len, map_len - (offset - map_start));
> -		crc = btrfs_csum_data(kaddr + offset - map_start,
> -				      crc, cur_len);
> +		crypto_shash_update(shash, kaddr + offset - map_start, cur_len);
> 		len -= cur_len;
> 		offset += cur_len;
> 	}
> 	memset(result, 0, BTRFS_CSUM_SIZE);
> 
> -	btrfs_csum_final(crc, result);
> +	crypto_shash_final(shash, result);
> 
> 	return 0;
> }
> @@ -376,17 +372,22 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
> {
> 	struct btrfs_super_block *disk_sb =
> 		(struct btrfs_super_block *)raw_disk_sb;
> -	u32 crc = ~(u32)0;
> 	char result[BTRFS_CSUM_SIZE];
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> +
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> +	crypto_shash_init(shash);
> 
> 	/*
> 	 * The super_block structure does not span the whole
> 	 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
> 	 * is filled with zeros and is included in the checksum.
> 	 */
> -	crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
> -			      crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> -	btrfs_csum_final(crc, result);
> +	crypto_shash_update(shash, raw_disk_sb + BTRFS_CSUM_SIZE,
> +			    BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> +	crypto_shash_final(shash, result);
> 
> 	if (memcmp(disk_sb->csum, result, btrfs_super_csum_size(disk_sb)))
> 		return 1;
> @@ -3514,17 +3515,21 @@ struct buffer_head *btrfs_read_dev_super(struct block_device *bdev)
> static int write_dev_supers(struct btrfs_device *device,
> 			    struct btrfs_super_block *sb, int max_mirrors)
> {
> +	struct btrfs_fs_info *fs_info = device->fs_info;
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> 	struct buffer_head *bh;
> 	int i;
> 	int ret;
> 	int errors = 0;
> -	u32 crc;
> 	u64 bytenr;
> 	int op_flags;
> 
> 	if (max_mirrors == 0)
> 		max_mirrors = BTRFS_SUPER_MIRROR_MAX;
> 
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> 	for (i = 0; i < max_mirrors; i++) {
> 		bytenr = btrfs_sb_offset(i);
> 		if (bytenr + BTRFS_SUPER_INFO_SIZE >=
> @@ -3533,10 +3538,10 @@ static int write_dev_supers(struct btrfs_device *device,
> 
> 		btrfs_set_super_bytenr(sb, bytenr);
> 
> -		crc = ~(u32)0;
> -		crc = btrfs_csum_data((const char *)sb + BTRFS_CSUM_SIZE, crc,
> -				      BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> -		btrfs_csum_final(crc, sb->csum);
> +		crypto_shash_init(shash);
> +		crypto_shash_update(shash, (const char *)sb + BTRFS_CSUM_SIZE,
> +				    BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> +		crypto_shash_final(shash, sb->csum);
> 
> 		/* One reference for us, and we leave it for the caller */
> 		bh = __getblk(device->bdev, bytenr / BTRFS_BDEV_BLOCKSIZE,
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index a0161aa1ea0b..e80f7c45a307 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -115,8 +115,6 @@ int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,
> 			  int atomic);
> int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid, int level,
> 		      struct btrfs_key *first_key);
> -u32 btrfs_csum_data(const char *data, u32 seed, size_t len);
> -void btrfs_csum_final(u32 crc, u8 *result);
> blk_status_t btrfs_bio_wq_end_io(struct btrfs_fs_info *info, struct bio *bio,
> 			enum btrfs_wq_endio_type metadata);
> blk_status_t btrfs_wq_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 0bb77392ec08..ba2405b4931b 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -8,6 +8,7 @@
> #include <linux/pagemap.h>
> #include <linux/highmem.h>
> #include <linux/sched/mm.h>
> +#include <crypto/hash.h>
> #include "ctree.h"
> #include "disk-io.h"
> #include "transaction.h"
> @@ -432,6 +433,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
> 		       u64 file_start, int contig)
> {
> 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> 	struct btrfs_ordered_sum *sums;
> 	struct btrfs_ordered_extent *ordered = NULL;
> 	char *data;
> @@ -465,6 +467,9 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
> 	sums->bytenr = (u64)bio->bi_iter.bi_sector << 9;
> 	index = 0;
> 
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> 	bio_for_each_segment(bvec, bio, iter) {
> 		if (!contig)
> 			offset = page_offset(bvec.bv_page) + bvec.bv_offset;
> @@ -479,7 +484,6 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
> 						 - 1);
> 
> 		for (i = 0; i < nr_sectors; i++) {
> -			u32 tmp;
> 			if (offset >= ordered->file_offset + ordered->len ||
> 				offset < ordered->file_offset) {
> 				unsigned long bytes_left;
> @@ -505,15 +509,13 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
> 				index = 0;
> 			}
> 
> -			memset(&sums->sums[index], 0xff, csum_size);
> +			crypto_shash_init(shash);
> 			data = kmap_atomic(bvec.bv_page);
> -			tmp = btrfs_csum_data(data + bvec.bv_offset
> -						+ (i * fs_info->sectorsize),
> -						*(u32 *)&sums->sums[index],
> -						fs_info->sectorsize);
> +			crypto_shash_update(shash, data + bvec.bv_offset
> +					    + (i * fs_info->sectorsize),
> +					    fs_info->sectorsize);
> 			kunmap_atomic(data);
> -			btrfs_csum_final(tmp,
> -					(char *)(sums->sums + index));
> +			crypto_shash_final(shash, (char *)(sums->sums + index));
> 			index += csum_size;
> 			offset += fs_info->sectorsize;
> 			this_sum_bytes += fs_info->sectorsize;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index b6d549c993f6..402c9ea8239d 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3203,23 +3203,31 @@ static int __readpage_endio_check(struct inode *inode,
> 				  int icsum, struct page *page,
> 				  int pgoff, u64 start, size_t len)
> {
> +	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> 	char *kaddr;
> -	u32 csum_expected;
> -	u32 csum = ~(u32)0;
> +	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	u8 *csum_expected;
> +	u8 csum[BTRFS_CSUM_SIZE];
> 
> -	csum_expected = *(((u32 *)io_bio->csum) + icsum);
> +	csum_expected = ((u8 *)io_bio->csum) + icsum * csum_size;
> 
> 	kaddr = kmap_atomic(page);
> -	csum = btrfs_csum_data(kaddr + pgoff, csum,  len);
> -	btrfs_csum_final(csum, (u8 *)&csum);
> -	if (csum != csum_expected)
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> +	crypto_shash_init(shash);
> +	crypto_shash_update(shash, kaddr + pgoff, len);
> +	crypto_shash_final(shash, csum);
> +
> +	if (memcmp(csum, csum_expected, csum_size))
> 		goto zeroit;
> 
> 	kunmap_atomic(kaddr);
> 	return 0;
> zeroit:
> -	btrfs_print_data_csum_error(BTRFS_I(inode), start, csum, csum_expected,
> -				    io_bio->mirror_num);
> +	btrfs_print_data_csum_error(BTRFS_I(inode), start, *(u32 *)csum,
> +				    *(u32 *)csum_expected, io_bio->mirror_num);
> 	memset(kaddr + pgoff, 1, len);
> 	flush_dcache_page(page);
> 	kunmap_atomic(kaddr);
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 2cf3cf9e9c9b..65bd4999c7ad 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -6,6 +6,7 @@
> #include <linux/blkdev.h>
> #include <linux/ratelimit.h>
> #include <linux/sched/mm.h>
> +#include <crypto/hash.h>
> #include "ctree.h"
> #include "volumes.h"
> #include "disk-io.h"
> @@ -1787,11 +1788,12 @@ static int scrub_checksum(struct scrub_block *sblock)
> static int scrub_checksum_data(struct scrub_block *sblock)
> {
> 	struct scrub_ctx *sctx = sblock->sctx;
> +	struct btrfs_fs_info *fs_info = sctx->fs_info;
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> 	u8 csum[BTRFS_CSUM_SIZE];
> 	u8 *on_disk_csum;
> 	struct page *page;
> 	void *buffer;
> -	u32 crc = ~(u32)0;
> 	u64 len;
> 	int index;
> 
> @@ -1799,16 +1801,22 @@ static int scrub_checksum_data(struct scrub_block *sblock)
> 	if (!sblock->pagev[0]->have_csum)
> 		return 0;
> 
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> +	crypto_shash_init(shash);
> +
> 	on_disk_csum = sblock->pagev[0]->csum;
> 	page = sblock->pagev[0]->page;
> 	buffer = kmap_atomic(page);
> 
> +	memset(csum, 0xff, btrfs_super_csum_size(sctx->fs_info->super_copy));
> 	len = sctx->fs_info->sectorsize;
> 	index = 0;
> 	for (;;) {
> 		u64 l = min_t(u64, len, PAGE_SIZE);
> 
> -		crc = btrfs_csum_data(buffer, crc, l);
> +		crypto_shash_update(shash, buffer, l);
> 		kunmap_atomic(buffer);
> 		len -= l;
> 		if (len == 0)
> @@ -1820,7 +1828,7 @@ static int scrub_checksum_data(struct scrub_block *sblock)
> 		buffer = kmap_atomic(page);
> 	}
> 
> -	btrfs_csum_final(crc, csum);
> +	crypto_shash_final(shash, csum);
> 	if (memcmp(csum, on_disk_csum, sctx->csum_size))
> 		sblock->checksum_error = 1;
> 
> @@ -1832,16 +1840,21 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
> 	struct scrub_ctx *sctx = sblock->sctx;
> 	struct btrfs_header *h;
> 	struct btrfs_fs_info *fs_info = sctx->fs_info;
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> 	u8 calculated_csum[BTRFS_CSUM_SIZE];
> 	u8 on_disk_csum[BTRFS_CSUM_SIZE];
> 	struct page *page;
> 	void *mapped_buffer;
> 	u64 mapped_size;
> 	void *p;
> -	u32 crc = ~(u32)0;
> 	u64 len;
> 	int index;
> 
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> +	crypto_shash_init(shash);
> +
> 	BUG_ON(sblock->page_count < 1);
> 	page = sblock->pagev[0]->page;
> 	mapped_buffer = kmap_atomic(page);
> @@ -1875,7 +1888,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
> 	for (;;) {
> 		u64 l = min_t(u64, len, mapped_size);
> 
> -		crc = btrfs_csum_data(p, crc, l);
> +		crypto_shash_update(shash, p, l);
> 		kunmap_atomic(mapped_buffer);
> 		len -= l;
> 		if (len == 0)
> @@ -1889,7 +1902,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
> 		p = mapped_buffer;
> 	}
> 
> -	btrfs_csum_final(crc, calculated_csum);
> +	crypto_shash_final(shash, calculated_csum);
> 	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
> 		sblock->checksum_error = 1;
> 
> @@ -1900,18 +1913,24 @@ static int scrub_checksum_super(struct scrub_block *sblock)
> {
> 	struct btrfs_super_block *s;
> 	struct scrub_ctx *sctx = sblock->sctx;
> +	struct btrfs_fs_info *fs_info = sctx->fs_info;
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> 	u8 calculated_csum[BTRFS_CSUM_SIZE];
> 	u8 on_disk_csum[BTRFS_CSUM_SIZE];
> 	struct page *page;
> 	void *mapped_buffer;
> 	u64 mapped_size;
> 	void *p;
> -	u32 crc = ~(u32)0;
> 	int fail_gen = 0;
> 	int fail_cor = 0;
> 	u64 len;
> 	int index;
> 
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +
> +	crypto_shash_init(shash);
> +
> 	BUG_ON(sblock->page_count < 1);
> 	page = sblock->pagev[0]->page;
> 	mapped_buffer = kmap_atomic(page);
> @@ -1934,7 +1953,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
> 	for (;;) {
> 		u64 l = min_t(u64, len, mapped_size);
> 
> -		crc = btrfs_csum_data(p, crc, l);
> +		crypto_shash_update(shash, p, l);
> 		kunmap_atomic(mapped_buffer);
> 		len -= l;
> 		if (len == 0)
> @@ -1948,7 +1967,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
> 		p = mapped_buffer;
> 	}
> 
> -	btrfs_csum_final(crc, calculated_csum);
> +	crypto_shash_final(shash, calculated_csum);
> 	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
> 		++fail_cor;
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 2c66d9ea6a3b..f40516ca5963 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2465,3 +2465,4 @@ late_initcall(init_btrfs_fs);
> module_exit(exit_btrfs_fs)
> 
> MODULE_LICENSE("GPL");
> +MODULE_SOFTDEP("pre: crc32c");
> -- 
> 2.16.4
> 

If you aim for using as many of the hardware drivers as possible,
it might be better to use the ahash API, since some drivers
(eg. CAAM on NXP's i.MX) only implement that API and not shash.
Looking at crypto_ahash_init_tfm(...) in crypto/ahash.c using
drivers that implement shash through the ahash API should work
fine though.

I just found that out myself today [1]. ;)

- David

[1] https://lore.kernel.org/linux-crypto/729A4150-93A0-456B-B7AB-6D3A446E600E@sigma-star.at/T/#u

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming
  2019-05-29 19:32   ` David Gstir
@ 2019-05-30 10:14     ` David Sterba
  2019-05-30 10:27       ` David Gstir
  0 siblings, 1 reply; 28+ messages in thread
From: David Sterba @ 2019-05-30 10:14 UTC (permalink / raw)
  To: David Gstir
  Cc: Johannes Thumshirn, Chris Mason, Richard Weinberger,
	Nikolay Borisov, Linux BTRFS Mailinglist

On Wed, May 29, 2019 at 09:32:59PM +0200, David Gstir wrote:
> If you aim for using as many of the hardware drivers as possible,
> it might be better to use the ahash API, since some drivers
> (eg. CAAM on NXP's i.MX) only implement that API and not shash.
> Looking at crypto_ahash_init_tfm(...) in crypto/ahash.c using
> drivers that implement shash through the ahash API should work
> fine though.
> 
> I just found that out myself today [1]. ;)
> 
> - David
> 
> [1] https://lore.kernel.org/linux-crypto/729A4150-93A0-456B-B7AB-6D3A446E600E@sigma-star.at/T/#u

The thread says otherwise. Using SHASH interface for AHASH does not
work. Besides checksumming in btrfs is called from atomic contexts so
the sleeping part of the async API can't work at all (crypto_wait_req
indirectly calls schedule).

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming
  2019-05-30 10:14     ` David Sterba
@ 2019-05-30 10:27       ` David Gstir
  0 siblings, 0 replies; 28+ messages in thread
From: David Gstir @ 2019-05-30 10:27 UTC (permalink / raw)
  To: David Sterba
  Cc: Johannes Thumshirn, Chris Mason, Richard Weinberger,
	Nikolay Borisov, Linux BTRFS Mailinglist


> On 30.05.2019, at 12:14, David Sterba <dsterba@suse.cz> wrote:
> 
> On Wed, May 29, 2019 at 09:32:59PM +0200, David Gstir wrote:
>> If you aim for using as many of the hardware drivers as possible,
>> it might be better to use the ahash API, since some drivers
>> (eg. CAAM on NXP's i.MX) only implement that API and not shash.
>> Looking at crypto_ahash_init_tfm(...) in crypto/ahash.c using
>> drivers that implement shash through the ahash API should work
>> fine though.
>> 
>> I just found that out myself today [1]. ;)
>> 
>> - David
>> 
>> [1] https://lore.kernel.org/linux-crypto/729A4150-93A0-456B-B7AB-6D3A446E600E@sigma-star.at/T/#u
> 
> The thread says otherwise. Using SHASH interface for AHASH does not
> work.

Correct, but that's not what I wrote. I suggested that you can use the ahash API
instead of the shash API.

> Besides checksumming in btrfs is called from atomic contexts so
> the sleeping part of the async API can't work at all (crypto_wait_req
> indirectly calls schedule).

Yeah, you're right. I overlooked that. So the ahash API is generally not an option
in this case here.

- David

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 07/13] btrfs: add common checksum type validation
  2019-05-22  8:19 ` [PATCH v3 07/13] btrfs: add common checksum type validation Johannes Thumshirn
@ 2019-05-30 15:47   ` David Sterba
  0 siblings, 0 replies; 28+ messages in thread
From: David Sterba @ 2019-05-30 15:47 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir, Nikolay Borisov

On Wed, May 22, 2019 at 10:19:04AM +0200, Johannes Thumshirn wrote:
> Currently btrfs is only supporting CRC32C as checksumming algorithm. As
> this is about to change provide a function to validate the checksum type in
> the superblock against all possible algorithms.
> 
> This makes adding new algorithms easier as there are fewer places to adjust
> when adding new algorithms.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> Reviewed-by: David Sterba <dsterba@suse.com>
> 
> ---
> Changes to v2:
> - Only pass csum_type into btrfs_supported_csum() (David)
> - Directly return in btrfs_check_super_csum() (David)
> ---
>  fs/btrfs/disk-io.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 663efce22d98..594583273782 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -356,6 +356,16 @@ static int verify_parent_transid(struct extent_io_tree *io_tree,
>  	return ret;
>  }
>  
> +static bool btrfs_supported_super_csum(u16 csum_type)
> +{
> +	switch (csum_type) {
> +	case BTRFS_CSUM_TYPE_CRC32:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  /*
>   * Return 0 if the superblock checksum type matches the checksum value of that
>   * algorithm. Pass the raw disk superblock data.
> @@ -366,7 +376,12 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
>  	struct btrfs_super_block *disk_sb =
>  		(struct btrfs_super_block *)raw_disk_sb;
>  	u16 csum_type = btrfs_super_csum_type(disk_sb);
> -	int ret = 0;
> +
> +	if (!btrfs_supported_super_csum(csum_type)) {
> +		btrfs_err(fs_info, "unsupported checksum algorithm %u",
> +			  csum_type);
> +		return 1;
> +	}
>  
>  	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
>  		u32 crc = ~(u32)0;
> @@ -382,16 +397,10 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
>  		btrfs_csum_final(crc, result);
>  
>  		if (memcmp(raw_disk_sb, result, sizeof(result)))
> -			ret = 1;
> +			return 1;
>  	}
>  
> -	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
> -		btrfs_err(fs_info, "unsupported checksum algorithm %u",
> -				csum_type);
> -		ret = 1;
> -	}
> -
> -	return ret;
> +	return 0;
>  }
>  
>  int btrfs_verify_level_key(struct extent_buffer *eb, int level,
> @@ -2577,7 +2586,7 @@ static int btrfs_validate_write_super(struct btrfs_fs_info *fs_info,
>  	ret = validate_super(fs_info, sb, -1);
>  	if (ret < 0)
>  		goto out;
> -	if (btrfs_super_csum_type(sb) != BTRFS_CSUM_TYPE_CRC32) {
> +	if (!btrfs_supported_super_csum(sb)) {

Type mismatch, this needs the type now, not the superblock. And
open_ctree also needs an update.

>  		ret = -EUCLEAN;
>  		btrfs_err(fs_info, "invalid csum type, has %u want %u",
>  			  btrfs_super_csum_type(sb), BTRFS_CSUM_TYPE_CRC32);
> -- 
> 2.16.4

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 08/13] btrfs: check for supported superblock checksum type before checksum validation
  2019-05-22  8:19 ` [PATCH v3 08/13] btrfs: check for supported superblock checksum type before checksum validation Johannes Thumshirn
@ 2019-05-30 15:49   ` David Sterba
  0 siblings, 0 replies; 28+ messages in thread
From: David Sterba @ 2019-05-30 15:49 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir, Nikolay Borisov

On Wed, May 22, 2019 at 10:19:05AM +0200, Johannes Thumshirn wrote:
> Now that we have factorerd out the superblock checksum type validation, we
> can check for supported superblock checksum types before doing the actual
> validation of the superblock read from disk.
> 
> This leads the path to further simplifications of btrfs_check_super_csum()
> later on.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
> ---
> Changes to v2:
> - Print on-disk checksum type if we encounter an unsupported type (David)
> ---
>  fs/btrfs/disk-io.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 594583273782..f541d3c15d99 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2821,6 +2821,16 @@ int open_ctree(struct super_block *sb,
>  		goto fail_alloc;
>  	}
>  
> +	if (!btrfs_supported_super_csum((struct btrfs_super_block *)
> +					bh->b_data)) {

Previous patch changed this to u16

> +		btrfs_err(fs_info, "unsupported checksum algorithm: %d",

that's %u

> +			  btrfs_super_csum_type((struct btrfs_super_block *)
> +						bh->b_data));
> +		err = -EINVAL;
> +		brelse(bh);
> +		goto fail_alloc;
> +	}
> +
>  	/*
>  	 * We want to check superblock checksum, the type is stored inside.
>  	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
> -- 
> 2.16.4

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 06/13] btrfs: format checksums according to type for printing
  2019-05-27 16:57   ` David Sterba
@ 2019-06-03  9:33     ` Johannes Thumshirn
  0 siblings, 0 replies; 28+ messages in thread
From: Johannes Thumshirn @ 2019-06-03  9:33 UTC (permalink / raw)
  To: dsterba, David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir, Nikolay Borisov

On Mon, May 27, 2019 at 06:57:19PM +0200, David Sterba wrote:
> I think the helper is not needed at all, the format "%*phN" can be used
> for crc32c too without the intermediate buffers. For better readability,
> some macros can be added like
> 
> 	"this is wrong csum " CSUM_FORMAT " end of string",
> 	CSUM_FORMAT_VALUE(csum_size, csum_bytes)
> 
> with CSUM_FORMAT "0x%*phN" and
> CSUM_FORMAT_VALUE(size, bytes)	size, bytes
> 
> ie. just for the explict requirement of the variable length required by
> "*".

Good idea, will be updating the patch.

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 00/13] Add support for other checksums
  2019-05-27 17:19 ` [PATCH v3 00/13] Add support for other checksums David Sterba
@ 2019-06-03  9:38   ` Johannes Thumshirn
  2019-06-03 12:40     ` David Sterba
  0 siblings, 1 reply; 28+ messages in thread
From: Johannes Thumshirn @ 2019-06-03  9:38 UTC (permalink / raw)
  To: dsterba, David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir, Nikolay Borisov

On Mon, May 27, 2019 at 07:19:54PM +0200, David Sterba wrote:
> 1-5 are reviewed and ok, 6 and 13 should be reworked, 7-12 is ok. I
> can't put the branch to next yet due to the csum formatting "issues" but
> will do once you resend. Should be ok just 6 and 13 as they're
> independent.

I'd still like to hold back 13/13. SHA-256 doesn't seem to be well received by
the community as the "slow" hash and using a plain SHA-256 is not sufficient
for the dm-verity/fs-verity like approach I intend to implement in subsequent
patches.

For the record, the current idea is to use a HMAC(SHA-256) as checksum
algorithm with a key provided at mkfs and mount time.

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 00/13] Add support for other checksums
  2019-06-03  9:38   ` Johannes Thumshirn
@ 2019-06-03 12:40     ` David Sterba
  0 siblings, 0 replies; 28+ messages in thread
From: David Sterba @ 2019-06-03 12:40 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: dsterba, David Sterba, Linux BTRFS Mailinglist, Chris Mason,
	Richard Weinberger, David Gstir, Nikolay Borisov

On Mon, Jun 03, 2019 at 11:38:40AM +0200, Johannes Thumshirn wrote:
> On Mon, May 27, 2019 at 07:19:54PM +0200, David Sterba wrote:
> > 1-5 are reviewed and ok, 6 and 13 should be reworked, 7-12 is ok. I
> > can't put the branch to next yet due to the csum formatting "issues" but
> > will do once you resend. Should be ok just 6 and 13 as they're
> > independent.
> 
> I'd still like to hold back 13/13. SHA-256 doesn't seem to be well received by
> the community as the "slow" hash and using a plain SHA-256 is not sufficient
> for the dm-verity/fs-verity like approach I intend to implement in subsequent
> patches.
> 
> For the record, the current idea is to use a HMAC(SHA-256) as checksum
> algorithm with a key provided at mkfs and mount time.

The patch actually adding the new hash won't be merged to any
to-be-released branch until we have the final list, but for testing
purposes the patch will be in for-next and available via linux-next.

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2019-06-03 12:39 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-22  8:18 [PATCH v3 00/13] Add support for other checksums Johannes Thumshirn
2019-05-22  8:18 ` [PATCH v3 01/13] btrfs: use btrfs_csum_data() instead of directly calling crc32c Johannes Thumshirn
2019-05-22  8:18 ` [PATCH v3 02/13] btrfs: resurrect btrfs_crc32c() Johannes Thumshirn
2019-05-22  8:19 ` [PATCH v3 03/13] btrfs: use btrfs_crc32c{,_final}() in for free space cache Johannes Thumshirn
2019-05-22  8:19 ` [PATCH v3 04/13] btrfs: don't assume ordered sums to be 4 bytes Johannes Thumshirn
2019-05-22  8:19 ` [PATCH v3 05/13] btrfs: dont assume compressed_bio " Johannes Thumshirn
2019-05-22  8:19 ` [PATCH v3 06/13] btrfs: format checksums according to type for printing Johannes Thumshirn
2019-05-27 16:57   ` David Sterba
2019-06-03  9:33     ` Johannes Thumshirn
2019-05-22  8:19 ` [PATCH v3 07/13] btrfs: add common checksum type validation Johannes Thumshirn
2019-05-30 15:47   ` David Sterba
2019-05-22  8:19 ` [PATCH v3 08/13] btrfs: check for supported superblock checksum type before checksum validation Johannes Thumshirn
2019-05-30 15:49   ` David Sterba
2019-05-22  8:19 ` [PATCH v3 09/13] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions Johannes Thumshirn
2019-05-22  8:19 ` [PATCH v3 10/13] btrfs: add boilerplate code for directly including the crypto framework Johannes Thumshirn
2019-05-22  8:19 ` [PATCH v3 11/13] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
2019-05-22  8:33   ` Nikolay Borisov
2019-05-22  8:35     ` Johannes Thumshirn
2019-05-22  9:04     ` Johannes Thumshirn
2019-05-29 19:32   ` David Gstir
2019-05-30 10:14     ` David Sterba
2019-05-30 10:27       ` David Gstir
2019-05-22  8:19 ` [PATCH v3 12/13] btrfs: remove assumption about csum type form btrfs_print_data_csum_error() Johannes Thumshirn
2019-05-22  8:19 ` [RFC PATCH v3 13/13] btrfs: add sha256 as another checksum algorithm Johannes Thumshirn
2019-05-27 17:10   ` David Sterba
2019-05-27 17:19 ` [PATCH v3 00/13] Add support for other checksums David Sterba
2019-06-03  9:38   ` Johannes Thumshirn
2019-06-03 12:40     ` David Sterba

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.