2019-10-22 18:09:00

by Pratik Shinde

[permalink] [raw]
Subject: [PATCH-v3] erofs: code for verifying superblock checksum of an erofs image.

Patch for kernel side changes of checksum feature.Used kernel's
crc32c library for calculating the checksum.

Signed-off-by: Pratik Shinde <[email protected]>
---
fs/erofs/erofs_fs.h | 5 +++--
fs/erofs/internal.h | 3 ++-
fs/erofs/super.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index b1ee565..4d8097a 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -17,6 +17,7 @@
*/
#define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING 0x00000001
#define EROFS_ALL_FEATURE_INCOMPAT EROFS_FEATURE_INCOMPAT_LZ4_0PADDING
+#define EROFS_FEATURE_COMPAT_SB_CHKSUM 0x00000001

/* 128-byte erofs on-disk super block */
struct erofs_super_block {
@@ -37,8 +38,8 @@ struct erofs_super_block {
__u8 uuid[16]; /* 128-bit uuid for volume */
__u8 volume_name[16]; /* volume name */
__le32 feature_incompat;
-
- __u8 reserved2[44];
+ __le32 chksum_blocks; /* number of blocks used for checksum */
+ __u8 reserved2[40];
};

/*
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 544a453..cec27ca 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -86,7 +86,7 @@ struct erofs_sb_info {
u8 uuid[16]; /* 128-bit uuid for volume */
u8 volume_name[16]; /* volume name */
u32 feature_incompat;
-
+ u32 feature_compat;
unsigned int mount_opt;
};

@@ -426,6 +426,7 @@ static inline void z_erofs_exit_zip_subsystem(void) {}
#endif /* !CONFIG_EROFS_FS_ZIP */

#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
+#define EFSBADCRC EBADMSG /* Bad crc found */

#endif /* __EROFS_INTERNAL_H */

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0e36949..a2a638a 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -9,6 +9,7 @@
#include <linux/statfs.h>
#include <linux/parser.h>
#include <linux/seq_file.h>
+#include <linux/crc32c.h>
#include "xattr.h"

#define CREATE_TRACE_POINTS
@@ -46,6 +47,42 @@ void _erofs_info(struct super_block *sb, const char *function,
va_end(args);
}

