Message ID | 20230717112703.60130-1-jefflexu@linux.alibaba.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c923:0:b0:3e4:2afc:c1 with SMTP id j3csp1052663vqt; Mon, 17 Jul 2023 04:40:28 -0700 (PDT) X-Google-Smtp-Source: APBJJlFFrOGjBMKsW2A3sbB6sc9DFGWxTF8U9F3u0IIVxYTQV7vt1JDIx5Ln8LFe5D/pCBVVDJzR X-Received: by 2002:a05:6a20:42a9:b0:133:6219:15e2 with SMTP id o41-20020a056a2042a900b00133621915e2mr15316401pzj.21.1689594027858; Mon, 17 Jul 2023 04:40:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689594027; cv=none; d=google.com; s=arc-20160816; b=gT+IjI1hNd+hhyky/DTk7Dm+8LjK4AruJx2cVJOcAkg79fNl87n9M5Ar3s/IVanKn5 IlkeyUM/ge1qoYvUqyMe4vUQigUrubgL1il+ChrzTlh1l9E1/IbY7Xy8rNq1wTCHX4Pf Esvh5W2Rl39blkDlDoODqeGg/vnCVkNMMB5c/VgfAhfiagLjd0xSAUFXQ7qsI/gVbZhd 8ySTV3y+kLp1hkwROePbFJRqa1b5aAtXKZPGkg+ZsZVSpxwaCnig8OEUoHTxVoXseICW vM/a4p+myCA8pPM595K7YsCNTVbMq0B7LThVdX/AIh7NSXqWMc5povs7QngJLjPYfHQp Sr2A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=nyeBNbX2azWt1kI2Z9Fk+sv0QWQ4EEaGIjSHh7ZVxbA=; fh=EIWZp1cj2wwl8nxfYaWZLk2CKEAo1oYa6SeG5AIPFkU=; b=jwN+ZnCGMv+qVFaLyzCyI3p3rWYhI4W2S0YCzRgCrToAqyGh5Zjys0+JHLyp7AexSV +Ufu4mN6KmPFKoZn5AvtFdcdaRdVmRErEWyPpl099MN0m7y7DcT3vg45UFJoT9yO7gaA Q5vnibUDNrTdfSMoADqGur7YM4ojF9z3WGmCL5drbRoDrU+Hwza5NTEWJwZg/7gR0whB uwlY87s6L0OB5XuflTkDiDRVw4x9m1tBnR44FG0PNj5d118hvuIJ60xhcY+qWqVJAZSQ c0BHJUMwzkdIQ5bqoAw8K8Auin8CKgpryplu9ZBGRw+MOv2HGo/uImXygjlo4IJWVwIC MScQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id 24-20020a631258000000b005579a12f405si11102237pgs.200.2023.07.17.04.40.14; Mon, 17 Jul 2023 04:40:27 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229450AbjGQL1K (ORCPT <rfc822;hadasmailinglist@gmail.com> + 99 others); Mon, 17 Jul 2023 07:27:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42172 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229541AbjGQL1J (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 17 Jul 2023 07:27:09 -0400 Received: from out30-111.freemail.mail.aliyun.com (out30-111.freemail.mail.aliyun.com [115.124.30.111]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C2F6BE4C for <linux-kernel@vger.kernel.org>; Mon, 17 Jul 2023 04:27:07 -0700 (PDT) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R961e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018045192;MF=jefflexu@linux.alibaba.com;NM=1;PH=DS;RN=5;SR=0;TI=SMTPD_---0Vnc8ouu_1689593223; Received: from localhost(mailfrom:jefflexu@linux.alibaba.com fp:SMTPD_---0Vnc8ouu_1689593223) by smtp.aliyun-inc.com; Mon, 17 Jul 2023 19:27:04 +0800 From: Jingbo Xu <jefflexu@linux.alibaba.com> To: hsiangkao@linux.alibaba.com, chao@kernel.org, huyue2@coolpad.com, linux-erofs@lists.ozlabs.org Cc: linux-kernel@vger.kernel.org Subject: [PATCH v2] erofs: deprecate superblock checksum feature Date: Mon, 17 Jul 2023 19:27:03 +0800 Message-Id: <20230717112703.60130-1-jefflexu@linux.alibaba.com> X-Mailer: git-send-email 2.19.1.6.gb485710b MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,UNPARSEABLE_RELAY,USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1771667747409825423 X-GMAIL-MSGID: 1771667747409825423 |
Series |
[v2] erofs: deprecate superblock checksum feature
|
|
Commit Message
Jingbo Xu
July 17, 2023, 11:27 a.m. UTC
Later we're going to try the self-contained image verification.
The current superblock checksum feature has quite limited
functionality, instead, merkle trees can provide better protection
for image integrity.
xxhash is also used in the following xattr name filter feature. It is
redundant for one filesystem to rely on two hashing algorithms at the
same time.
Since the superblock checksum is a compatible feature, just deprecate
it now.
Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
---
changes since v1:
- improve commit message (Gao Xiang)
v1: https://lore.kernel.org/all/20230714033832.111740-1-jefflexu@linux.alibaba.com/
---
fs/erofs/Kconfig | 1 -
fs/erofs/super.c | 44 +++++---------------------------------------
2 files changed, 5 insertions(+), 40 deletions(-)
Comments
On 2023/7/17 19:27, Jingbo Xu wrote: > Later we're going to try the self-contained image verification. > The current superblock checksum feature has quite limited > functionality, instead, merkle trees can provide better protection > for image integrity. > > xxhash is also used in the following xattr name filter feature. It is > redundant for one filesystem to rely on two hashing algorithms at the > same time. > > Since the superblock checksum is a compatible feature, just deprecate > it now. > > Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com> I tend to agree on this since it slightly impacts mount time too: Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com> Thanks, Gao Xiang
On 2023/7/17 19:27, Jingbo Xu wrote: > Later we're going to try the self-contained image verification. > The current superblock checksum feature has quite limited > functionality, instead, merkle trees can provide better protection > for image integrity. > > xxhash is also used in the following xattr name filter feature. It is > redundant for one filesystem to rely on two hashing algorithms at the > same time. > > Since the superblock checksum is a compatible feature, just deprecate > it now. > > Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com> Reviewed-by: Chao Yu <chao@kernel.org> Thanks,
On 2023-07-17 19:27:03+0800, Jingbo Xu wrote: > Later we're going to try the self-contained image verification. > The current superblock checksum feature has quite limited > functionality, instead, merkle trees can provide better protection > for image integrity. The crc32c checksum is also used by libblkid to gain more confidence in its filesystem detection. I guess a merkle tree would be much harder to implement. This is for example used by the mount(8) cli program to allow mounting of devices without explicitly needing to specify a filesystem. Note: libblkid tests for EROFS_FEATURE_SB_CSUM so at least it won't break when the checksum is removed. > xxhash is also used in the following xattr name filter feature. It is > redundant for one filesystem to rely on two hashing algorithms at the > same time. > > Since the superblock checksum is a compatible feature, just deprecate > it now. > > Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com> > --- > changes since v1: > - improve commit message (Gao Xiang) > > v1: https://lore.kernel.org/all/20230714033832.111740-1-jefflexu@linux.alibaba.com/ > --- > fs/erofs/Kconfig | 1 - > fs/erofs/super.c | 44 +++++--------------------------------------- > 2 files changed, 5 insertions(+), 40 deletions(-) > [..]
Hi Thomas, On 2023/7/30 21:31, Thomas Weißschuh wrote: > On 2023-07-17 19:27:03+0800, Jingbo Xu wrote: >> Later we're going to try the self-contained image verification. >> The current superblock checksum feature has quite limited >> functionality, instead, merkle trees can provide better protection >> for image integrity. > > The crc32c checksum is also used by libblkid to gain more confidence > in its filesystem detection. > I guess a merkle tree would be much harder to implement. > > This is for example used by the mount(8) cli program to allow mounting > of devices without explicitly needing to specify a filesystem. > > Note: libblkid tests for EROFS_FEATURE_SB_CSUM so at least it won't > break when the checksum is removed. I'm not sure if we could switch EROFS_FEATURE_SB_CSUM to a simpler checksum instead (e.g. just sum each byte up if both EROFS_FEATURE_SB_CSUM and COMPAT_XATTR_FILTER bits are set, or ignore checksums completely at least in the kernel) if the better filesystem detection by using sb chksum is needed (not sure if other filesystems have sb chksum or just do magic comparsion)? The main problem here is after xattr name filter feature is added (xxhash is generally faster than crc32c), there could be two hard-depended hashing algorithms, this increases more dependency especially for embededed devices. Thanks, Gao Xiang
Hi Gao! On 2023-07-30 22:01:11+0800, Gao Xiang wrote: > On 2023/7/30 21:31, Thomas Weißschuh wrote: > > On 2023-07-17 19:27:03+0800, Jingbo Xu wrote: > > > Later we're going to try the self-contained image verification. > > > The current superblock checksum feature has quite limited > > > functionality, instead, merkle trees can provide better protection > > > for image integrity. > > > > The crc32c checksum is also used by libblkid to gain more confidence > > in its filesystem detection. > > I guess a merkle tree would be much harder to implement. > > > > This is for example used by the mount(8) cli program to allow mounting > > of devices without explicitly needing to specify a filesystem. > > > > Note: libblkid tests for EROFS_FEATURE_SB_CSUM so at least it won't > > break when the checksum is removed. > I'm not sure if we could switch EROFS_FEATURE_SB_CSUM to a simpler > checksum instead (e.g. just sum each byte up if both > EROFS_FEATURE_SB_CSUM and COMPAT_XATTR_FILTER bits are set, or > ignore checksums completely at least in the kernel) if the better > filesystem detection by using sb chksum is needed (not sure if other > filesystems have sb chksum or just do magic comparsion)? Overloading EROFS_FEATURE_SB_CSUM in combination with COMPAT_XATTR_FILTER would break all existing deployments of libblkid, so it's not an option. All other serious and halfway modern filesystems do have superblock checksums which are also checked by libblkid. > The main problem here is after xattr name filter feature is added > (xxhash is generally faster than crc32c), there could be two > hard-depended hashing algorithms, this increases more dependency > especially for embededed devices. From libblkid side nothing really speaks against a simpler checksum. XOR is easy to implement and xxhash is already part of libblkid for other filesystems. The drawbacks are: * It would need a completely new feature bit in erofs. * Old versions of libblkid could not validate checksums on newer filesystems.
On 2023/7/30 22:28, Thomas Weißschuh wrote: > Hi Gao! > > On 2023-07-30 22:01:11+0800, Gao Xiang wrote: >> On 2023/7/30 21:31, Thomas Weißschuh wrote: >>> On 2023-07-17 19:27:03+0800, Jingbo Xu wrote: >>>> Later we're going to try the self-contained image verification. >>>> The current superblock checksum feature has quite limited >>>> functionality, instead, merkle trees can provide better protection >>>> for image integrity. >>> >>> The crc32c checksum is also used by libblkid to gain more confidence >>> in its filesystem detection. >>> I guess a merkle tree would be much harder to implement. >>> >>> This is for example used by the mount(8) cli program to allow mounting >>> of devices without explicitly needing to specify a filesystem. >>> >>> Note: libblkid tests for EROFS_FEATURE_SB_CSUM so at least it won't >>> break when the checksum is removed. > >> I'm not sure if we could switch EROFS_FEATURE_SB_CSUM to a simpler >> checksum instead (e.g. just sum each byte up if both >> EROFS_FEATURE_SB_CSUM and COMPAT_XATTR_FILTER bits are set, or >> ignore checksums completely at least in the kernel) if the better >> filesystem detection by using sb chksum is needed (not sure if other >> filesystems have sb chksum or just do magic comparsion)? > > Overloading EROFS_FEATURE_SB_CSUM in combination with > COMPAT_XATTR_FILTER would break all existing deployments of libblkid, so > it's not an option. I think for libblkid, you could still use: EROFS_FEATURE_SB_CSUM is not set, don't check anything; EROFS_FEATURE_SB_CSUM only is set, check with crc32c; EROFS_FEATURE_SB_CSUM | COMPAT_XATTR_FILTER (or some other bit) is set, check with a simpler hash? > > All other serious and halfway modern filesystems do have superblock > checksums which are also checked by libblkid. > >> The main problem here is after xattr name filter feature is added >> (xxhash is generally faster than crc32c), there could be two >> hard-depended hashing algorithms, this increases more dependency >> especially for embededed devices. > > From libblkid side nothing really speaks against a simpler checksum. > XOR is easy to implement and xxhash is already part of libblkid for > other filesystems. > > The drawbacks are: > * It would need a completely new feature bit in erofs. > * Old versions of libblkid could not validate checksums on newer > filesystems. just checking magic for Old versions of libblkid will cause false positive in which extent? Thanks, Gao Xiang
On 2023-07-30 22:37:19+0800, Gao Xiang wrote: > On 2023/7/30 22:28, Thomas Weißschuh wrote: > > On 2023-07-30 22:01:11+0800, Gao Xiang wrote: > > > On 2023/7/30 21:31, Thomas Weißschuh wrote: > > > > On 2023-07-17 19:27:03+0800, Jingbo Xu wrote: > > > > > Later we're going to try the self-contained image verification. > > > > > The current superblock checksum feature has quite limited > > > > > functionality, instead, merkle trees can provide better protection > > > > > for image integrity. > > > > > > > > The crc32c checksum is also used by libblkid to gain more confidence > > > > in its filesystem detection. > > > > I guess a merkle tree would be much harder to implement. > > > > > > > > This is for example used by the mount(8) cli program to allow mounting > > > > of devices without explicitly needing to specify a filesystem. > > > > > > > > Note: libblkid tests for EROFS_FEATURE_SB_CSUM so at least it won't > > > > break when the checksum is removed. > > > > > I'm not sure if we could switch EROFS_FEATURE_SB_CSUM to a simpler > > > checksum instead (e.g. just sum each byte up if both > > > EROFS_FEATURE_SB_CSUM and COMPAT_XATTR_FILTER bits are set, or > > > ignore checksums completely at least in the kernel) if the better > > > filesystem detection by using sb chksum is needed (not sure if other > > > filesystems have sb chksum or just do magic comparsion)? > > > > Overloading EROFS_FEATURE_SB_CSUM in combination with > > COMPAT_XATTR_FILTER would break all existing deployments of libblkid, so > > it's not an option. > > I think for libblkid, you could still use: > EROFS_FEATURE_SB_CSUM is not set, don't check anything; > EROFS_FEATURE_SB_CSUM only is set, check with crc32c; > EROFS_FEATURE_SB_CSUM | COMPAT_XATTR_FILTER (or some other bit) is > set, check with a simpler hash? We could change this in newer versions of libblkid. But we can't change the older versions that are already deployed today. And the current code does if (EROFS_FEATURE_SB_CSUM) validate_crc32c(); So to stay compatible we need to keep the relationship of EROFS_FEATURE_SB_CSUM -> crc32c. > > All other serious and halfway modern filesystems do have superblock > > checksums which are also checked by libblkid. > > > > > The main problem here is after xattr name filter feature is added > > > (xxhash is generally faster than crc32c), there could be two > > > hard-depended hashing algorithms, this increases more dependency > > > especially for embededed devices. > > > > From libblkid side nothing really speaks against a simpler checksum. > > XOR is easy to implement and xxhash is already part of libblkid for > > other filesystems. > > > > The drawbacks are: > > * It would need a completely new feature bit in erofs. > > * Old versions of libblkid could not validate checksums on newer > > filesystems. > > just checking magic for Old versions of libblkid will cause false > positive in which extent? Hard to tell for sure. But it would not surprise me if it would indeed affect users as experience has shown. Imagine for example erofs filesystems that have then been overwritten with another filesystem but still have a valid erofs magic. With the checksum validation the new filesystem is detected correctly, without it will find the old erofs. Sometimes the files inside some filesystem look like the superblock of another filesystem and break the detection. etc. Having some sort of checksum makes this much easier to handle.
On 2023/7/30 22:49, Thomas Weißschuh wrote: > On 2023-07-30 22:37:19+0800, Gao Xiang wrote: >> On 2023/7/30 22:28, Thomas Weißschuh wrote: >>> On 2023-07-30 22:01:11+0800, Gao Xiang wrote: >>>> On 2023/7/30 21:31, Thomas Weißschuh wrote: >>>>> On 2023-07-17 19:27:03+0800, Jingbo Xu wrote: >>>>>> Later we're going to try the self-contained image verification. >>>>>> The current superblock checksum feature has quite limited >>>>>> functionality, instead, merkle trees can provide better protection >>>>>> for image integrity. >>>>> >>>>> The crc32c checksum is also used by libblkid to gain more confidence >>>>> in its filesystem detection. >>>>> I guess a merkle tree would be much harder to implement. >>>>> >>>>> This is for example used by the mount(8) cli program to allow mounting >>>>> of devices without explicitly needing to specify a filesystem. >>>>> >>>>> Note: libblkid tests for EROFS_FEATURE_SB_CSUM so at least it won't >>>>> break when the checksum is removed. >>> >>>> I'm not sure if we could switch EROFS_FEATURE_SB_CSUM to a simpler >>>> checksum instead (e.g. just sum each byte up if both >>>> EROFS_FEATURE_SB_CSUM and COMPAT_XATTR_FILTER bits are set, or >>>> ignore checksums completely at least in the kernel) if the better >>>> filesystem detection by using sb chksum is needed (not sure if other >>>> filesystems have sb chksum or just do magic comparsion)? >>> >>> Overloading EROFS_FEATURE_SB_CSUM in combination with >>> COMPAT_XATTR_FILTER would break all existing deployments of libblkid, so >>> it's not an option. >> >> I think for libblkid, you could still use: >> EROFS_FEATURE_SB_CSUM is not set, don't check anything; >> EROFS_FEATURE_SB_CSUM only is set, check with crc32c; >> EROFS_FEATURE_SB_CSUM | COMPAT_XATTR_FILTER (or some other bit) is >> set, check with a simpler hash? > > We could change this in newer versions of libblkid. > But we can't change the older versions that are already deployed today. > > And the current code does > > if (EROFS_FEATURE_SB_CSUM) > validate_crc32c(); > > So to stay compatible we need to keep the relationship of > EROFS_FEATURE_SB_CSUM -> crc32c. Yes, you are right, thanks for reminder. We really need a new bit for this. > >>> All other serious and halfway modern filesystems do have superblock >>> checksums which are also checked by libblkid. >>> >>>> The main problem here is after xattr name filter feature is added >>>> (xxhash is generally faster than crc32c), there could be two >>>> hard-depended hashing algorithms, this increases more dependency >>>> especially for embededed devices. >>> >>> From libblkid side nothing really speaks against a simpler checksum. >>> XOR is easy to implement and xxhash is already part of libblkid for >>> other filesystems. >>> >>> The drawbacks are: >>> * It would need a completely new feature bit in erofs. >>> * Old versions of libblkid could not validate checksums on newer >>> filesystems. >> >> just checking magic for Old versions of libblkid will cause false >> positive in which extent? > > Hard to tell for sure. But it would not surprise me if it would indeed > affect users as experience has shown. > > Imagine for example erofs filesystems that have then been overwritten > with another filesystem but still have a valid erofs magic. > With the checksum validation the new filesystem is detected correctly, > without it will find the old erofs. > > Sometimes the files inside some filesystem look like the superblock of > another filesystem and break the detection. > > etc. > > Having some sort of checksum makes this much easier to handle. Yes, but just checking magic for old versions of libblkid for new generated images only. I'm not sure about this all (I just suggest that we might need a simpler algorithm like XOR instead for sb_chksum otherwise it seems too heavy), let me just drop this commit from -next for further discussion. Thanks, Gao Xiang
Hi, Thomas, On 7/30/23 10:28 PM, Thomas Weißschuh wrote: > Hi Gao! > > On 2023-07-30 22:01:11+0800, Gao Xiang wrote: >> On 2023/7/30 21:31, Thomas Weißschuh wrote: >>> On 2023-07-17 19:27:03+0800, Jingbo Xu wrote: >>>> Later we're going to try the self-contained image verification. >>>> The current superblock checksum feature has quite limited >>>> functionality, instead, merkle trees can provide better protection >>>> for image integrity. >>> >>> The crc32c checksum is also used by libblkid to gain more confidence >>> in its filesystem detection. >>> I guess a merkle tree would be much harder to implement. >>> >>> This is for example used by the mount(8) cli program to allow mounting >>> of devices without explicitly needing to specify a filesystem. >>> >>> Note: libblkid tests for EROFS_FEATURE_SB_CSUM so at least it won't >>> break when the checksum is removed. > >> I'm not sure if we could switch EROFS_FEATURE_SB_CSUM to a simpler >> checksum instead (e.g. just sum each byte up if both >> EROFS_FEATURE_SB_CSUM and COMPAT_XATTR_FILTER bits are set, or >> ignore checksums completely at least in the kernel) if the better >> filesystem detection by using sb chksum is needed (not sure if other >> filesystems have sb chksum or just do magic comparsion)? > > Overloading EROFS_FEATURE_SB_CSUM in combination with > COMPAT_XATTR_FILTER would break all existing deployments of libblkid, so > it's not an option. > > All other serious and halfway modern filesystems do have superblock > checksums which are also checked by libblkid. > >> The main problem here is after xattr name filter feature is added >> (xxhash is generally faster than crc32c), there could be two >> hard-depended hashing algorithms, this increases more dependency >> especially for embededed devices. > > From libblkid side nothing really speaks against a simpler checksum. > XOR is easy to implement and xxhash is already part of libblkid for > other filesystems. > > The drawbacks are: > * It would need a completely new feature bit in erofs. > * Old versions of libblkid could not validate checksums on newer > filesystems. Thanks for pointing this out. we indeed need further discussion for a better solution. As mentioned previously, we don't want two hashing algorithms dependency for erofs. The best idea as far as I can come up with is that, introduce a new feature bit indicating XOR hashing algorithm for the sb checksum, while the original EROFS_FEATURE_SB_CSUM is not set. As for the old version libblkid, only fs magic is available for the fs type detection, not perfect but in a best-effort way.
diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig index f259d92c9720..ebcb1f6a426a 100644 --- a/fs/erofs/Kconfig +++ b/fs/erofs/Kconfig @@ -4,7 +4,6 @@ config EROFS_FS tristate "EROFS filesystem support" depends on BLOCK select FS_IOMAP - select LIBCRC32C help EROFS (Enhanced Read-Only File System) is a lightweight read-only file system with modern designs (e.g. no buffer heads, inline diff --git a/fs/erofs/super.c b/fs/erofs/super.c index 9d6a3c6158bd..bb6a966ac4d4 100644 --- a/fs/erofs/super.c +++ b/fs/erofs/super.c @@ -8,7 +8,6 @@ #include <linux/statfs.h> #include <linux/parser.h> #include <linux/seq_file.h> -#include <linux/crc32c.h> #include <linux/fs_context.h> #include <linux/fs_parser.h> #include <linux/dax.h> @@ -51,33 +50,6 @@ 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) -{ - size_t len = 1 << EROFS_SB(sb)->blkszbits; - struct erofs_super_block *dsb; - u32 expected_crc, crc; - - if (len > EROFS_SUPER_OFFSET) - len -= EROFS_SUPER_OFFSET; - - dsb = kmemdup(sbdata + EROFS_SUPER_OFFSET, len, 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, len); - 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; @@ -113,15 +85,16 @@ static void erofs_free_inode(struct inode *inode) static bool check_layout_compatibility(struct super_block *sb, struct erofs_super_block *dsb) { - const unsigned int feature = le32_to_cpu(dsb->feature_incompat); + struct erofs_sb_info *sbi = EROFS_SB(sb); - EROFS_SB(sb)->feature_incompat = feature; + sbi->feature_compat = le32_to_cpu(dsb->feature_compat); + sbi->feature_incompat = le32_to_cpu(dsb->feature_incompat); /* check if current kernel meets all mandatory requirements */ - if (feature & (~EROFS_ALL_FEATURE_INCOMPAT)) { + if (sbi->feature_incompat & (~EROFS_ALL_FEATURE_INCOMPAT)) { erofs_err(sb, "unidentified incompatible feature %x, please upgrade kernel version", - feature & ~EROFS_ALL_FEATURE_INCOMPAT); + sbi->feature_incompat & ~EROFS_ALL_FEATURE_INCOMPAT); return false; } return true; @@ -365,13 +338,6 @@ static int erofs_read_superblock(struct super_block *sb) goto out; } - sbi->feature_compat = le32_to_cpu(dsb->feature_compat); - if (erofs_sb_has_sb_chksum(sbi)) { - ret = erofs_superblock_csum_verify(sb, data); - if (ret) - goto out; - } - ret = -EINVAL; if (!check_layout_compatibility(sb, dsb)) goto out;