+static int erofs_validate_sb_chksum(void *sbdata, struct super_block *sb)
+{
+ u32 disk_chksum, nblocks, crc = 0;
+ void *kaddr;
+ struct page *page;
+ struct erofs_super_block *dsb;
+ char *buf;
+ int i, ret = 0;
+
+ buf = kmalloc(EROFS_BLKSIZ, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+ memcpy(buf, sbdata, EROFS_BLKSIZ);
+ dsb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
+ disk_chksum = le32_to_cpu(dsb->checksum);
+ nblocks = le32_to_cpu(dsb->chksum_blocks);
+ dsb->checksum = 0;
+ crc = crc32c(0, buf, EROFS_BLKSIZ);
+ for (i = 1; i < nblocks; i++) {
+ page = erofs_get_meta_page(sb, i);
+ if (IS_ERR(page)) {
+ ret = PTR_ERR(page);
+ goto out;
+ }
+ kaddr = kmap_atomic(page);
+ crc = crc32c(crc, kaddr, EROFS_BLKSIZ);
+ kunmap_atomic(kaddr);
+ unlock_page(page);
+ put_page(page);
+ }
+ if (crc != disk_chksum)
+ ret = -EFSBADCRC;
+out: kfree(buf);
+ return ret;
+}
+
static void erofs_inode_init_once(void *ptr)
{
struct erofs_inode *vi = ptr;
@@ -121,6 +158,13 @@ static int erofs_read_superblock(struct super_block *sb)
goto out;
}

+ if (dsb->feature_compat & EROFS_FEATURE_COMPAT_SB_CHKSUM) {
+ ret = erofs_validate_sb_chksum(data, sb);
+ if (ret < 0) {
+ erofs_err(sb, "super block checksum incorrect");
+ goto out;
+ }
+ }
blkszbits = dsb->blkszbits;
/* 9(512 bytes) + LOG_SECTORS_PER_BLOCK == LOG_BLOCK_SIZE */
if (blkszbits != LOG_BLOCK_SIZE) {
--
2.9.3


2019-10-23 05:33:32

by Gao Xiang

[permalink] [raw]
Subject: [PATCH v4] erofs: support superblock checksum

From: Pratik Shinde <[email protected]>

Introduce superblock checksum feature in order to check
a number of given blocks at mounting time.

Signed-off-by: Pratik Shinde <[email protected]>
Signed-off-by: Gao Xiang <[email protected]>
---
changes from v3:
(based on Pratik's v3 patch)
- add LIBCRC32C dependency;
- use kmap() in order to avoid sleeping in atomic context;
- skip the first 1024 byte for x86 boot sector,
co-tested with userspace utils,
https://lore.kernel.org/r/[email protected]

fs/erofs/Kconfig | 1 +
fs/erofs/erofs_fs.h | 6 +++--
fs/erofs/internal.h | 2 ++
fs/erofs/super.c | 53 +++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index 9d634d3a1845..74b0aaa7114c 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -3,6 +3,7 @@
config EROFS_FS
tristate "EROFS filesystem support"
depends on BLOCK
+ select LIBCRC32C
help
EROFS (Enhanced Read-Only File System) is a lightweight
read-only file system with modern designs (eg. page-sized
diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index b1ee5654750d..461913be1d1c 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -11,6 +11,8 @@

#define EROFS_SUPER_OFFSET 1024

+#define EROFS_FEATURE_COMPAT_SB_CHKSUM 0x00000001
+
/*
* Any bits that aren't in EROFS_ALL_FEATURE_INCOMPAT should
* be incompatible with this kernel version.
@@ -37,8 +39,8 @@ struct erofs_super_block {
__u8 uuid[16]; /* 128-bit uuid for volume */
__u8 volume_name[16]; /* volume name */
__le32 feature_incompat;
-
- __u8 reserved2[44];
+ __le32 chksum_blocks; /* number of blocks used for checksum */
+ __u8 reserved2[40];
};

/*
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 544a453f3076..a3778f597bf6 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -85,6 +85,7 @@ struct erofs_sb_info {

u8 uuid[16]; /* 128-bit uuid for volume */
u8 volume_name[16]; /* volume name */
+ u32 feature_compat;
u32 feature_incompat;

unsigned int mount_opt;
@@ -426,6 +427,7 @@ static inline void z_erofs_exit_zip_subsystem(void) {}
#endif /* !CONFIG_EROFS_FS_ZIP */

#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
+#define EFSBADCRC EBADMSG /* Bad CRC detected */

#endif /* __EROFS_INTERNAL_H */

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0e369494f2f2..18d1ec18a671 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -9,6 +9,7 @@
#include <linux/statfs.h>
#include <linux/parser.h>
#include <linux/seq_file.h>
+#include <linux/crc32c.h>
#include "xattr.h"

#define CREATE_TRACE_POINTS
@@ -46,6 +47,47 @@ void _erofs_info(struct super_block *sb, const char *function,
va_end(args);
}

+static int erofs_superblock_csum_verify(struct super_block *sb, void *sbdata)
+{
+ struct erofs_super_block *dsb;
+ u32 expected_crc, nblocks, crc;
+ void *kaddr;
+ struct page *page;
+ int i;
+
+ dsb = kmemdup(sbdata + EROFS_SUPER_OFFSET,
+ EROFS_BLKSIZ - EROFS_SUPER_OFFSET, GFP_KERNEL);
+ if (!dsb)
+ return -ENOMEM;
+
+ expected_crc = le32_to_cpu(dsb->checksum);
+ nblocks = le32_to_cpu(dsb->chksum_blocks);
+ dsb->checksum = 0;
+ /* to allow for x86 boot sectors and other oddities. */
+ crc = crc32c(~0, dsb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
+ kfree(dsb);
+
+ for (i = 1; i < nblocks; i++) {
+ page = erofs_get_meta_page(sb, i);
+ if (IS_ERR(page))
+ return PTR_ERR(page);
+
+ kaddr = kmap_atomic(page);
+ crc = crc32c(crc, kaddr, EROFS_BLKSIZ);
+ kunmap_atomic(kaddr);
+
+ unlock_page(page);
+ put_page(page);
+ }
+
+ if (crc != expected_crc) {
+ erofs_err(sb, "invalid checksum 0x%08x, 0x%08x expected",
+ crc, expected_crc);
+ return -EFSBADCRC;
+ }
+ return 0;
+}
+
static void erofs_inode_init_once(void *ptr)
{
struct erofs_inode *vi = ptr;
@@ -112,7 +154,7 @@ static int erofs_read_superblock(struct super_block *sb)

sbi = EROFS_SB(sb);

- data = kmap_atomic(page);
+ data = kmap(page);
dsb = (struct erofs_super_block *)(data + EROFS_SUPER_OFFSET);

ret = -EINVAL;
@@ -121,6 +163,13 @@ static int erofs_read_superblock(struct super_block *sb)
goto out;
}

+ sbi->feature_compat = le32_to_cpu(dsb->feature_compat);
+ if (sbi->feature_compat & EROFS_FEATURE_COMPAT_SB_CHKSUM) {
+ ret = erofs_superblock_csum_verify(sb, data);
+ if (ret)
+ goto out;
+ }
+
blkszbits = dsb->blkszbits;
/* 9(512 bytes) + LOG_SECTORS_PER_BLOCK == LOG_BLOCK_SIZE */
if (blkszbits != LOG_BLOCK_SIZE) {
@@ -155,7 +204,7 @@ static int erofs_read_superblock(struct super_block *sb)
}
ret = 0;
out:
- kunmap_atomic(data);
+ kunmap(data);
put_page(page);
return ret;
}
--
2.17.1

2019-10-23 08:18:34

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH v4] erofs: support superblock checksum

Hi, Xiang, Pratik,

On 2019/10/23 12:05, Gao Xiang wrote:
> From: Pratik Shinde <[email protected]>
>
> Introduce superblock checksum feature in order to check
> a number of given blocks at mounting time.
>
> Signed-off-by: Pratik Shinde <[email protected]>
> Signed-off-by: Gao Xiang <[email protected]>
> ---
> changes from v3:
> (based on Pratik's v3 patch)
> - add LIBCRC32C dependency;
> - use kmap() in order to avoid sleeping in atomic context;
> - skip the first 1024 byte for x86 boot sector,
> co-tested with userspace utils,
> https://lore.kernel.org/r/[email protected]
>
> fs/erofs/Kconfig | 1 +
> fs/erofs/erofs_fs.h | 6 +++--
> fs/erofs/internal.h | 2 ++
> fs/erofs/super.c | 53 +++++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
> index 9d634d3a1845..74b0aaa7114c 100644
> --- a/fs/erofs/Kconfig
> +++ b/fs/erofs/Kconfig
> @@ -3,6 +3,7 @@
> config EROFS_FS
> tristate "EROFS filesystem support"
> depends on BLOCK
> + select LIBCRC32C
> help
> EROFS (Enhanced Read-Only File System) is a lightweight
> read-only file system with modern designs (eg. page-sized
> diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
> index b1ee5654750d..461913be1d1c 100644
> --- a/fs/erofs/erofs_fs.h
> +++ b/fs/erofs/erofs_fs.h
> @@ -11,6 +11,8 @@
>
> #define EROFS_SUPER_OFFSET 1024
>
> +#define EROFS_FEATURE_COMPAT_SB_CHKSUM 0x00000001
> +
> /*
> * Any bits that aren't in EROFS_ALL_FEATURE_INCOMPAT should
> * be incompatible with this kernel version.
> @@ -37,8 +39,8 @@ struct erofs_super_block {
> __u8 uuid[16]; /* 128-bit uuid for volume */
> __u8 volume_name[16]; /* volume name */
> __le32 feature_incompat;
> -
> - __u8 reserved2[44];
> + __le32 chksum_blocks; /* number of blocks used for checksum */
> + __u8 reserved2[40];
> };
>
> /*
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 544a453f3076..a3778f597bf6 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -85,6 +85,7 @@ struct erofs_sb_info {
>
> u8 uuid[16]; /* 128-bit uuid for volume */
> u8 volume_name[16]; /* volume name */
> + u32 feature_compat;
> u32 feature_incompat;
>
> unsigned int mount_opt;
> @@ -426,6 +427,7 @@ static inline void z_erofs_exit_zip_subsystem(void) {}
> #endif /* !CONFIG_EROFS_FS_ZIP */
>
> #define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
> +#define EFSBADCRC EBADMSG /* Bad CRC detected */
>
> #endif /* __EROFS_INTERNAL_H */
>
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 0e369494f2f2..18d1ec18a671 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -9,6 +9,7 @@
> #include <linux/statfs.h>
> #include <linux/parser.h>
> #include <linux/seq_file.h>
> +#include <linux/crc32c.h>
> #include "xattr.h"
>
> #define CREATE_TRACE_POINTS
> @@ -46,6 +47,47 @@ void _erofs_info(struct super_block *sb, const char *function,
> va_end(args);
> }
>
> +static int erofs_superblock_csum_verify(struct super_block *sb, void *sbdata)
> +{
> + struct erofs_super_block *dsb;
> + u32 expected_crc, nblocks, crc;
> + void *kaddr;
> + struct page *page;
> + int i;
> +
> + dsb = kmemdup(sbdata + EROFS_SUPER_OFFSET,
> + EROFS_BLKSIZ - EROFS_SUPER_OFFSET, GFP_KERNEL);
> + if (!dsb)
> + return -ENOMEM;
> +
> + expected_crc = le32_to_cpu(dsb->checksum);
> + nblocks = le32_to_cpu(dsb->chksum_blocks);

Now, we try to use nblocks's value before checking its validation, I guess fuzz
test can easily make the value extreme larger, result in checking latter blocks
unnecessarily.

IMO, we'd better
1. check validation of superblock to make sure all fields in sb are valid
2. use .nblocks to count and check payload blocks following sb

Thanks,

> + dsb->checksum = 0;
> + /* to allow for x86 boot sectors and other oddities. */
> + crc = crc32c(~0, dsb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
> + kfree(dsb);
> +
> + for (i = 1; i < nblocks; i++) {
> + page = erofs_get_meta_page(sb, i);
> + if (IS_ERR(page))
> + return PTR_ERR(page);
> +
> + kaddr = kmap_atomic(page);
> + crc = crc32c(crc, kaddr, EROFS_BLKSIZ);
> + kunmap_atomic(kaddr);
> +
> + unlock_page(page);
> + put_page(page);
> + }
> +
> + if (crc != expected_crc) {
> + erofs_err(sb, "invalid checksum 0x%08x, 0x%08x expected",
> + crc, expected_crc);
> + return -EFSBADCRC;
> + }
> + return 0;
> +}
> +
> static void erofs_inode_init_once(void *ptr)
> {
> struct erofs_inode *vi = ptr;
> @@ -112,7 +154,7 @@ static int erofs_read_superblock(struct super_block *sb)
>
> sbi = EROFS_SB(sb);
>
> - data = kmap_atomic(page);
> + data = kmap(page);
> dsb = (struct erofs_super_block *)(data + EROFS_SUPER_OFFSET);
>
> ret = -EINVAL;
> @@ -121,6 +163,13 @@ static int erofs_read_superblock(struct super_block *sb)
> goto out;
> }
>
> + sbi->feature_compat = le32_to_cpu(dsb->feature_compat);
> + if (sbi->feature_compat & EROFS_FEATURE_COMPAT_SB_CHKSUM) {
> + ret = erofs_superblock_csum_verify(sb, data);
> + if (ret)
> + goto out;
> + }
> +
> blkszbits = dsb->blkszbits;
> /* 9(512 bytes) + LOG_SECTORS_PER_BLOCK == LOG_BLOCK_SIZE */
> if (blkszbits != LOG_BLOCK_SIZE) {
> @@ -155,7 +204,7 @@ static int erofs_read_superblock(struct super_block *sb)
> }
> ret = 0;
> out:
> - kunmap_atomic(data);
> + kunmap(data);
> put_page(page);
> return ret;
> }
>

2019-10-23 08:43:58

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH v4] erofs: support superblock checksum

Hi Chao,

On Wed, Oct 23, 2019 at 04:15:29PM +0800, Chao Yu wrote:
> Hi, Xiang, Pratik,
>
> On 2019/10/23 12:05, Gao Xiang wrote:

<snip>

> > }
> >
> > +static int erofs_superblock_csum_verify(struct super_block *sb, void *sbdata)
> > +{
> > + struct erofs_super_block *dsb;
> > + u32 expected_crc, nblocks, crc;
> > + void *kaddr;
> > + struct page *page;
> > + int i;
> > +
> > + dsb = kmemdup(sbdata + EROFS_SUPER_OFFSET,
> > + EROFS_BLKSIZ - EROFS_SUPER_OFFSET, GFP_KERNEL);
> > + if (!dsb)
> > + return -ENOMEM;
> > +
> > + expected_crc = le32_to_cpu(dsb->checksum);
> > + nblocks = le32_to_cpu(dsb->chksum_blocks);
>
> Now, we try to use nblocks's value before checking its validation, I guess fuzz
> test can easily make the value extreme larger, result in checking latter blocks
> unnecessarily.
>
> IMO, we'd better
> 1. check validation of superblock to make sure all fields in sb are valid
> 2. use .nblocks to count and check payload blocks following sb

That is quite a good point. :-)

My first thought is to check the following payloads of sb (e.g, some per-fs
metadata should be checked at mount time together. or for small images, check
the whole image at the mount time) as well since if we introduce a new feature
to some kernel version, forward compatibility needs to be considered. So it's
better to make proper scalability, for this case, we have some choices:
1) limit `chksum_blocks' upbound at runtime (e.g. refuse >= 65536 blocks,
totally 256M.)
2) just get rid of the whole `chksum_blocks' mess and checksum the first 4k
at all, don't consider any latter scalability.

Some perferred idea about this? I plan to release erofs-utils v1.0 tomorrow
and hold up this feature for the next erofs-utils release, but I think we can
get it ready for v5.5 since it is not quite complex feature...

Thanks,
Gao Xiang

2019-10-28 20:10:25

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH v4] erofs: support superblock checksum

On 2019/10/23 16:45, Gao Xiang wrote:
> Hi Chao,
>
> On Wed, Oct 23, 2019 at 04:15:29PM +0800, Chao Yu wrote:
>> Hi, Xiang, Pratik,
>>
>> On 2019/10/23 12:05, Gao Xiang wrote:
>
> <snip>
>
>>> }
>>>
>>> +static int erofs_superblock_csum_verify(struct super_block *sb, void *sbdata)
>>> +{
>>> + struct erofs_super_block *dsb;
>>> + u32 expected_crc, nblocks, crc;
>>> + void *kaddr;
>>> + struct page *page;
>>> + int i;
>>> +
>>> + dsb = kmemdup(sbdata + EROFS_SUPER_OFFSET,
>>> + EROFS_BLKSIZ - EROFS_SUPER_OFFSET, GFP_KERNEL);
>>> + if (!dsb)
>>> + return -ENOMEM;
>>> +
>>> + expected_crc = le32_to_cpu(dsb->checksum);
>>> + nblocks = le32_to_cpu(dsb->chksum_blocks);
>>
>> Now, we try to use nblocks's value before checking its validation, I guess fuzz
>> test can easily make the value extreme larger, result in checking latter blocks
>> unnecessarily.
>>
>> IMO, we'd better
>> 1. check validation of superblock to make sure all fields in sb are valid
>> 2. use .nblocks to count and check payload blocks following sb
>
> That is quite a good point. :-)
>
> My first thought is to check the following payloads of sb (e.g, some per-fs
> metadata should be checked at mount time together. or for small images, check
> the whole image at the mount time) as well since if we introduce a new feature
> to some kernel version, forward compatibility needs to be considered. So it's
> better to make proper scalability, for this case, we have some choices:
> 1) limit `chksum_blocks' upbound at runtime (e.g. refuse >= 65536 blocks,
> totally 256M.)
> 2) just get rid of the whole `chksum_blocks' mess and checksum the first 4k
> at all, don't consider any latter scalability.

Xiang, sorry for later reply...

I prefer method 2), let's enable chksum feature only on superblock first,
chksum_blocks feature can be added later.

Thanks,

>
> Some perferred idea about this? I plan to release erofs-utils v1.0 tomorrow
> and hold up this feature for the next erofs-utils release, but I think we can
> get it ready for v5.5 since it is not quite complex feature...
>
> Thanks,
> Gao Xiang
>
> .
>

2019-10-28 20:43:45

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH v4] erofs: support superblock checksum

Hi Chao,

On Mon, Oct 28, 2019 at 08:36:00PM +0800, Chao Yu wrote:
> On 2019/10/23 16:45, Gao Xiang wrote:

<snip>

> > That is quite a good point. :-)
> >
> > My first thought is to check the following payloads of sb (e.g, some per-fs
> > metadata should be checked at mount time together. or for small images, check
> > the whole image at the mount time) as well since if we introduce a new feature
> > to some kernel version, forward compatibility needs to be considered. So it's
> > better to make proper scalability, for this case, we have some choices:
> > 1) limit `chksum_blocks' upbound at runtime (e.g. refuse >= 65536 blocks,
> > totally 256M.)
> > 2) just get rid of the whole `chksum_blocks' mess and checksum the first 4k
> > at all, don't consider any latter scalability.
>
> Xiang, sorry for later reply...
>
> I prefer method 2), let's enable chksum feature only on superblock first,
> chksum_blocks feature can be added later.

Okay, got it. I will resend patch soon.

Thanks,
Gao Xiang

>
> Thanks,
>
> >
> > Some perferred idea about this? I plan to release erofs-utils v1.0 tomorrow
> > and hold up this feature for the next erofs-utils release, but I think we can
> > get it ready for v5.5 since it is not quite complex feature...
> >
> > Thanks,
> > Gao Xiang
> >
> > .
> >

2019-10-29 02:47:01

by Gao Xiang

[permalink] [raw]
Subject: [PATCH v5] erofs: support superblock checksum

From: Pratik Shinde <[email protected]>

Introduce superblock checksum feature in order to verify
a number of given blocks at mounting time.

Signed-off-by: Pratik Shinde <[email protected]>
Signed-off-by: Gao Xiang <[email protected]>
---
changes since v4:
- Get rid of `chksum_blocks' pointed out by Chao;
https://lore.kernel.org/r/[email protected]
- Co-tested with mkfs patch:
https://lore.kernel.org/r/[email protected]

fs/erofs/Kconfig | 1 +
fs/erofs/erofs_fs.h | 3 ++-
fs/erofs/internal.h | 2 ++
fs/erofs/super.c | 36 ++++++++++++++++++++++++++++++++++--
4 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index 9d634d3a1845..74b0aaa7114c 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -3,6 +3,7 @@
config EROFS_FS
tristate "EROFS filesystem support"
depends on BLOCK
+ select LIBCRC32C
help
EROFS (Enhanced Read-Only File System) is a lightweight
read-only file system with modern designs (eg. page-sized
diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index b1ee5654750d..385fa49c7749 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -11,6 +11,8 @@

#define EROFS_SUPER_OFFSET 1024

+#define EROFS_FEATURE_COMPAT_SB_CHKSUM 0x00000001
+
/*
* Any bits that aren't in EROFS_ALL_FEATURE_INCOMPAT should
* be incompatible with this kernel version.
@@ -37,7 +39,6 @@ struct erofs_super_block {
__u8 uuid[16]; /* 128-bit uuid for volume */
__u8 volume_name[16]; /* volume name */
__le32 feature_incompat;
-
__u8 reserved2[44];
};

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 544a453f3076..a3778f597bf6 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -85,6 +85,7 @@ struct erofs_sb_info {

u8 uuid[16]; /* 128-bit uuid for volume */
u8 volume_name[16]; /* volume name */
+ u32 feature_compat;
u32 feature_incompat;

unsigned int mount_opt;
@@ -426,6 +427,7 @@ static inline void z_erofs_exit_zip_subsystem(void) {}
#endif /* !CONFIG_EROFS_FS_ZIP */

#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
+#define EFSBADCRC EBADMSG /* Bad CRC detected */

#endif /* __EROFS_INTERNAL_H */

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0e369494f2f2..2fcf44b656dd 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -9,6 +9,7 @@
#include <linux/statfs.h>
#include <linux/parser.h>
#include <linux/seq_file.h>
+#include <linux/crc32c.h>
#include "xattr.h"

#define CREATE_TRACE_POINTS
@@ -46,6 +47,30 @@ void _erofs_info(struct super_block *sb, const char *function,
va_end(args);
}

+static int erofs_superblock_csum_verify(struct super_block *sb, void *sbdata)
+{
+ struct erofs_super_block *dsb;
+ u32 expected_crc, crc;
+
+ dsb = kmemdup(sbdata + EROFS_SUPER_OFFSET,
+ EROFS_BLKSIZ - EROFS_SUPER_OFFSET, GFP_KERNEL);
+ if (!dsb)
+ return -ENOMEM;
+
+ expected_crc = le32_to_cpu(dsb->checksum);
+ dsb->checksum = 0;
+ /* to allow for x86 boot sectors and other oddities. */
+ crc = crc32c(~0, dsb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
+ kfree(dsb);
+
+ if (crc != expected_crc) {
+ erofs_err(sb, "invalid checksum 0x%08x, 0x%08x expected",
+ crc, expected_crc);
+ return -EFSBADCRC;
+ }
+ return 0;
+}
+
static void erofs_inode_init_once(void *ptr)
{
struct erofs_inode *vi = ptr;
@@ -112,7 +137,7 @@ static int erofs_read_superblock(struct super_block *sb)

sbi = EROFS_SB(sb);

- data = kmap_atomic(page);
+ data = kmap(page);
dsb = (struct erofs_super_block *)(data + EROFS_SUPER_OFFSET);

ret = -EINVAL;
@@ -121,6 +146,13 @@ static int erofs_read_superblock(struct super_block *sb)
goto out;
}

+ sbi->feature_compat = le32_to_cpu(dsb->feature_compat);
+ if (sbi->feature_compat & EROFS_FEATURE_COMPAT_SB_CHKSUM) {
+ ret = erofs_superblock_csum_verify(sb, data);
+ if (ret)
+ goto out;
+ }
+
blkszbits = dsb->blkszbits;
/* 9(512 bytes) + LOG_SECTORS_PER_BLOCK == LOG_BLOCK_SIZE */
if (blkszbits != LOG_BLOCK_SIZE) {
@@ -155,7 +187,7 @@ static int erofs_read_superblock(struct super_block *sb)
}
ret = 0;
out:
- kunmap_atomic(data);
+ kunmap(data);
put_page(page);
return ret;
}
--
2.17.1

2019-10-30 02:35:55

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH v5] erofs: support superblock checksum

On 2019/10/28 22:32, Gao Xiang wrote:
> From: Pratik Shinde <[email protected]>
>
> Introduce superblock checksum feature in order to verify
> a number of given blocks at mounting time.
>
> Signed-off-by: Pratik Shinde <[email protected]>
> Signed-off-by: Gao Xiang <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,

2019-10-30 02:56:48

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH v5] erofs: support superblock checksum

On Wed, Oct 30, 2019 at 10:33:47AM +0800, Chao Yu wrote:
> On 2019/10/28 22:32, Gao Xiang wrote:
> > From: Pratik Shinde <[email protected]>
> >
> > Introduce superblock checksum feature in order to verify
> > a number of given blocks at mounting time.
> >
> > Signed-off-by: Pratik Shinde <[email protected]>
> > Signed-off-by: Gao Xiang <[email protected]>
>
> Reviewed-by: Chao Yu <[email protected]>

Thanks for reviewing, I just noticed the commit message isn't
fixed as well, let me resend v6...

Thanks,
Gao Xiang

>
> Thanks,

2019-10-30 05:09:33

by Gao Xiang

[permalink] [raw]
Subject: [PATCH v6] erofs: support superblock checksum

From: Pratik Shinde <[email protected]>

Introduce superblock checksum feature in order to
check at mounting time.

Note that the first 1024 bytes are ignore for x86
boot sectors and other oddities.

Signed-off-by: Pratik Shinde <[email protected]>
Reviewed-by: Chao Yu <[email protected]>
Signed-off-by: Gao Xiang <[email protected]>
---
changes since v5:
- update commit message which was missed in v5.

fs/erofs/Kconfig | 1 +
fs/erofs/erofs_fs.h | 3 ++-
fs/erofs/internal.h | 2 ++
fs/erofs/super.c | 36 ++++++++++++++++++++++++++++++++++--
4 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index 9d634d3a1845..74b0aaa7114c 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -3,6 +3,7 @@
config EROFS_FS
tristate "EROFS filesystem support"
depends on BLOCK
+ select LIBCRC32C
help
EROFS (Enhanced Read-Only File System) is a lightweight
read-only file system with modern designs (eg. page-sized
diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index b1ee5654750d..385fa49c7749 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -11,6 +11,8 @@

#define EROFS_SUPER_OFFSET 1024

+#define EROFS_FEATURE_COMPAT_SB_CHKSUM 0x00000001
+
/*
* Any bits that aren't in EROFS_ALL_FEATURE_INCOMPAT should
* be incompatible with this kernel version.
@@ -37,7 +39,6 @@ struct erofs_super_block {
__u8 uuid[16]; /* 128-bit uuid for volume */
__u8 volume_name[16]; /* volume name */
__le32 feature_incompat;
-
__u8 reserved2[44];
};

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 544a453f3076..a3778f597bf6 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -85,6 +85,7 @@ struct erofs_sb_info {

u8 uuid[16]; /* 128-bit uuid for volume */
u8 volume_name[16]; /* volume name */
+ u32 feature_compat;
u32 feature_incompat;

unsigned int mount_opt;
@@ -426,6 +427,7 @@ static inline void z_erofs_exit_zip_subsystem(void) {}
#endif /* !CONFIG_EROFS_FS_ZIP */

#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
+#define EFSBADCRC EBADMSG /* Bad CRC detected */

#endif /* __EROFS_INTERNAL_H */

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0e369494f2f2..2fcf44b656dd 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -9,6 +9,7 @@
#include <linux/statfs.h>
#include <linux/parser.h>
#include <linux/seq_file.h>
+#include <linux/crc32c.h>
#include "xattr.h"

#define CREATE_TRACE_POINTS
@@ -46,6 +47,30 @@ void _erofs_info(struct super_block *sb, const char *function,
va_end(args);
}

+static int erofs_superblock_csum_verify(struct super_block *sb, void *sbdata)
+{
+ struct erofs_super_block *dsb;
+ u32 expected_crc, crc;
+
+ dsb = kmemdup(sbdata + EROFS_SUPER_OFFSET,
+ EROFS_BLKSIZ - EROFS_SUPER_OFFSET, GFP_KERNEL);
+ if (!dsb)
+ return -ENOMEM;
+
+ expected_crc = le32_to_cpu(dsb->checksum);
+ dsb->checksum = 0;
+ /* to allow for x86 boot sectors and other oddities. */
+ crc = crc32c(~0, dsb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
+ kfree(dsb);
+
+ if (crc != expected_crc) {
+ erofs_err(sb, "invalid checksum 0x%08x, 0x%08x expected",
+ crc, expected_crc);
+ return -EFSBADCRC;
+ }
+ return 0;
+}
+
static void erofs_inode_init_once(void *ptr)
{
struct erofs_inode *vi = ptr;
@@ -112,7 +137,7 @@ static int erofs_read_superblock(struct super_block *sb)

sbi = EROFS_SB(sb);

- data = kmap_atomic(page);
+ data = kmap(page);
dsb = (struct erofs_super_block *)(data + EROFS_SUPER_OFFSET);

ret = -EINVAL;
@@ -121,6 +146,13 @@ static int erofs_read_superblock(struct super_block *sb)
goto out;
}

+ sbi->feature_compat = le32_to_cpu(dsb->feature_compat);
+ if (sbi->feature_compat & EROFS_FEATURE_COMPAT_SB_CHKSUM) {
+ ret = erofs_superblock_csum_verify(sb, data);
+ if (ret)
+ goto out;
+ }
+
blkszbits = dsb->blkszbits;
/* 9(512 bytes) + LOG_SECTORS_PER_BLOCK == LOG_BLOCK_SIZE */
if (blkszbits != LOG_BLOCK_SIZE) {
@@ -155,7 +187,7 @@ static int erofs_read_superblock(struct super_block *sb)
}
ret = 0;
out:
- kunmap_atomic(data);
+ kunmap(data);
put_page(page);
return ret;
}
--
2.17.1

2019-11-04 02:48:46

by Gao Xiang

[permalink] [raw]
Subject: [PATCH v7] erofs: support superblock checksum

From: Pratik Shinde <[email protected]>

Introduce superblock checksum feature in order to
check at mounting time.

Note that the first 1024 bytes are ignore for x86
boot sectors and other oddities.

Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Pratik Shinde <[email protected]>
Reviewed-by: Chao Yu <[email protected]>
Cc: Dan Carpenter <[email protected]>
Signed-off-by: Gao Xiang <[email protected]>
---
changes since v6:
- fix kunmap(data) to kunmap(page) by mistake
reported by Dan Carpenter;

fs/erofs/Kconfig | 1 +
fs/erofs/erofs_fs.h | 3 ++-
fs/erofs/internal.h | 1 +
fs/erofs/super.c | 36 ++++++++++++++++++++++++++++++++++--
4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index 9d634d3a1845..74b0aaa7114c 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -3,6 +3,7 @@
config EROFS_FS
tristate "EROFS filesystem support"
depends on BLOCK
+ select LIBCRC32C
help
EROFS (Enhanced Read-Only File System) is a lightweight
read-only file system with modern designs (eg. page-sized
diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index b1ee5654750d..385fa49c7749 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -11,6 +11,8 @@

#define EROFS_SUPER_OFFSET 1024

+#define EROFS_FEATURE_COMPAT_SB_CHKSUM 0x00000001
+
/*
* Any bits that aren't in EROFS_ALL_FEATURE_INCOMPAT should
* be incompatible with this kernel version.
@@ -37,7 +39,6 @@ struct erofs_super_block {
__u8 uuid[16]; /* 128-bit uuid for volume */
__u8 volume_name[16]; /* volume name */
__le32 feature_incompat;
-
__u8 reserved2[44];
};

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 544a453f3076..96d97eab88e3 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -85,6 +85,7 @@ struct erofs_sb_info {

u8 uuid[16]; /* 128-bit uuid for volume */
u8 volume_name[16]; /* volume name */
+ u32 feature_compat;
u32 feature_incompat;

unsigned int mount_opt;
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0e369494f2f2..849c0bdf49d9 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -9,6 +9,7 @@
#include <linux/statfs.h>
#include <linux/parser.h>
#include <linux/seq_file.h>
+#include <linux/crc32c.h>
#include "xattr.h"

#define CREATE_TRACE_POINTS
@@ -46,6 +47,30 @@ void _erofs_info(struct super_block *sb, const char *function,
va_end(args);
}

+static int erofs_superblock_csum_verify(struct super_block *sb, void *sbdata)
+{
+ struct erofs_super_block *dsb;
+ u32 expected_crc, crc;
+
+ dsb = kmemdup(sbdata + EROFS_SUPER_OFFSET,
+ EROFS_BLKSIZ - EROFS_SUPER_OFFSET, GFP_KERNEL);
+ if (!dsb)
+ return -ENOMEM;
+
+ expected_crc = le32_to_cpu(dsb->checksum);
+ dsb->checksum = 0;
+ /* to allow for x86 boot sectors and other oddities. */
+ crc = crc32c(~0, dsb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
+ kfree(dsb);
+
+ if (crc != expected_crc) {
+ erofs_err(sb, "invalid checksum 0x%08x, 0x%08x expected",
+ crc, expected_crc);
+ return -EBADMSG;
+ }
+ return 0;
+}
+
static void erofs_inode_init_once(void *ptr)
{
struct erofs_inode *vi = ptr;
@@ -112,7 +137,7 @@ static int erofs_read_superblock(struct super_block *sb)

sbi = EROFS_SB(sb);

- data = kmap_atomic(page);
+ data = kmap(page);
dsb = (struct erofs_super_block *)(data + EROFS_SUPER_OFFSET);

ret = -EINVAL;
@@ -121,6 +146,13 @@ static int erofs_read_superblock(struct super_block *sb)
goto out;
}

+ sbi->feature_compat = le32_to_cpu(dsb->feature_compat);
+ if (sbi->feature_compat & EROFS_FEATURE_COMPAT_SB_CHKSUM) {
+ ret = erofs_superblock_csum_verify(sb, data);
+ if (ret)
+ goto out;
+ }
+
blkszbits = dsb->blkszbits;
/* 9(512 bytes) + LOG_SECTORS_PER_BLOCK == LOG_BLOCK_SIZE */
if (blkszbits != LOG_BLOCK_SIZE) {
@@ -155,7 +187,7 @@ static int erofs_read_superblock(struct super_block *sb)
}
ret = 0;
out:
- kunmap_atomic(data);
+ kunmap(page);
put_page(page);
return ret;
}
--
2.17.1