EROFS filesystem has been merged into linux-staging for a year.
EROFS is designed to be a better solution of saving extra storage
space with guaranteed end-to-end performance for read-only files
with the help of reduced metadata, fixed-sized output compression
and decompression inplace technologies.
In the past year, EROFS was greatly improved by many people as
a staging driver, self-tested, betaed by a large number of our
internal users, successfully applied to almost all in-service
HUAWEI smartphones as the part of EMUI 9.1 and proven to be stable
enough to be moved out of staging.
EROFS is a self-contained filesystem driver. Although there are
still some TODOs to be more generic, we have a dedicated team
actively keeping on working on EROFS in order to make it better
with the evolution of Linux kernel as the other in-kernel filesystems.
As Pavel suggested, it's better to do as one commit since git
can do moves and all histories will be saved in this way.
Let's promote it from staging and enhance it more actively as
a "real" part of kernel for more wider scenarios!
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Alexander Viro <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Theodore Ts'o <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: David Sterba <[email protected]>
Cc: Amir Goldstein <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Darrick J . Wong <[email protected]>
Cc: Dave Chinner <[email protected]>
Cc: Jaegeuk Kim <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Chao Yu <[email protected]>
Cc: Miao Xie <[email protected]>
Cc: Li Guifu <[email protected]>
Cc: Fang Wei <[email protected]>
Signed-off-by: Gao Xiang <[email protected]>
---
Hi,
This is a formal moving patch based on a previous patch for staging tree
https://lore.kernel.org/r/[email protected]/
The previous related topic is
https://lore.kernel.org/r/[email protected]/
changelog since RFC:
- Update commit message for better conclusion;
- Remove the file names from the comments at the top of the files suggested by Stephen;
- Update MAINTAINERS reminded by a kind person.
Thank you very much,
Gao Xiang
.../filesystems/erofs.txt | 4 --
MAINTAINERS | 14 +++---
drivers/staging/Kconfig | 2 -
drivers/staging/Makefile | 1 -
drivers/staging/erofs/TODO | 46 -------------------
fs/Kconfig | 1 +
fs/Makefile | 1 +
{drivers/staging => fs}/erofs/Kconfig | 0
{drivers/staging => fs}/erofs/Makefile | 4 +-
{drivers/staging => fs}/erofs/compress.h | 2 -
{drivers/staging => fs}/erofs/data.c | 2 -
{drivers/staging => fs}/erofs/decompressor.c | 2 -
{drivers/staging => fs}/erofs/dir.c | 2 -
{drivers/staging => fs}/erofs/erofs_fs.h | 3 --
{drivers/staging => fs}/erofs/inode.c | 2 -
{drivers/staging => fs}/erofs/internal.h | 3 +-
{drivers/staging => fs}/erofs/namei.c | 2 -
{drivers/staging => fs}/erofs/super.c | 2 -
{drivers/staging => fs}/erofs/tagptr.h | 0
{drivers/staging => fs}/erofs/utils.c | 2 -
{drivers/staging => fs}/erofs/xattr.c | 2 -
{drivers/staging => fs}/erofs/xattr.h | 2 -
{drivers/staging => fs}/erofs/zdata.c | 2 -
{drivers/staging => fs}/erofs/zdata.h | 2 -
{drivers/staging => fs}/erofs/zmap.c | 2 -
{drivers/staging => fs}/erofs/zpvec.h | 2 -
.../include => include}/trace/events/erofs.h | 0
include/uapi/linux/magic.h | 1 +
28 files changed, 12 insertions(+), 96 deletions(-)
rename {drivers/staging/erofs/Documentation => Documentation}/filesystems/erofs.txt (98%)
delete mode 100644 drivers/staging/erofs/TODO
rename {drivers/staging => fs}/erofs/Kconfig (100%)
rename {drivers/staging => fs}/erofs/Makefile (68%)
rename {drivers/staging => fs}/erofs/compress.h (96%)
rename {drivers/staging => fs}/erofs/data.c (99%)
rename {drivers/staging => fs}/erofs/decompressor.c (99%)
rename {drivers/staging => fs}/erofs/dir.c (98%)
rename {drivers/staging => fs}/erofs/erofs_fs.h (99%)
rename {drivers/staging => fs}/erofs/inode.c (99%)
rename {drivers/staging => fs}/erofs/internal.h (99%)
rename {drivers/staging => fs}/erofs/namei.c (99%)
rename {drivers/staging => fs}/erofs/super.c (99%)
rename {drivers/staging => fs}/erofs/tagptr.h (100%)
rename {drivers/staging => fs}/erofs/utils.c (99%)
rename {drivers/staging => fs}/erofs/xattr.c (99%)
rename {drivers/staging => fs}/erofs/xattr.h (98%)
rename {drivers/staging => fs}/erofs/zdata.c (99%)
rename {drivers/staging => fs}/erofs/zdata.h (99%)
rename {drivers/staging => fs}/erofs/zmap.c (99%)
rename {drivers/staging => fs}/erofs/zpvec.h (98%)
rename {drivers/staging/erofs/include => include}/trace/events/erofs.h (100%)
diff --git a/drivers/staging/erofs/Documentation/filesystems/erofs.txt b/Documentation/filesystems/erofs.txt
similarity index 98%
rename from drivers/staging/erofs/Documentation/filesystems/erofs.txt
rename to Documentation/filesystems/erofs.txt
index 0eab600ca7ca..38aa9126ec98 100644
--- a/drivers/staging/erofs/Documentation/filesystems/erofs.txt
+++ b/Documentation/filesystems/erofs.txt
@@ -49,10 +49,6 @@ Bugs and patches are welcome, please kindly help us and send to the following
linux-erofs mailing list:
>> linux-erofs mailing list <[email protected]>
-Note that EROFS is still working in progress as a Linux staging driver,
-Cc the staging mailing list as well is highly recommended:
->> Linux Driver Project Developer List <[email protected]>
-
Mount options
=============
diff --git a/MAINTAINERS b/MAINTAINERS
index 429d61119980..5a8dbcafed00 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6046,6 +6046,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/kristoffer/linux-hpc.git
F: drivers/video/fbdev/s1d13xxxfb.c
F: include/video/s1d13xxxfb.h
+EROFS FILE SYSTEM
+M: Gao Xiang <[email protected]>
+M: Chao Yu <[email protected]>
+L: [email protected]
+S: Maintained
+F: fs/erofs/
+
ERRSEQ ERROR TRACKING INFRASTRUCTURE
M: Jeff Layton <[email protected]>
S: Maintained
@@ -15215,13 +15222,6 @@ M: H Hartley Sweeten <[email protected]>
S: Odd Fixes
F: drivers/staging/comedi/
-STAGING - EROFS FILE SYSTEM
-M: Gao Xiang <[email protected]>
-M: Chao Yu <[email protected]>
-L: [email protected]
-S: Maintained
-F: drivers/staging/erofs/
-
STAGING - FIELDBUS SUBSYSTEM
M: Sven Van Asbroeck <[email protected]>
S: Maintained
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 7c96a01eef6c..d972ec8e71fb 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -112,8 +112,6 @@ source "drivers/staging/gasket/Kconfig"
source "drivers/staging/axis-fifo/Kconfig"
-source "drivers/staging/erofs/Kconfig"
-
source "drivers/staging/fieldbus/Kconfig"
source "drivers/staging/kpc2000/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index fcaac9693b83..6018b9a4a077 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -46,7 +46,6 @@ obj-$(CONFIG_DMA_RALINK) += ralink-gdma/
obj-$(CONFIG_SOC_MT7621) += mt7621-dts/
obj-$(CONFIG_STAGING_GASKET_FRAMEWORK) += gasket/
obj-$(CONFIG_XIL_AXIS_FIFO) += axis-fifo/
-obj-$(CONFIG_EROFS_FS) += erofs/
obj-$(CONFIG_FIELDBUS_DEV) += fieldbus/
obj-$(CONFIG_KPC2000) += kpc2000/
obj-$(CONFIG_ISDN_CAPI) += isdn/
diff --git a/drivers/staging/erofs/TODO b/drivers/staging/erofs/TODO
deleted file mode 100644
index a8608b2f72bd..000000000000
--- a/drivers/staging/erofs/TODO
+++ /dev/null
@@ -1,46 +0,0 @@
-
-EROFS is still working in progress, thus it is not suitable
-for all productive uses. play at your own risk :)
-
-TODO List:
- - add the missing error handling code
- (mainly existed in xattr and decompression submodules);
-
- - finalize erofs ondisk format design (which means that
- minor on-disk revisions could happen later);
-
- - documentation and detailed technical analysis;
-
- - general code review and clean up
- (including confusing variable names and code snippets);
-
- - support larger compressed clustersizes for selection
- (currently erofs only works as expected with the page-sized
- compressed cluster configuration, usually 4KB);
-
- - support more lossless data compression algorithms
- in addition to LZ4 algorithms in VLE approach;
-
- - data deduplication and other useful features.
-
-The following git tree provides the file system user-space
-tools under development (ex, formatting tool mkfs.erofs):
->> git://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git
-
-The open-source development of erofs-utils is at the early stage.
-Contact the original author Li Guifu <[email protected]> and
-the co-maintainer Fang Wei <[email protected]> for the latest news
-and more details.
-
-Code, suggestions, etc, are welcome. Please feel free to
-ask and send patches,
-
-To:
- linux-erofs mailing list <[email protected]>
- Gao Xiang <[email protected]>
- Chao Yu <[email protected]>
-
-Cc: (for linux-kernel upstream patches)
- Greg Kroah-Hartman <[email protected]>
- linux-staging mailing list <[email protected]>
-
diff --git a/fs/Kconfig b/fs/Kconfig
index bfb1c6095c7a..669d46550e6d 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -261,6 +261,7 @@ source "fs/romfs/Kconfig"
source "fs/pstore/Kconfig"
source "fs/sysv/Kconfig"
source "fs/ufs/Kconfig"
+source "fs/erofs/Kconfig"
endif # MISC_FILESYSTEMS
diff --git a/fs/Makefile b/fs/Makefile
index d60089fd689b..b2e4973a0bea 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -130,3 +130,4 @@ obj-$(CONFIG_F2FS_FS) += f2fs/
obj-$(CONFIG_CEPH_FS) += ceph/
obj-$(CONFIG_PSTORE) += pstore/
obj-$(CONFIG_EFIVAR_FS) += efivarfs/
+obj-$(CONFIG_EROFS_FS) += erofs/
diff --git a/drivers/staging/erofs/Kconfig b/fs/erofs/Kconfig
similarity index 100%
rename from drivers/staging/erofs/Kconfig
rename to fs/erofs/Kconfig
diff --git a/drivers/staging/erofs/Makefile b/fs/erofs/Makefile
similarity index 68%
rename from drivers/staging/erofs/Makefile
rename to fs/erofs/Makefile
index 5cdae21cb5af..46f2aa4ba46c 100644
--- a/drivers/staging/erofs/Makefile
+++ b/fs/erofs/Makefile
@@ -1,12 +1,10 @@
# SPDX-License-Identifier: GPL-2.0-only
-EROFS_VERSION = "1.0pre1"
+EROFS_VERSION = "1.0"
ccflags-y += -DEROFS_VERSION=\"$(EROFS_VERSION)\"
obj-$(CONFIG_EROFS_FS) += erofs.o
-# staging requirement: to be self-contained in its own directory
-ccflags-y += -I $(srctree)/$(src)/include
erofs-objs := super.o inode.o data.o namei.o dir.o utils.o
erofs-$(CONFIG_EROFS_FS_XATTR) += xattr.o
erofs-$(CONFIG_EROFS_FS_ZIP) += decompressor.o zmap.o zdata.o
diff --git a/drivers/staging/erofs/compress.h b/fs/erofs/compress.h
similarity index 96%
rename from drivers/staging/erofs/compress.h
rename to fs/erofs/compress.h
index 043013f9ef1b..07d279fd5d67 100644
--- a/drivers/staging/erofs/compress.h
+++ b/fs/erofs/compress.h
@@ -1,7 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * linux/drivers/staging/erofs/compress.h
- *
* Copyright (C) 2019 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/data.c b/fs/erofs/data.c
similarity index 99%
rename from drivers/staging/erofs/data.c
rename to fs/erofs/data.c
index 72c4b4c5296b..fda16ec8863e 100644
--- a/drivers/staging/erofs/data.c
+++ b/fs/erofs/data.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/data.c
- *
* Copyright (C) 2017-2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/decompressor.c b/fs/erofs/decompressor.c
similarity index 99%
rename from drivers/staging/erofs/decompressor.c
rename to fs/erofs/decompressor.c
index 32a811ac704a..5f4b7f302863 100644
--- a/drivers/staging/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/decompressor.c
- *
* Copyright (C) 2019 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/dir.c b/fs/erofs/dir.c
similarity index 98%
rename from drivers/staging/erofs/dir.c
rename to fs/erofs/dir.c
index 5f38382637e6..637d70108d59 100644
--- a/drivers/staging/erofs/dir.c
+++ b/fs/erofs/dir.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/dir.c
- *
* Copyright (C) 2017-2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
similarity index 99%
rename from drivers/staging/erofs/erofs_fs.h
rename to fs/erofs/erofs_fs.h
index 6db70f395937..afa7d45ca958 100644
--- a/drivers/staging/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -1,7 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only OR Apache-2.0 */
/*
- * linux/drivers/staging/erofs/erofs_fs.h
- *
* Copyright (C) 2017-2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
@@ -10,7 +8,6 @@
#define __EROFS_FS_H
/* Enhanced(Extended) ROM File System */
-#define EROFS_SUPER_MAGIC_V1 0xE0F5E1E2
#define EROFS_SUPER_OFFSET 1024
/*
diff --git a/drivers/staging/erofs/inode.c b/fs/erofs/inode.c
similarity index 99%
rename from drivers/staging/erofs/inode.c
rename to fs/erofs/inode.c
index cbc2c342a37f..80f4fe919ee7 100644
--- a/drivers/staging/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/inode.c
- *
* Copyright (C) 2017-2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/internal.h b/fs/erofs/internal.h
similarity index 99%
rename from drivers/staging/erofs/internal.h
rename to fs/erofs/internal.h
index 0e8d58546c52..620b73fcc416 100644
--- a/drivers/staging/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -1,7 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * linux/drivers/staging/erofs/internal.h
- *
* Copyright (C) 2017-2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
@@ -15,6 +13,7 @@
#include <linux/pagemap.h>
#include <linux/bio.h>
#include <linux/buffer_head.h>
+#include <linux/magic.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
#include "erofs_fs.h"
diff --git a/drivers/staging/erofs/namei.c b/fs/erofs/namei.c
similarity index 99%
rename from drivers/staging/erofs/namei.c
rename to fs/erofs/namei.c
index 8334a910acef..8832b5d95d91 100644
--- a/drivers/staging/erofs/namei.c
+++ b/fs/erofs/namei.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/namei.c
- *
* Copyright (C) 2017-2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/super.c b/fs/erofs/super.c
similarity index 99%
rename from drivers/staging/erofs/super.c
rename to fs/erofs/super.c
index f65a1ff9f42f..bd3b1ae05b21 100644
--- a/drivers/staging/erofs/super.c
+++ b/fs/erofs/super.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/super.c
- *
* Copyright (C) 2017-2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/tagptr.h b/fs/erofs/tagptr.h
similarity index 100%
rename from drivers/staging/erofs/tagptr.h
rename to fs/erofs/tagptr.h
diff --git a/drivers/staging/erofs/utils.c b/fs/erofs/utils.c
similarity index 99%
rename from drivers/staging/erofs/utils.c
rename to fs/erofs/utils.c
index 814c2ee037ae..1dd041aa0f5a 100644
--- a/drivers/staging/erofs/utils.c
+++ b/fs/erofs/utils.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/utils.c
- *
* Copyright (C) 2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/xattr.c b/fs/erofs/xattr.c
similarity index 99%
rename from drivers/staging/erofs/xattr.c
rename to fs/erofs/xattr.c
index e7e5840e3f9d..a8286998a079 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/xattr.c
- *
* Copyright (C) 2017-2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/xattr.h b/fs/erofs/xattr.h
similarity index 98%
rename from drivers/staging/erofs/xattr.h
rename to fs/erofs/xattr.h
index e20249647541..c5ca47d814dd 100644
--- a/drivers/staging/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -1,7 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * linux/drivers/staging/erofs/xattr.h
- *
* Copyright (C) 2017-2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/zdata.c b/fs/erofs/zdata.c
similarity index 99%
rename from drivers/staging/erofs/zdata.c
rename to fs/erofs/zdata.c
index 2d7aaf98f7de..48251cb2aa39 100644
--- a/drivers/staging/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/zdata.c
- *
* Copyright (C) 2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/zdata.h b/fs/erofs/zdata.h
similarity index 99%
rename from drivers/staging/erofs/zdata.h
rename to fs/erofs/zdata.h
index e11fe1959ca2..4fc547bc01f9 100644
--- a/drivers/staging/erofs/zdata.h
+++ b/fs/erofs/zdata.h
@@ -1,7 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * linux/drivers/staging/erofs/zdata.h
- *
* Copyright (C) 2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/zmap.c b/fs/erofs/zmap.c
similarity index 99%
rename from drivers/staging/erofs/zmap.c
rename to fs/erofs/zmap.c
index b61b9b5950ac..764656151662 100644
--- a/drivers/staging/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * linux/drivers/staging/erofs/zmap.c
- *
* Copyright (C) 2018-2019 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/zpvec.h b/fs/erofs/zpvec.h
similarity index 98%
rename from drivers/staging/erofs/zpvec.h
rename to fs/erofs/zpvec.h
index 9798f5627786..bd3cee16491c 100644
--- a/drivers/staging/erofs/zpvec.h
+++ b/fs/erofs/zpvec.h
@@ -1,7 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * linux/drivers/staging/erofs/zpvec.h
- *
* Copyright (C) 2018 HUAWEI, Inc.
* http://www.huawei.com/
* Created by Gao Xiang <[email protected]>
diff --git a/drivers/staging/erofs/include/trace/events/erofs.h b/include/trace/events/erofs.h
similarity index 100%
rename from drivers/staging/erofs/include/trace/events/erofs.h
rename to include/trace/events/erofs.h
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index 1274c692e59c..903cc2d2750b 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -19,6 +19,7 @@
#define SQUASHFS_MAGIC 0x73717368
#define ECRYPTFS_SUPER_MAGIC 0xf15f
#define EFS_SUPER_MAGIC 0x414A53
+#define EROFS_SUPER_MAGIC_V1 0xE0F5E1E2
#define EXT2_SUPER_MAGIC 0xEF53
#define EXT3_SUPER_MAGIC 0xEF53
#define XENFS_SUPER_MAGIC 0xabba1974
--
2.17.1
----- Ursprüngliche Mail -----
> Von: "Gao Xiang" <[email protected]>
> An: "Greg Kroah-Hartman" <[email protected]>, "Al Viro" <[email protected]>, "linux-fsdevel"
> <[email protected]>, [email protected], [email protected], "linux-kernel"
> <[email protected]>
> CC: "Andrew Morton" <[email protected]>, "Stephen Rothwell" <[email protected]>, "tytso" <[email protected]>,
> "Pavel Machek" <[email protected]>, "David Sterba" <[email protected]>, "Amir Goldstein" <[email protected]>, "Christoph
> Hellwig" <[email protected]>, "Darrick J . Wong" <[email protected]>, "Dave Chinner" <[email protected]>,
> "Jaegeuk Kim" <[email protected]>, "Jan Kara" <[email protected]>, "richard" <[email protected]>, "torvalds"
> <[email protected]>, "Chao Yu" <[email protected]>, "Miao Xie" <[email protected]>, "Li Guifu"
> <[email protected]>, "Fang Wei" <[email protected]>, "Gao Xiang" <[email protected]>
> Gesendet: Samstag, 17. August 2019 10:23:13
> Betreff: [PATCH] erofs: move erofs out of staging
> EROFS filesystem has been merged into linux-staging for a year.
>
> EROFS is designed to be a better solution of saving extra storage
> space with guaranteed end-to-end performance for read-only files
> with the help of reduced metadata, fixed-sized output compression
> and decompression inplace technologies.
How does erofs compare to squashfs?
IIUC it is designed to be faster. Do you have numbers?
Feel free to point me older mails if you already showed numbers,
I have to admit I didn't follow the development very closely.
Thanks,
//richard
Hi Richard,
On Sat, Aug 17, 2019 at 11:19:50PM +0200, Richard Weinberger wrote:
> ----- Urspr?ngliche Mail -----
> > Von: "Gao Xiang" <[email protected]>
> > An: "Greg Kroah-Hartman" <[email protected]>, "Al Viro" <[email protected]>, "linux-fsdevel"
> > <[email protected]>, [email protected], [email protected], "linux-kernel"
> > <[email protected]>
> > CC: "Andrew Morton" <[email protected]>, "Stephen Rothwell" <[email protected]>, "tytso" <[email protected]>,
> > "Pavel Machek" <[email protected]>, "David Sterba" <[email protected]>, "Amir Goldstein" <[email protected]>, "Christoph
> > Hellwig" <[email protected]>, "Darrick J . Wong" <[email protected]>, "Dave Chinner" <[email protected]>,
> > "Jaegeuk Kim" <[email protected]>, "Jan Kara" <[email protected]>, "richard" <[email protected]>, "torvalds"
> > <[email protected]>, "Chao Yu" <[email protected]>, "Miao Xie" <[email protected]>, "Li Guifu"
> > <[email protected]>, "Fang Wei" <[email protected]>, "Gao Xiang" <[email protected]>
> > Gesendet: Samstag, 17. August 2019 10:23:13
> > Betreff: [PATCH] erofs: move erofs out of staging
>
> > EROFS filesystem has been merged into linux-staging for a year.
> >
> > EROFS is designed to be a better solution of saving extra storage
> > space with guaranteed end-to-end performance for read-only files
> > with the help of reduced metadata, fixed-sized output compression
> > and decompression inplace technologies.
>
> How does erofs compare to squashfs?
> IIUC it is designed to be faster. Do you have numbers?
> Feel free to point me older mails if you already showed numbers,
> I have to admit I didn't follow the development very closely.
You can see the following related material which has microbenchmark
tested on my laptop:
https://static.sched.com/hosted_files/kccncosschn19eng/19/EROFS%20file%20system_OSS2019_Final.pdf
which was mentioned in the related topic as well:
https://lore.kernel.org/r/[email protected]/
Thanks,
Gao Xiang
>
> Thanks,
> //richard
----- Ursprüngliche Mail -----
>> How does erofs compare to squashfs?
>> IIUC it is designed to be faster. Do you have numbers?
>> Feel free to point me older mails if you already showed numbers,
>> I have to admit I didn't follow the development very closely.
>
> You can see the following related material which has microbenchmark
> tested on my laptop:
> https://static.sched.com/hosted_files/kccncosschn19eng/19/EROFS%20file%20system_OSS2019_Final.pdf
>
> which was mentioned in the related topic as well:
> https://lore.kernel.org/r/[email protected]/
Thanks!
Will read into.
While digging a little into the code I noticed that you have very few
checks of the on-disk data.
For example ->u.i_blkaddr. I gave it a try and created a
malformed filesystem where u.i_blkaddr is 0xdeadbeef, it causes the kernel
to loop forever around erofs_read_raw_page().
Thanks,
//richard
Hi Richard,
On Sun, Aug 18, 2019 at 01:25:58AM +0200, Richard Weinberger wrote:
> ----- Urspr?ngliche Mail -----
> >> How does erofs compare to squashfs?
> >> IIUC it is designed to be faster. Do you have numbers?
> >> Feel free to point me older mails if you already showed numbers,
> >> I have to admit I didn't follow the development very closely.
> >
> > You can see the following related material which has microbenchmark
> > tested on my laptop:
> > https://static.sched.com/hosted_files/kccncosschn19eng/19/EROFS%20file%20system_OSS2019_Final.pdf
> >
> > which was mentioned in the related topic as well:
> > https://lore.kernel.org/r/[email protected]/
>
> Thanks!
> Will read into.
Yes, it was mentioned in the related topic from v1 and I you can have
a try with the latest kernel and enwik9 silesia.tar testdata.
>
> While digging a little into the code I noticed that you have very few
> checks of the on-disk data.
> For example ->u.i_blkaddr. I gave it a try and created a
> malformed filesystem where u.i_blkaddr is 0xdeadbeef, it causes the kernel
> to loop forever around erofs_read_raw_page().
I don't fuzz all the on-disk fields for EROFS, I will do later..
You can see many in-kernel filesystems are still hardening the related
stuff. Anyway, I will dig into this field you mentioned recently, but
I think it can be fixed easily later.
Thanks,
Gao Xiang
>
> Thanks,
> //richard
On Sun, Aug 18, 2019 at 07:38:47AM +0800, Gao Xiang wrote:
> Hi Richard,
>
> On Sun, Aug 18, 2019 at 01:25:58AM +0200, Richard Weinberger wrote:
[]
> >
> > While digging a little into the code I noticed that you have very few
> > checks of the on-disk data.
> > For example ->u.i_blkaddr. I gave it a try and created a
> > malformed filesystem where u.i_blkaddr is 0xdeadbeef, it causes the kernel
> > to loop forever around erofs_read_raw_page().
>
> I don't fuzz all the on-disk fields for EROFS, I will do later..
> You can see many in-kernel filesystems are still hardening the related
> stuff. Anyway, I will dig into this field you mentioned recently, but
> I think it can be fixed easily later.
...I take a simple try with the following erofs-utils diff and
a directory containing enwik9 only, with the latest kernel (5.3-rc)
and command line is
mkfs/mkfs.erofs -d9 enwik9.img testdir.
diff --git a/lib/inode.c b/lib/inode.c
index 581f263..2540338 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -388,8 +388,7 @@ static bool erofs_bh_flush_write_inode(struct erofs_buffer_head *bh)
v1.i_u.compressed_blocks =
cpu_to_le32(inode->u.i_blocks);
else
- v1.i_u.raw_blkaddr =
- cpu_to_le32(inode->u.i_blkaddr);
+ v1.i_u.raw_blkaddr = 0xdeadbeef;
break;
}
I tested the corrupted image with looped device and real blockdevice
by dd, and it seems fine....
[36283.012381] erofs: initializing erofs 1.0
[36283.012510] erofs: successfully to initialize erofs
[36283.012975] erofs: read_super, device -> /dev/loop17
[36283.012976] erofs: options -> (null)
[36283.012983] erofs: root inode @ nid 36
[36283.012995] erofs: mounted on /dev/loop17 with opts: (null).
[36297.354090] attempt to access beyond end of device
[36297.354098] loop17: rw=0, want=29887428984, limit=1953128
[36297.354107] attempt to access beyond end of device
[36297.354109] loop17: rw=0, want=29887428480, limit=1953128
[36301.827234] attempt to access beyond end of device
[36301.827243] loop17: rw=0, want=29887428480, limit=1953128
[36371.426889] erofs: unmounted for /dev/loop17
[36518.156114] erofs: read_super, device -> /dev/nvme0n1p4
[36518.156115] erofs: options -> (null)
[36518.156260] erofs: root inode @ nid 36
[36518.156384] erofs: mounted on /dev/nvme0n1p4 with opts: (null).
[36522.818884] attempt to access beyond end of device
[36522.818889] nvme0n1p4: rw=0, want=29887428984, limit=62781440
[36522.818895] attempt to access beyond end of device
[36522.818896] nvme0n1p4: rw=0, want=29887428480, limit=62781440
[36524.072018] attempt to access beyond end of device
[36524.072028] nvme0n1p4: rw=0, want=29887428480, limit=62781440
Could you give me more hints how to reproduce that? and I will
dig into more maybe it needs more conditions...
Thanks,
Gao Xiang
>
> Thanks,
> Gao Xiang
>
> >
> > Thanks,
> > //richard
On Sun, Aug 18, 2019 at 08:04:11AM +0800, Gao Xiang wrote:
> On Sun, Aug 18, 2019 at 07:38:47AM +0800, Gao Xiang wrote:
> > Hi Richard,
> >
> > On Sun, Aug 18, 2019 at 01:25:58AM +0200, Richard Weinberger wrote:
>
> []
>
> > >
> > > While digging a little into the code I noticed that you have very few
> > > checks of the on-disk data.
> > > For example ->u.i_blkaddr. I gave it a try and created a
> > > malformed filesystem where u.i_blkaddr is 0xdeadbeef, it causes the kernel
> > > to loop forever around erofs_read_raw_page().
> >
> > I don't fuzz all the on-disk fields for EROFS, I will do later..
> > You can see many in-kernel filesystems are still hardening the related
> > stuff. Anyway, I will dig into this field you mentioned recently, but
> > I think it can be fixed easily later.
>
> ...I take a simple try with the following erofs-utils diff and
> a directory containing enwik9 only, with the latest kernel (5.3-rc)
> and command line is
> mkfs/mkfs.erofs -d9 enwik9.img testdir.
>
> diff --git a/lib/inode.c b/lib/inode.c
> index 581f263..2540338 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -388,8 +388,7 @@ static bool erofs_bh_flush_write_inode(struct erofs_buffer_head *bh)
> v1.i_u.compressed_blocks =
> cpu_to_le32(inode->u.i_blocks);
> else
> - v1.i_u.raw_blkaddr =
> - cpu_to_le32(inode->u.i_blkaddr);
> + v1.i_u.raw_blkaddr = 0xdeadbeef;
> break;
> }
>
> I tested the corrupted image with looped device and real blockdevice
> by dd, and it seems fine....
> [36283.012381] erofs: initializing erofs 1.0
> [36283.012510] erofs: successfully to initialize erofs
> [36283.012975] erofs: read_super, device -> /dev/loop17
> [36283.012976] erofs: options -> (null)
> [36283.012983] erofs: root inode @ nid 36
> [36283.012995] erofs: mounted on /dev/loop17 with opts: (null).
> [36297.354090] attempt to access beyond end of device
> [36297.354098] loop17: rw=0, want=29887428984, limit=1953128
> [36297.354107] attempt to access beyond end of device
> [36297.354109] loop17: rw=0, want=29887428480, limit=1953128
> [36301.827234] attempt to access beyond end of device
> [36301.827243] loop17: rw=0, want=29887428480, limit=1953128
> [36371.426889] erofs: unmounted for /dev/loop17
> [36518.156114] erofs: read_super, device -> /dev/nvme0n1p4
> [36518.156115] erofs: options -> (null)
> [36518.156260] erofs: root inode @ nid 36
> [36518.156384] erofs: mounted on /dev/nvme0n1p4 with opts: (null).
> [36522.818884] attempt to access beyond end of device
> [36522.818889] nvme0n1p4: rw=0, want=29887428984, limit=62781440
> [36522.818895] attempt to access beyond end of device
> [36522.818896] nvme0n1p4: rw=0, want=29887428480, limit=62781440
> [36524.072018] attempt to access beyond end of device
> [36524.072028] nvme0n1p4: rw=0, want=29887428480, limit=62781440
>
> Could you give me more hints how to reproduce that? and I will
> dig into more maybe it needs more conditions...
I think I found what happened here... That is not a bug due to lack of
check of on-disk ->u.i_blkaddr (seems block layer will handle access
beyond end of device) but actually a bug of erofs_readdir:
diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index fda16ec8863e..5b5f35d47370 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -329,6 +329,8 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
trace_erofs_readpage(page, true);
+ WARN_ON(1);
+
bio = erofs_read_raw_page(NULL, page->mapping,
page, &last_block, 1, false);
@@ -379,6 +381,8 @@ static int erofs_raw_access_readpages(struct file *filp,
/* the rare case (end in gaps) */
if (unlikely(bio))
__submit_bio(bio, REQ_OP_READ, 0);
+
+ WARN_ON(1);
return 0;
}
diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index 637d70108d59..ccca954438ed 100644
--- a/fs/erofs/dir.c
+++ b/fs/erofs/dir.c
@@ -80,8 +80,10 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
unsigned int nameoff, maxsize;
dentry_page = read_mapping_page(mapping, i, NULL);
- if (IS_ERR(dentry_page))
- continue;
+ if (IS_ERR(dentry_page)) {
+ err = PTR_ERR(dentry_page);
+ break;
+ }
de = (struct erofs_dirent *)kmap(dentry_page);
It's a forever loop due to error handling of the read_mapping_page above.
I will fix that in another patch and thanks for your report!
Thanks,
Gao Xiang
>
> Thanks,
> Gao Xiang
>
> >
> > Thanks,
> > Gao Xiang
> >
> > >
> > > Thanks,
> > > //richard
----- Ursprüngliche Mail -----
>> While digging a little into the code I noticed that you have very few
>> checks of the on-disk data.
>> For example ->u.i_blkaddr. I gave it a try and created a
>> malformed filesystem where u.i_blkaddr is 0xdeadbeef, it causes the kernel
>> to loop forever around erofs_read_raw_page().
>
> I don't fuzz all the on-disk fields for EROFS, I will do later..
> You can see many in-kernel filesystems are still hardening the related
> stuff. Anyway, I will dig into this field you mentioned recently, but
> I think it can be fixed easily later.
This is no excuse to redo all these bugs. :-)
I know that many in-kernel filesystems trust the disk ultimately, this is a
problem and huge attack vector.
Thanks,
//richard
On Sun, Aug 18, 2019 at 10:16:50AM +0200, Richard Weinberger wrote:
> ----- Urspr?ngliche Mail -----
> >> While digging a little into the code I noticed that you have very few
> >> checks of the on-disk data.
> >> For example ->u.i_blkaddr. I gave it a try and created a
> >> malformed filesystem where u.i_blkaddr is 0xdeadbeef, it causes the kernel
> >> to loop forever around erofs_read_raw_page().
> >
> > I don't fuzz all the on-disk fields for EROFS, I will do later..
> > You can see many in-kernel filesystems are still hardening the related
> > stuff. Anyway, I will dig into this field you mentioned recently, but
> > I think it can be fixed easily later.
>
> This is no excuse to redo all these bugs. :-)
I agree with you, but what can we do now is trying our best to fuzz
all the fields.
So, what is your opinion about EROFS?
>
> I know that many in-kernel filesystems trust the disk ultimately, this is a
> problem and huge attack vector.
I EROFS already has many error handing paths to recover corrupted images,
and your discovery is a bug out of one error handing path miswritten by me.
I cannot make a guarantee that all the new things (every new kernel version
will introduce new feature / new code) are bug-free since I am not a machine
or code parser.
My answer about this EROFS will be more stable with our development, we have
a dedicated team with paid job, and since we currently use EROFS on the top of
dm-verity for current Android, which will keep us from corrupted images.
But yes, we will focus on fuzzing all the images for generic usages,
and we will backport all these patches to old stable versions.
Thanks,
Gao Xiang
>
> Thanks,
> //richard
----- Ursprüngliche Mail -----
> I agree with you, but what can we do now is trying our best to fuzz
> all the fields.
>
> So, what is your opinion about EROFS?
All I'm saying is that you should not blindly trust the disk.
Another thing that raises my attention is in superblock_read():
memcpy(sbi->volume_name, layout->volume_name,
sizeof(layout->volume_name));
Where do you check whether ->volume_name has a NUL terminator?
Currently this field has no user, maybe will add a check upon usage.
But this kind of things makes me wonder.
Thanks,
//richard
On Sun, Aug 18, 2019 at 11:03:53AM +0200, Richard Weinberger wrote:
> ----- Urspr?ngliche Mail -----
> > I agree with you, but what can we do now is trying our best to fuzz
> > all the fields.
> >
> > So, what is your opinion about EROFS?
>
> All I'm saying is that you should not blindly trust the disk.
>
> Another thing that raises my attention is in superblock_read():
> memcpy(sbi->volume_name, layout->volume_name,
> sizeof(layout->volume_name));
>
> Where do you check whether ->volume_name has a NUL terminator?
> Currently this field has no user, maybe will add a check upon usage.
> But this kind of things makes me wonder.
You have looked at reiserfs lately, right? :)
Not to say that erofs shouldn't be worked on to fix these kinds of
issues, just that it's not an unheard of thing to trust the disk image.
Especially for the normal usage model of erofs, where the whole disk
image is verfied before it is allowed to be mounted as part of the boot
process.
thanks,
greg k-h
----- Ursprüngliche Mail -----
> You have looked at reiserfs lately, right? :)
Don't remind me of that. ;-)
> Not to say that erofs shouldn't be worked on to fix these kinds of
> issues, just that it's not an unheard of thing to trust the disk image.
> Especially for the normal usage model of erofs, where the whole disk
> image is verfied before it is allowed to be mounted as part of the boot
> process.
For normal use I see no problem at all.
I fear distros that try to mount anything you plug into your USB.
At least SUSE already blacklists erofs:
https://github.com/openSUSE/suse-module-tools/blob/master/suse-module-tools.spec#L24
Thanks,
//richard
On Sun, Aug 18, 2019 at 11:03:53AM +0200, Richard Weinberger wrote:
> ----- Urspr??ngliche Mail -----
> > I agree with you, but what can we do now is trying our best to fuzz
> > all the fields.
> >
> > So, what is your opinion about EROFS?
>
> All I'm saying is that you should not blindly trust the disk.
I completely agree with you, and I'm teaching EROFS to
make the little naughty boy more strong... (we already
have many error handling code, but I think I will teach him
more, yes.)
>
> Another thing that raises my attention is in superblock_read():
> memcpy(sbi->volume_name, layout->volume_name,
> sizeof(layout->volume_name));
>
> Where do you check whether ->volume_name has a NUL terminator?
> Currently this field has no user, maybe will add a check upon usage.
> But this kind of things makes me wonder.
Yes, I think this is a good point. :)
Since volume_name is not used currently, I will fix it when
it use late.
I will make a note here (or more straightforward, I will fix it
to avoid potential bug now.)
Thanks,
Gao Xiang
>
> Thanks,
> //richard
Hi Richard,
On 2019-8-18 17:21, Richard Weinberger wrote:
> For normal use I see no problem at all.
> I fear distros that try to mount anything you plug into your USB.
>
> At least SUSE already blacklists erofs:
> https://github.com/openSUSE/suse-module-tools/blob/master/suse-module-tools.spec#L24
Thanks for letting us know current status of erofs in SUSE distro.
Currently erofs cares more about the requirement of Android, in there, we are
safe on fuzzed image case as dm-verity can keep all partition data being
verified before mount.
For other scenarios, like distro, erofs should improve itself step by step as
many mainline filesystems in many aspects to fit the there-in requirement. :)
Thanks,
>
> Thanks,
> //richard
>
On Sun, Aug 18, 2019 at 11:21:13AM +0200, Richard Weinberger wrote:
> > Not to say that erofs shouldn't be worked on to fix these kinds of
> > issues, just that it's not an unheard of thing to trust the disk image.
> > Especially for the normal usage model of erofs, where the whole disk
> > image is verfied before it is allowed to be mounted as part of the boot
> > process.
>
> For normal use I see no problem at all.
> I fear distros that try to mount anything you plug into your USB.
>
> At least SUSE already blacklists erofs:
> https://github.com/openSUSE/suse-module-tools/blob/master/suse-module-tools.spec#L24
Note that of the mainstream file systems, ext4 and xfs don't guarantee
that it's safe to blindly take maliciously provided file systems, such
as those provided by a untrusted container, and mount it on a file
system without problems. As I recall, one of the XFS developers
described file system fuzzing reports as a denial of service attack on
the developers. And on the ext4 side, while I try to address them, it
is by no means considered a high priority work item, and I don't
consider fixes of fuzzing reports to be worthy of coordinated
disclosure or a high priority bug to fix. (I have closed more bugs in
this area than XFS has, although that may be that ext4 started with
more file system format bugs than XFS; however given the time to first
bug in 2017 using American Fuzzy Lop[1] being 5 seconds for btrfs, 10
seconds for f2fs, 25 seconds for reiserfs, 4 minutes for ntfs, 1h45m
for xfs, and 2h for ext4, that seems unlikely.)
[1] https://events.static.linuxfound.org/sites/events/files/slides/AFL%20filesystem%20fuzzing%2C%20Vault%202016_0.pdf
So holding a file system like EROFS to a higher standard than say,
ext4, xfs, or btrfs hardly seems fair. There seems to be a very
unfortunate tendancy for us to hold new file systems to impossibly
high standards, when in fact, adding a file system to Linux should
not, in my opinion, be a remarkable event. We have a huge number of
them already, many of which are barely maintained and probably have
far worse issues than file systems trying to get into the clubhouse.
If a file system is requesting core changes to the VM or block layers,
sure, we should care about the interfaces. But this nitpicking about
whether or not a file system can be trusted in what I consider to be
COMPLETELY INSANE CONTAINER USE CASES is really not fair.
Cheers,
- Ted
On Sun, Aug 18, 2019 at 11:11:54AM -0400, Theodore Y. Ts'o wrote:
> Note that of the mainstream file systems, ext4 and xfs don't guarantee
> that it's safe to blindly take maliciously provided file systems, such
> as those provided by a untrusted container, and mount it on a file
> system without problems. As I recall, one of the XFS developers
> described file system fuzzing reports as a denial of service attack on
> the developers.
I think this greatly misrepresents the general attitute of the XFS
developers. We take sanity checks for the modern v5 on disk format
very series, and put a lot of effort into handling corrupted file
systems as good as possible, although there are of course no guaranteeѕ.
The quote that you've taken out of context is for the legacy v4 format
that has no checksums and other integrity features.
Hi Ted,
On Sun, Aug 18, 2019 at 11:11:54AM -0400, Theodore Y. Ts'o wrote:
> On Sun, Aug 18, 2019 at 11:21:13AM +0200, Richard Weinberger wrote:
> > > Not to say that erofs shouldn't be worked on to fix these kinds of
> > > issues, just that it's not an unheard of thing to trust the disk image.
> > > Especially for the normal usage model of erofs, where the whole disk
> > > image is verfied before it is allowed to be mounted as part of the boot
> > > process.
> >
> > For normal use I see no problem at all.
> > I fear distros that try to mount anything you plug into your USB.
> >
> > At least SUSE already blacklists erofs:
> > https://github.com/openSUSE/suse-module-tools/blob/master/suse-module-tools.spec#L24
>
> Note that of the mainstream file systems, ext4 and xfs don't guarantee
> that it's safe to blindly take maliciously provided file systems, such
> as those provided by a untrusted container, and mount it on a file
> system without problems. As I recall, one of the XFS developers
> described file system fuzzing reports as a denial of service attack on
> the developers. And on the ext4 side, while I try to address them, it
> is by no means considered a high priority work item, and I don't
> consider fixes of fuzzing reports to be worthy of coordinated
> disclosure or a high priority bug to fix. (I have closed more bugs in
> this area than XFS has, although that may be that ext4 started with
> more file system format bugs than XFS; however given the time to first
> bug in 2017 using American Fuzzy Lop[1] being 5 seconds for btrfs, 10
> seconds for f2fs, 25 seconds for reiserfs, 4 minutes for ntfs, 1h45m
> for xfs, and 2h for ext4, that seems unlikely.)
>
> [1] https://events.static.linuxfound.org/sites/events/files/slides/AFL%20filesystem%20fuzzing%2C%20Vault%202016_0.pdf
>
> So holding a file system like EROFS to a higher standard than say,
> ext4, xfs, or btrfs hardly seems fair. There seems to be a very
> unfortunate tendancy for us to hold new file systems to impossibly
> high standards, when in fact, adding a file system to Linux should
> not, in my opinion, be a remarkable event. We have a huge number of
> them already, many of which are barely maintained and probably have
> far worse issues than file systems trying to get into the clubhouse.
>
> If a file system is requesting core changes to the VM or block layers,
> sure, we should care about the interfaces. But this nitpicking about
> whether or not a file system can be trusted in what I consider to be
> COMPLETELY INSANE CONTAINER USE CASES is really not fair.
Thanks for your kind reply and understanding...
Yes, EROFS is now still like a little baby, and what I can do is to write
more code to make it grown up... but I personally cannot write bug-free
code all the time (because sometimes I could be in a low mood...)
In the past year, we already adds many error handling path for corrupted
images and handles all BUG_ONs in a more proper way... we are doing our best...
Our team will continue focusing on all bug reports from external /
internal sources and fix them all in time. and for more wider scenerios,
I'd like to build an autofuzzer tools based on erofs-utils to make EROFS
more strong as one of the next step.
Thanks,
Gao Xiang
>
> Cheers,
>
> - Ted
On Sun, Aug 18, 2019 at 08:58:12AM -0700, Christoph Hellwig wrote:
> On Sun, Aug 18, 2019 at 11:11:54AM -0400, Theodore Y. Ts'o wrote:
> > Note that of the mainstream file systems, ext4 and xfs don't guarantee
> > that it's safe to blindly take maliciously provided file systems, such
> > as those provided by a untrusted container, and mount it on a file
> > system without problems. As I recall, one of the XFS developers
> > described file system fuzzing reports as a denial of service attack on
> > the developers.
>
> I think this greatly misrepresents the general attitute of the XFS
> developers. We take sanity checks for the modern v5 on disk format
> very series, and put a lot of effort into handling corrupted file
> systems as good as possible, although there are of course no guaranteeѕ.
>
> The quote that you've taken out of context is for the legacy v4 format
> that has no checksums and other integrity features.
Ted's observation was about maliciously-crafted filesystems, though, so
integrity-only features such as metadata checksums are irrelevant. Also the
filesystem version is irrelevant; anything accepted by the kernel code (even if
it's legacy/deprecated) is open attack surface.
I personally consider it *mandatory* that we deal with this stuff. But I can
understand that we don't do a good job at it, so we shouldn't hold a new
filesystem to an unfairly high standard relative to other filesystems...
- Eric
On Sun, Aug 18, 2019 at 09:16:38AM -0700, Eric Biggers wrote:
> Ted's observation was about maliciously-crafted filesystems, though, so
> integrity-only features such as metadata checksums are irrelevant. Also the
> filesystem version is irrelevant; anything accepted by the kernel code (even if
I think allowing users to mount file systems (any of ours) without
privilege is a rather bad idea. But that doesn't mean we should not be
as robust as we can. Optionally disabling support for legacy formats
at compile and/or runtime is something we should actively look into as
well.
> it's legacy/deprecated) is open attack surface.
>
> I personally consider it *mandatory* that we deal with this stuff. But I can
> understand that we don't do a good job at it, so we shouldn't hold a new
> filesystem to an unfairly high standard relative to other filesystems...
I very much disagree. We can't really force anyone to fix up old file
systems. But we can very much hold new ones to (slightly) higher
standards. Thats the only way to get the average quality up. Some as
for things like code style - we can't magically fix up all old stuff,
but we can and usually do hold new code to higher standards. (Often not
to standards as high as I'd personally prefer, btw).
Hi Hch,
On Sun, Aug 18, 2019 at 09:22:01AM -0700, Christoph Hellwig wrote:
> On Sun, Aug 18, 2019 at 09:16:38AM -0700, Eric Biggers wrote:
> > Ted's observation was about maliciously-crafted filesystems, though, so
> > integrity-only features such as metadata checksums are irrelevant. Also the
> > filesystem version is irrelevant; anything accepted by the kernel code (even if
>
> I think allowing users to mount file systems (any of ours) without
> privilege is a rather bad idea. But that doesn't mean we should not be
> as robust as we can. Optionally disabling support for legacy formats
> at compile and/or runtime is something we should actively look into as
> well.
>
> > it's legacy/deprecated) is open attack surface.
> >
> > I personally consider it *mandatory* that we deal with this stuff. But I can
> > understand that we don't do a good job at it, so we shouldn't hold a new
> > filesystem to an unfairly high standard relative to other filesystems...
>
> I very much disagree. We can't really force anyone to fix up old file
> systems. But we can very much hold new ones to (slightly) higher
> standards. Thats the only way to get the average quality up. Some as
> for things like code style - we can't magically fix up all old stuff,
> but we can and usually do hold new code to higher standards. (Often not
> to standards as high as I'd personally prefer, btw).
I personally don't want to discuss about other fses here...
I think XFS developers do great jobs all the time and
EROFS is a simple file system compared with these
generic file systems.
I can promise you that our team will fix bug reports in time, and
I personally think the current EROFS code is not as bad as a bullsh**t...
If you have some time, I'm very happy if you can take some of
your precious time on our work...
Thanks,
Gao Xiang
----- Ursprüngliche Mail -----
> So holding a file system like EROFS to a higher standard than say,
> ext4, xfs, or btrfs hardly seems fair.
Nobody claimed that.
Thanks,
//richard
On Sun, Aug 18, 2019 at 09:22:01AM -0700, Christoph Hellwig wrote:
> On Sun, Aug 18, 2019 at 09:16:38AM -0700, Eric Biggers wrote:
> > Ted's observation was about maliciously-crafted filesystems, though, so
> > integrity-only features such as metadata checksums are irrelevant. Also the
> > filesystem version is irrelevant; anything accepted by the kernel code (even if
>
> I think allowing users to mount file systems (any of ours) without
> privilege is a rather bad idea. But that doesn't mean we should not be
> as robust as we can. Optionally disabling support for legacy formats
> at compile and/or runtime is something we should actively look into as
> well.
>
> > it's legacy/deprecated) is open attack surface.
> >
> > I personally consider it *mandatory* that we deal with this stuff. But I can
> > understand that we don't do a good job at it, so we shouldn't hold a new
> > filesystem to an unfairly high standard relative to other filesystems...
>
> I very much disagree. We can't really force anyone to fix up old file
> systems. But we can very much hold new ones to (slightly) higher
> standards. Thats the only way to get the average quality up. Some as
> for things like code style - we can't magically fix up all old stuff,
> but we can and usually do hold new code to higher standards. (Often not
> to standards as high as I'd personally prefer, btw).
Not sure what you're even disagreeing with, as I *do* expect new filesystems to
be held to a high standard, and to be written with the assumption that the
on-disk data may be corrupted or malicious. We just can't expect the bar to be
so high (e.g. no bugs) that it's never been attained by *any* filesystem even
after years/decades of active development. If the developers were careful, the
code generally looks robust, and they are willing to address such bugs as they
are found, realistically that's as good as we can expect to get...
- Eric
On Sun, Aug 18, 2019 at 08:58:12AM -0700, Christoph Hellwig wrote:
> On Sun, Aug 18, 2019 at 11:11:54AM -0400, Theodore Y. Ts'o wrote:
> > Note that of the mainstream file systems, ext4 and xfs don't guarantee
> > that it's safe to blindly take maliciously provided file systems, such
> > as those provided by a untrusted container, and mount it on a file
> > system without problems. As I recall, one of the XFS developers
> > described file system fuzzing reports as a denial of service attack on
> > the developers.
>
> I think this greatly misrepresents the general attitute of the XFS
> developers. We take sanity checks for the modern v5 on disk format
> very series, and put a lot of effort into handling corrupted file
> systems as good as possible, although there are of course no guaranteeѕ.
>
> The quote that you've taken out of context is for the legacy v4 format
> that has no checksums and other integrity features.
Actually, what Prof. Kim's research group was doing was taking the
latest file system formats (for ext4 and xfs) and fixing up the
checksum after fuzzing the metadata blocks. The goal was to find
potential security vulnerabilities, not to see if file systems would
crash if fed invalid input. At least for ext4, at least one of
Prof. Kim's fuzzing results was one that that I believe could have
been leveraged into a stack overflow attack. I can't speak to his
results with respect to XFS, since I didn't look at them.
Cheers,
- Ted
On Sun, Aug 18, 2019 at 07:06:40PM +0200, Richard Weinberger wrote:
> > So holding a file system like EROFS to a higher standard than say,
> > ext4, xfs, or btrfs hardly seems fair.
>
> Nobody claimed that.
Pointing out that erofs has issues in this area when Gao Xiang is
asking if erofs can be moved out of staging and join the "official
clubhouse" of file systems could certainly be reasonable interpreted
as such. Reporting such vulnerablities are a good thing, and
hopefully all file system maintainers will welcome them. Doing them
on a e-mail thread about promoting out of erofs is certainly going to
lead to inferences of a double standard.
Cheers,
- Ted
On Sun, Aug 18, 2019 at 10:29:38AM -0700, Eric Biggers wrote:
> Not sure what you're even disagreeing with, as I *do* expect new filesystems to
> be held to a high standard, and to be written with the assumption that the
> on-disk data may be corrupted or malicious. We just can't expect the bar to be
> so high (e.g. no bugs) that it's never been attained by *any* filesystem even
> after years/decades of active development. If the developers were careful, the
> code generally looks robust, and they are willing to address such bugs as they
> are found, realistically that's as good as we can expect to get...
Well, the impression I got from Richards quick look and the reply to it is
that there is very little attempt to validate the ondisk data structure
and there is absolutely no priority to do so. Which is very different
from there is a bug or two here and there.
----- Ursprüngliche Mail -----
> Von: "tytso" <[email protected]>
> An: "richard" <[email protected]>
> CC: "Greg Kroah-Hartman" <[email protected]>, "Gao Xiang" <[email protected]>, "Jan Kara" <[email protected]>, "Chao
> Yu" <[email protected]>, "Dave Chinner" <[email protected]>, "David Sterba" <[email protected]>, "Miao Xie"
> <[email protected]>, "devel" <[email protected]>, "Stephen Rothwell" <[email protected]>, "Darrick"
> <[email protected]>, "Christoph Hellwig" <[email protected]>, "Amir Goldstein" <[email protected]>,
> "linux-erofs" <[email protected]>, "Al Viro" <[email protected]>, "Jaegeuk Kim" <[email protected]>,
> "linux-kernel" <[email protected]>, "Li Guifu" <[email protected]>, "Fang Wei" <[email protected]>,
> "Pavel Machek" <[email protected]>, "linux-fsdevel" <[email protected]>, "Andrew Morton"
> <[email protected]>, "torvalds" <[email protected]>
> Gesendet: Sonntag, 18. August 2019 19:46:21
> Betreff: Re: [PATCH] erofs: move erofs out of staging
> On Sun, Aug 18, 2019 at 07:06:40PM +0200, Richard Weinberger wrote:
>> > So holding a file system like EROFS to a higher standard than say,
>> > ext4, xfs, or btrfs hardly seems fair.
>>
>> Nobody claimed that.
>
> Pointing out that erofs has issues in this area when Gao Xiang is
> asking if erofs can be moved out of staging and join the "official
> clubhouse" of file systems could certainly be reasonable interpreted
> as such. Reporting such vulnerablities are a good thing, and
> hopefully all file system maintainers will welcome them. Doing them
> on a e-mail thread about promoting out of erofs is certainly going to
> lead to inferences of a double standard.
Well, this was not at all my intention.
erofs raised my attention and instead of wasting a new thread
I answered here and reported what I found while looking at it.
That's all.
Thanks,
//richard
Hi Hch,
On Sun, Aug 18, 2019 at 10:47:02AM -0700, Christoph Hellwig wrote:
> On Sun, Aug 18, 2019 at 10:29:38AM -0700, Eric Biggers wrote:
> > Not sure what you're even disagreeing with, as I *do* expect new filesystems to
> > be held to a high standard, and to be written with the assumption that the
> > on-disk data may be corrupted or malicious. We just can't expect the bar to be
> > so high (e.g. no bugs) that it's never been attained by *any* filesystem even
> > after years/decades of active development. If the developers were careful, the
> > code generally looks robust, and they are willing to address such bugs as they
> > are found, realistically that's as good as we can expect to get...
>
> Well, the impression I got from Richards quick look and the reply to it is
> that there is very little attempt to validate the ondisk data structure
> and there is absolutely no priority to do so. Which is very different
> from there is a bug or two here and there.
As my second reply to Richard, I didn't fuzz all the on-disk fields for EROFS.
and as my reply to Richard / Greg, current EROFS is used on the top of dm-verity.
I cannot say how well EROFS will be performed on malformed images (and you can
also find the bug richard pointed out is a miswritten break->continue by myself).
I posted the upstream EROFS post on July 4, 2019 and a month and a half later,
no one can tell me (yes, thanks for kind people reply me about their suggestion)
what we should do next (you can see these emails, I sent many times) to meet
the minimal upstream requirements and rare people can even dip into my code.
That is all I want to say. I will work on autofuzz these days, and I want to
know how to meet your requirements on this (you can tell us your standard,
how well should we do).
OK, you don't reply to my post once, I have no idea how to get your first reply.
Thanks,
Gao Xiang
Hi Richard,
On Sun, Aug 18, 2019 at 08:00:28PM +0200, Richard Weinberger wrote:
> ----- Urspr?ngliche Mail -----
> > Von: "tytso" <[email protected]>
> > An: "richard" <[email protected]>
> > CC: "Greg Kroah-Hartman" <[email protected]>, "Gao Xiang" <[email protected]>, "Jan Kara" <[email protected]>, "Chao
> > Yu" <[email protected]>, "Dave Chinner" <[email protected]>, "David Sterba" <[email protected]>, "Miao Xie"
> > <[email protected]>, "devel" <[email protected]>, "Stephen Rothwell" <[email protected]>, "Darrick"
> > <[email protected]>, "Christoph Hellwig" <[email protected]>, "Amir Goldstein" <[email protected]>,
> > "linux-erofs" <[email protected]>, "Al Viro" <[email protected]>, "Jaegeuk Kim" <[email protected]>,
> > "linux-kernel" <[email protected]>, "Li Guifu" <[email protected]>, "Fang Wei" <[email protected]>,
> > "Pavel Machek" <[email protected]>, "linux-fsdevel" <[email protected]>, "Andrew Morton"
> > <[email protected]>, "torvalds" <[email protected]>
> > Gesendet: Sonntag, 18. August 2019 19:46:21
> > Betreff: Re: [PATCH] erofs: move erofs out of staging
>
> > On Sun, Aug 18, 2019 at 07:06:40PM +0200, Richard Weinberger wrote:
> >> > So holding a file system like EROFS to a higher standard than say,
> >> > ext4, xfs, or btrfs hardly seems fair.
> >>
> >> Nobody claimed that.
> >
> > Pointing out that erofs has issues in this area when Gao Xiang is
> > asking if erofs can be moved out of staging and join the "official
> > clubhouse" of file systems could certainly be reasonable interpreted
> > as such. Reporting such vulnerablities are a good thing, and
> > hopefully all file system maintainers will welcome them. Doing them
> > on a e-mail thread about promoting out of erofs is certainly going to
> > lead to inferences of a double standard.
>
> Well, this was not at all my intention.
> erofs raised my attention and instead of wasting a new thread
> I answered here and reported what I found while looking at it.
> That's all.
Thank you very much, EROFS finally has some real concern
after a quite long time. I will do that but I really want
to upstream for 5.4LTS and hope to get your further report.
Thanks,
Gao Xiang
>
> Thanks,
> //richard
Hi all,
On Mon, Aug 19, 2019 at 02:16:55AM +0800, Gao Xiang wrote:
> Hi Hch,
>
> On Sun, Aug 18, 2019 at 10:47:02AM -0700, Christoph Hellwig wrote:
> > On Sun, Aug 18, 2019 at 10:29:38AM -0700, Eric Biggers wrote:
> > > Not sure what you're even disagreeing with, as I *do* expect new filesystems to
> > > be held to a high standard, and to be written with the assumption that the
> > > on-disk data may be corrupted or malicious. We just can't expect the bar to be
> > > so high (e.g. no bugs) that it's never been attained by *any* filesystem even
> > > after years/decades of active development. If the developers were careful, the
> > > code generally looks robust, and they are willing to address such bugs as they
> > > are found, realistically that's as good as we can expect to get...
> >
> > Well, the impression I got from Richards quick look and the reply to it is
> > that there is very little attempt to validate the ondisk data structure
> > and there is absolutely no priority to do so. Which is very different
> > from there is a bug or two here and there.
>
> As my second reply to Richard, I didn't fuzz all the on-disk fields for EROFS.
> and as my reply to Richard / Greg, current EROFS is used on the top of dm-verity.
>
> I cannot say how well EROFS will be performed on malformed images (and you can
> also find the bug richard pointed out is a miswritten break->continue by myself).
>
> I posted the upstream EROFS post on July 4, 2019 and a month and a half later,
> no one can tell me (yes, thanks for kind people reply me about their suggestion)
> what we should do next (you can see these emails, I sent many times) to meet
> the minimal upstream requirements and rare people can even dip into my code.
>
> That is all I want to say. I will work on autofuzz these days, and I want to
> know how to meet your requirements on this (you can tell us your standard,
> how well should we do).
>
> OK, you don't reply to my post once, I have no idea how to get your first reply.
I have made a simple fuzzer to inject messy in inode metadata,
dir data, compressed indexes and super block,
https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
I am testing with some given dirs and the following script.
Does it look reasonable?
# !/bin/bash
mkdir -p mntdir
for ((i=0; i<1000; ++i)); do
mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
umount mntdir
mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
for j in `find mntdir -type f`; do
md5sum $j > /dev/null
done
done
Thanks,
Gao Xiang
>
> Thanks,
> Gao Xiang
>
Rename errln, infoln, and debugln to the typical pr_<level> uses
to the typical kernel styles of pr_<level>
Miscellanea:
o Add newline terminations to the uses
o Use "%s: ...", __func__ and not the atypical "%s, ...", __func__
o Trivial grammar changes in output logging
o Delete the now unused macros
Signed-off-by: Joe Perches <[email protected]>
---
drivers/staging/erofs/data.c | 6 ++--
drivers/staging/erofs/decompressor.c | 6 ++--
drivers/staging/erofs/dir.c | 8 +++---
drivers/staging/erofs/inode.c | 16 +++++------
drivers/staging/erofs/internal.h | 8 ++----
drivers/staging/erofs/namei.c | 4 +--
drivers/staging/erofs/super.c | 54 +++++++++++++++++++-----------------
drivers/staging/erofs/xattr.c | 4 +--
drivers/staging/erofs/zdata.c | 12 ++++----
drivers/staging/erofs/zdata.h | 2 +-
drivers/staging/erofs/zmap.c | 26 ++++++++---------
11 files changed, 73 insertions(+), 73 deletions(-)
diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index 4cdb743c8b8d..677d95e8fdeb 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -152,8 +152,8 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
map->m_flags |= EROFS_MAP_META;
} else {
- errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
- vi->nid, inode->i_size, map->m_la);
+ pr_err("internal error @ nid: %llu (size %llu), m_la 0x%llx\n",
+ vi->nid, inode->i_size, map->m_la);
DBG_BUGON(1);
err = -EIO;
goto err_out;
@@ -363,7 +363,7 @@ static int erofs_raw_access_readpages(struct file *filp,
/* all the page errors are ignored when readahead */
if (IS_ERR(bio)) {
- pr_err("%s, readahead error at page %lu of nid %llu\n",
+ pr_err("%s: readahead error at page %lu of nid %llu\n",
__func__, page->index,
EROFS_V(mapping->host)->nid);
diff --git a/drivers/staging/erofs/decompressor.c b/drivers/staging/erofs/decompressor.c
index 5361a2bbedb6..24d450ce66c1 100644
--- a/drivers/staging/erofs/decompressor.c
+++ b/drivers/staging/erofs/decompressor.c
@@ -166,9 +166,9 @@ static int lz4_decompress(struct z_erofs_decompress_req *rq, u8 *out)
inlen, rq->outputsize,
rq->outputsize);
if (ret < 0) {
- errln("%s, failed to decompress, in[%p, %u, %u] out[%p, %u]",
- __func__, src + inputmargin, inlen, inputmargin,
- out, rq->outputsize);
+ pr_err("%s: failed to decompress, in[%p, %u, %u] out[%p, %u]\n",
+ __func__, src + inputmargin, inlen, inputmargin,
+ out, rq->outputsize);
WARN_ON(1);
print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
16, 1, src + inputmargin, inlen, true);
diff --git a/drivers/staging/erofs/dir.c b/drivers/staging/erofs/dir.c
index 2fbfc4935077..526c7b5dd4db 100644
--- a/drivers/staging/erofs/dir.c
+++ b/drivers/staging/erofs/dir.c
@@ -29,8 +29,8 @@ static void debug_one_dentry(unsigned char d_type, const char *de_name,
memcpy(dbg_namebuf, de_name, de_namelen);
dbg_namebuf[de_namelen] = '\0';
- debugln("found dirent %s de_len %u d_type %d", dbg_namebuf,
- de_namelen, d_type);
+ pr_debug("found dirent %s de_len %u d_type %d\n",
+ dbg_namebuf, de_namelen, d_type);
#endif
}
@@ -104,8 +104,8 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
if (unlikely(nameoff < sizeof(struct erofs_dirent) ||
nameoff >= PAGE_SIZE)) {
- errln("%s, invalid de[0].nameoff %u",
- __func__, nameoff);
+ pr_err("%s: invalid de[0].nameoff %u\n",
+ __func__, nameoff);
err = -EIO;
goto skip_this;
diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index 286729143365..7b91f3baf8d4 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -21,8 +21,8 @@ static int read_inode(struct inode *inode, void *data)
vi->datamode = __inode_data_mapping(advise);
if (unlikely(vi->datamode >= EROFS_INODE_LAYOUT_MAX)) {
- errln("unsupported data mapping %u of nid %llu",
- vi->datamode, vi->nid);
+ pr_err("unsupported data mapping %u of nid %llu\n",
+ vi->datamode, vi->nid);
DBG_BUGON(1);
return -EIO;
}
@@ -92,8 +92,8 @@ static int read_inode(struct inode *inode, void *data)
if (is_inode_layout_compression(inode))
nblks = le32_to_cpu(v1->i_u.compressed_blocks);
} else {
- errln("unsupported on-disk inode version %u of nid %llu",
- __inode_version(advise), vi->nid);
+ pr_err("unsupported on-disk inode version %u of nid %llu\n",
+ __inode_version(advise), vi->nid);
DBG_BUGON(1);
return -EIO;
}
@@ -167,14 +167,14 @@ static int fill_inode(struct inode *inode, int isdir)
blkaddr = erofs_blknr(iloc(sbi, vi->nid));
ofs = erofs_blkoff(iloc(sbi, vi->nid));
- debugln("%s, reading inode nid %llu at %u of blkaddr %u",
- __func__, vi->nid, ofs, blkaddr);
+ pr_debug("%s: reading inode nid %llu at %u of blkaddr %u\n",
+ __func__, vi->nid, ofs, blkaddr);
page = erofs_get_meta_page(inode->i_sb, blkaddr, isdir);
if (IS_ERR(page)) {
- errln("failed to get inode (nid: %llu) page, err %ld",
- vi->nid, PTR_ERR(page));
+ pr_err("failed to get inode (nid: %llu) page, err %ld\n",
+ vi->nid, PTR_ERR(page));
return PTR_ERR(page);
}
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 4ce5991c381f..3833ae713355 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -23,13 +23,10 @@
#undef pr_fmt
#define pr_fmt(fmt) "erofs: " fmt
-#define errln(x, ...) pr_err(x "\n", ##__VA_ARGS__)
-#define infoln(x, ...) pr_info(x "\n", ##__VA_ARGS__)
#ifdef CONFIG_EROFS_FS_DEBUG
-#define debugln(x, ...) pr_debug(x "\n", ##__VA_ARGS__)
+#define DEBUG
#define DBG_BUGON BUG_ON
#else
-#define debugln(x, ...) ((void)0)
#define DBG_BUGON(x) ((void)(x))
#endif /* !CONFIG_EROFS_FS_DEBUG */
@@ -108,7 +105,8 @@ struct erofs_sb_info {
#ifdef CONFIG_EROFS_FAULT_INJECTION
#define erofs_show_injection_info(type) \
- infoln("inject %s in %s of %pS", erofs_fault_name[type], \
+ pr_info("inject %s in %s of %pS\n", \
+ erofs_fault_name[type], \
__func__, __builtin_return_address(0))
static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
index 8e06526da023..1cba4d471433 100644
--- a/drivers/staging/erofs/namei.c
+++ b/drivers/staging/erofs/namei.c
@@ -233,8 +233,8 @@ static struct dentry *erofs_lookup(struct inode *dir,
} else if (unlikely(err)) {
inode = ERR_PTR(err);
} else {
- debugln("%s, %s (nid %llu) found, d_type %u", __func__,
- dentry->d_name.name, nid, d_type);
+ pr_debug("%s: %s (nid %llu) found, d_type %u\n",
+ __func__, dentry->d_name.name, nid, d_type);
inode = erofs_iget(dir->i_sb, nid, d_type == EROFS_FT_DIR);
}
return d_splice_alias(inode, dentry);
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index f65a1ff9f42f..97096bfa5e73 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -75,8 +75,8 @@ static bool check_layout_compatibility(struct super_block *sb,
/* check if current kernel meets all mandatory requirements */
if (requirements & (~EROFS_ALL_REQUIREMENTS)) {
- errln("unidentified requirements %x, please upgrade kernel version",
- requirements & ~EROFS_ALL_REQUIREMENTS);
+ pr_err("unidentified requirements %x, please upgrade kernel version\n",
+ requirements & ~EROFS_ALL_REQUIREMENTS);
return false;
}
return true;
@@ -93,7 +93,7 @@ static int superblock_read(struct super_block *sb)
bh = sb_bread(sb, 0);
if (!bh) {
- errln("cannot read erofs superblock");
+ pr_err("cannot read erofs superblock\n");
return -EIO;
}
@@ -103,15 +103,15 @@ static int superblock_read(struct super_block *sb)
ret = -EINVAL;
if (le32_to_cpu(layout->magic) != EROFS_SUPER_MAGIC_V1) {
- errln("cannot find valid erofs superblock");
+ pr_err("cannot find valid erofs superblock\n");
goto out;
}
blkszbits = layout->blkszbits;
/* 9(512 bytes) + LOG_SECTORS_PER_BLOCK == LOG_BLOCK_SIZE */
if (unlikely(blkszbits != LOG_BLOCK_SIZE)) {
- errln("blksize %u isn't supported on this platform",
- 1 << blkszbits);
+ pr_err("blksize %u isn't supported on this platform\n",
+ 1 << blkszbits);
goto out;
}
@@ -187,7 +187,7 @@ static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
substring_t *args)
{
- infoln("fault_injection options not supported");
+ pr_info("fault_injection options not supported\n");
return 0;
}
@@ -205,7 +205,7 @@ static int erofs_build_cache_strategy(struct erofs_sb_info *sbi,
int err = 0;
if (!cs) {
- errln("Not enough memory to store cache strategy");
+ pr_err("Not enough memory to store cache strategy\n");
return -ENOMEM;
}
@@ -216,7 +216,7 @@ static int erofs_build_cache_strategy(struct erofs_sb_info *sbi,
} else if (!strcmp(cs, "readaround")) {
sbi->cache_strategy = EROFS_ZIP_CACHE_READAROUND;
} else {
- errln("Unrecognized cache strategy \"%s\"", cs);
+ pr_err("Unrecognized cache strategy \"%s\"\n", cs);
err = -EINVAL;
}
kfree(cs);
@@ -226,7 +226,7 @@ static int erofs_build_cache_strategy(struct erofs_sb_info *sbi,
static int erofs_build_cache_strategy(struct erofs_sb_info *sbi,
substring_t *args)
{
- infoln("EROFS compression is disabled, so cache strategy is ignored");
+ pr_info("EROFS compression is disabled, so cache strategy is ignored\n");
return 0;
}
#endif
@@ -294,10 +294,10 @@ static int parse_options(struct super_block *sb, char *options)
break;
#else
case Opt_user_xattr:
- infoln("user_xattr options not supported");
+ pr_info("user_xattr options not supported\n");
break;
case Opt_nouser_xattr:
- infoln("nouser_xattr options not supported");
+ pr_info("nouser_xattr options not supported\n");
break;
#endif
#ifdef CONFIG_EROFS_FS_POSIX_ACL
@@ -309,10 +309,10 @@ static int parse_options(struct super_block *sb, char *options)
break;
#else
case Opt_acl:
- infoln("acl options not supported");
+ pr_info("acl options not supported\n");
break;
case Opt_noacl:
- infoln("noacl options not supported");
+ pr_info("noacl options not supported\n");
break;
#endif
case Opt_fault_injection:
@@ -326,7 +326,8 @@ static int parse_options(struct super_block *sb, char *options)
return err;
break;
default:
- errln("Unrecognized mount option \"%s\" or missing value", p);
+ pr_err("Unrecognized mount option \"%s\" or missing value\n",
+ p);
return -EINVAL;
}
}
@@ -398,13 +399,13 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent)
struct erofs_sb_info *sbi;
int err;
- infoln("fill_super, device -> %s", sb->s_id);
- infoln("options -> %s", (char *)data);
+ pr_info("%s: device -> %s\n", __func__, sb->s_id);
+ pr_info("options -> %s\n", (char *)data);
sb->s_magic = EROFS_SUPER_MAGIC;
if (unlikely(!sb_set_blocksize(sb, EROFS_BLKSIZ))) {
- errln("failed to set erofs blksize");
+ pr_err("failed to set erofs blksize\n");
return -EINVAL;
}
@@ -434,7 +435,7 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent)
return err;
if (!silent)
- infoln("root inode @ nid %llu", ROOT_NID(sbi));
+ pr_info("root inode @ nid %llu\n", ROOT_NID(sbi));
if (test_opt(sbi, POSIX_ACL))
sb->s_flags |= SB_POSIXACL;
@@ -451,8 +452,8 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent)
return PTR_ERR(inode);
if (unlikely(!S_ISDIR(inode->i_mode))) {
- errln("rootino(nid %llu) is not a directory(i_mode %o)",
- ROOT_NID(sbi), inode->i_mode);
+ pr_err("rootino(nid %llu) is not a directory(i_mode %o)\n",
+ ROOT_NID(sbi), inode->i_mode);
iput(inode);
return -EINVAL;
}
@@ -468,7 +469,8 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent)
return err;
if (!silent)
- infoln("mounted on %s with opts: %s.", sb->s_id, (char *)data);
+ pr_info("mounted on %s with opts: %s\n",
+ sb->s_id, (char *)data);
return 0;
}
@@ -487,7 +489,7 @@ static void erofs_kill_sb(struct super_block *sb)
struct erofs_sb_info *sbi;
WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
- infoln("unmounting for %s", sb->s_id);
+ pr_info("unmounting for %s\n", sb->s_id);
kill_block_super(sb);
@@ -526,7 +528,7 @@ static int __init erofs_module_init(void)
int err;
erofs_check_ondisk_layout_definitions();
- infoln("initializing erofs " EROFS_VERSION);
+ pr_info("initializing erofs " EROFS_VERSION "\n");
err = erofs_init_inode_cache();
if (err)
@@ -544,7 +546,7 @@ static int __init erofs_module_init(void)
if (err)
goto fs_err;
- infoln("successfully to initialize erofs");
+ pr_info("successfully initialized erofs\n");
return 0;
fs_err:
@@ -563,7 +565,7 @@ static void __exit erofs_module_exit(void)
z_erofs_exit_zip_subsystem();
erofs_exit_shrinker();
erofs_exit_inode_cache();
- infoln("successfully finalize erofs");
+ pr_info("successfully finalized erofs\n");
}
/* get filesystem statistics */
diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index 289c7850ec96..e774a8c1bfae 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -69,8 +69,8 @@ static int init_inode_xattrs(struct inode *inode)
* undefined right now (maybe use later with some new sb feature).
*/
if (vi->xattr_isize == sizeof(struct erofs_xattr_ibody_header)) {
- errln("xattr_isize %d of nid %llu is not supported yet",
- vi->xattr_isize, vi->nid);
+ pr_err("xattr_isize %d of nid %llu is not supported yet\n",
+ vi->xattr_isize, vi->nid);
ret = -ENOTSUPP;
goto out_unlock;
} else if (vi->xattr_isize < sizeof(struct erofs_xattr_ibody_header)) {
diff --git a/drivers/staging/erofs/zdata.c b/drivers/staging/erofs/zdata.c
index 2d7aaf98f7de..17daf286747e 100644
--- a/drivers/staging/erofs/zdata.c
+++ b/drivers/staging/erofs/zdata.c
@@ -585,7 +585,7 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
}
/* go ahead the next map_blocks */
- debugln("%s: [out-of-range] pos %llu", __func__, offset + cur);
+ pr_debug("%s: [out-of-range] pos %llu\n", __func__, offset + cur);
if (z_erofs_collector_end(clt))
fe->backmost = false;
@@ -665,8 +665,8 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
out:
z_erofs_onlinepage_endio(page);
- debugln("%s, finish page: %pK spiltted: %u map->m_llen %llu",
- __func__, page, spiltted, map->m_llen);
+ pr_debug("%s: finish page: %pK spiltted: %u map->m_llen %llu\n",
+ __func__, page, spiltted, map->m_llen);
return err;
/* if some error occurred while processing this page */
@@ -1308,7 +1308,7 @@ static int z_erofs_vle_normalaccess_readpage(struct file *file,
(void)z_erofs_collector_end(&f.clt);
if (err) {
- errln("%s, failed to read, err [%d]", __func__, err);
+ pr_err("%s: failed to read, err [%d]\n", __func__, err);
goto out;
}
@@ -1380,8 +1380,8 @@ static int z_erofs_vle_normalaccess_readpages(struct file *filp,
if (err) {
struct erofs_vnode *vi = EROFS_V(inode);
- errln("%s, readahead error at page %lu of nid %llu",
- __func__, page->index, vi->nid);
+ pr_err("%s: readahead error at page %lu of nid %llu\n",
+ __func__, page->index, vi->nid);
}
put_page(page);
}
diff --git a/drivers/staging/erofs/zdata.h b/drivers/staging/erofs/zdata.h
index e11fe1959ca2..e96e8ee270d2 100644
--- a/drivers/staging/erofs/zdata.h
+++ b/drivers/staging/erofs/zdata.h
@@ -184,7 +184,7 @@ static inline void z_erofs_onlinepage_endio(struct page *page)
SetPageUptodate(page);
unlock_page(page);
}
- debugln("%s, page %p value %x", __func__, page, atomic_read(u.o));
+ pr_debug("%s: page %p value %x\n", __func__, page, atomic_read(u.o));
}
#define Z_EROFS_VMAP_ONSTACK_PAGES \
diff --git a/drivers/staging/erofs/zmap.c b/drivers/staging/erofs/zmap.c
index aeed5c674d9e..b2adf531379a 100644
--- a/drivers/staging/erofs/zmap.c
+++ b/drivers/staging/erofs/zmap.c
@@ -66,8 +66,8 @@ static int fill_inode_lazy(struct inode *inode)
vi->z_algorithmtype[1] = h->h_algorithmtype >> 4;
if (vi->z_algorithmtype[0] >= Z_EROFS_COMPRESSION_MAX) {
- errln("unknown compression format %u for nid %llu, please upgrade kernel",
- vi->z_algorithmtype[0], vi->nid);
+ pr_err("unknown compression format %u for nid %llu, please upgrade kernel\n",
+ vi->z_algorithmtype[0], vi->nid);
err = -ENOTSUPP;
goto unmap_done;
}
@@ -77,8 +77,8 @@ static int fill_inode_lazy(struct inode *inode)
((h->h_clusterbits >> 3) & 3);
if (vi->z_physical_clusterbits[0] != LOG_BLOCK_SIZE) {
- errln("unsupported physical clusterbits %u for nid %llu, please upgrade kernel",
- vi->z_physical_clusterbits[0], vi->nid);
+ pr_err("unsupported physical clusterbits %u for nid %llu, please upgrade kernel\n",
+ vi->z_physical_clusterbits[0], vi->nid);
err = -ENOTSUPP;
goto unmap_done;
}
@@ -358,8 +358,8 @@ static int vle_extent_lookback(struct z_erofs_maprecorder *m,
map->m_la = (lcn << lclusterbits) | m->clusterofs;
break;
default:
- errln("unknown type %u at lcn %lu of nid %llu",
- m->type, lcn, vi->nid);
+ pr_err("unknown type %u at lcn %lu of nid %llu\n",
+ m->type, lcn, vi->nid);
DBG_BUGON(1);
return -EIO;
}
@@ -417,8 +417,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
}
/* m.lcn should be >= 1 if endoff < m.clusterofs */
if (unlikely(!m.lcn)) {
- errln("invalid logical cluster 0 at nid %llu",
- vi->nid);
+ pr_err("invalid logical cluster 0 at nid %llu\n",
+ vi->nid);
err = -EIO;
goto unmap_out;
}
@@ -433,8 +433,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
goto unmap_out;
break;
default:
- errln("unknown type %u at offset %llu of nid %llu",
- m.type, ofs, vi->nid);
+ pr_err("unknown type %u at offset %llu of nid %llu\n",
+ m.type, ofs, vi->nid);
err = -EIO;
goto unmap_out;
}
@@ -449,9 +449,9 @@ int z_erofs_map_blocks_iter(struct inode *inode,
kunmap_atomic(m.kaddr);
out:
- debugln("%s, m_la %llu m_pa %llu m_llen %llu m_plen %llu m_flags 0%o",
- __func__, map->m_la, map->m_pa,
- map->m_llen, map->m_plen, map->m_flags);
+ pr_debug("%s: m_la %llu m_pa %llu m_llen %llu m_plen %llu m_flags 0%o\n",
+ __func__, map->m_la, map->m_pa,
+ map->m_llen, map->m_plen, map->m_flags);
trace_z_erofs_map_blocks_iter_exit(inode, map, flags, err);
Hi Joe,
On Sun, Aug 18, 2019 at 10:28:41PM -0700, Joe Perches wrote:
> Rename errln, infoln, and debugln to the typical pr_<level> uses
> to the typical kernel styles of pr_<level>
How about using erofs_err / ... to instead that?
- I can hardly see directly use pr_<level> for those filesystems in fs/...
- maybe we will introduce super_block to print more information
about the specific filesystem...
>
> Miscellanea:
>
> o Add newline terminations to the uses
> o Use "%s: ...", __func__ and not the atypical "%s, ...", __func__
Agreed.
Thanks,
Gao Xiang
> o Trivial grammar changes in output logging
> o Delete the now unused macros
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> drivers/staging/erofs/data.c | 6 ++--
> drivers/staging/erofs/decompressor.c | 6 ++--
> drivers/staging/erofs/dir.c | 8 +++---
> drivers/staging/erofs/inode.c | 16 +++++------
> drivers/staging/erofs/internal.h | 8 ++----
> drivers/staging/erofs/namei.c | 4 +--
> drivers/staging/erofs/super.c | 54 +++++++++++++++++++-----------------
> drivers/staging/erofs/xattr.c | 4 +--
> drivers/staging/erofs/zdata.c | 12 ++++----
> drivers/staging/erofs/zdata.h | 2 +-
> drivers/staging/erofs/zmap.c | 26 ++++++++---------
> 11 files changed, 73 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
> index 4cdb743c8b8d..677d95e8fdeb 100644
> --- a/drivers/staging/erofs/data.c
> +++ b/drivers/staging/erofs/data.c
> @@ -152,8 +152,8 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
>
> map->m_flags |= EROFS_MAP_META;
> } else {
> - errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
> - vi->nid, inode->i_size, map->m_la);
> + pr_err("internal error @ nid: %llu (size %llu), m_la 0x%llx\n",
> + vi->nid, inode->i_size, map->m_la);
> DBG_BUGON(1);
> err = -EIO;
> goto err_out;
> @@ -363,7 +363,7 @@ static int erofs_raw_access_readpages(struct file *filp,
>
> /* all the page errors are ignored when readahead */
> if (IS_ERR(bio)) {
> - pr_err("%s, readahead error at page %lu of nid %llu\n",
> + pr_err("%s: readahead error at page %lu of nid %llu\n",
> __func__, page->index,
> EROFS_V(mapping->host)->nid);
>
> diff --git a/drivers/staging/erofs/decompressor.c b/drivers/staging/erofs/decompressor.c
> index 5361a2bbedb6..24d450ce66c1 100644
> --- a/drivers/staging/erofs/decompressor.c
> +++ b/drivers/staging/erofs/decompressor.c
> @@ -166,9 +166,9 @@ static int lz4_decompress(struct z_erofs_decompress_req *rq, u8 *out)
> inlen, rq->outputsize,
> rq->outputsize);
> if (ret < 0) {
> - errln("%s, failed to decompress, in[%p, %u, %u] out[%p, %u]",
> - __func__, src + inputmargin, inlen, inputmargin,
> - out, rq->outputsize);
> + pr_err("%s: failed to decompress, in[%p, %u, %u] out[%p, %u]\n",
> + __func__, src + inputmargin, inlen, inputmargin,
> + out, rq->outputsize);
> WARN_ON(1);
> print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
> 16, 1, src + inputmargin, inlen, true);
> diff --git a/drivers/staging/erofs/dir.c b/drivers/staging/erofs/dir.c
> index 2fbfc4935077..526c7b5dd4db 100644
> --- a/drivers/staging/erofs/dir.c
> +++ b/drivers/staging/erofs/dir.c
> @@ -29,8 +29,8 @@ static void debug_one_dentry(unsigned char d_type, const char *de_name,
> memcpy(dbg_namebuf, de_name, de_namelen);
> dbg_namebuf[de_namelen] = '\0';
>
> - debugln("found dirent %s de_len %u d_type %d", dbg_namebuf,
> - de_namelen, d_type);
> + pr_debug("found dirent %s de_len %u d_type %d\n",
> + dbg_namebuf, de_namelen, d_type);
> #endif
> }
>
> @@ -104,8 +104,8 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
>
> if (unlikely(nameoff < sizeof(struct erofs_dirent) ||
> nameoff >= PAGE_SIZE)) {
> - errln("%s, invalid de[0].nameoff %u",
> - __func__, nameoff);
> + pr_err("%s: invalid de[0].nameoff %u\n",
> + __func__, nameoff);
>
> err = -EIO;
> goto skip_this;
> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> index 286729143365..7b91f3baf8d4 100644
> --- a/drivers/staging/erofs/inode.c
> +++ b/drivers/staging/erofs/inode.c
> @@ -21,8 +21,8 @@ static int read_inode(struct inode *inode, void *data)
> vi->datamode = __inode_data_mapping(advise);
>
> if (unlikely(vi->datamode >= EROFS_INODE_LAYOUT_MAX)) {
> - errln("unsupported data mapping %u of nid %llu",
> - vi->datamode, vi->nid);
> + pr_err("unsupported data mapping %u of nid %llu\n",
> + vi->datamode, vi->nid);
> DBG_BUGON(1);
> return -EIO;
> }
> @@ -92,8 +92,8 @@ static int read_inode(struct inode *inode, void *data)
> if (is_inode_layout_compression(inode))
> nblks = le32_to_cpu(v1->i_u.compressed_blocks);
> } else {
> - errln("unsupported on-disk inode version %u of nid %llu",
> - __inode_version(advise), vi->nid);
> + pr_err("unsupported on-disk inode version %u of nid %llu\n",
> + __inode_version(advise), vi->nid);
> DBG_BUGON(1);
> return -EIO;
> }
> @@ -167,14 +167,14 @@ static int fill_inode(struct inode *inode, int isdir)
> blkaddr = erofs_blknr(iloc(sbi, vi->nid));
> ofs = erofs_blkoff(iloc(sbi, vi->nid));
>
> - debugln("%s, reading inode nid %llu at %u of blkaddr %u",
> - __func__, vi->nid, ofs, blkaddr);
> + pr_debug("%s: reading inode nid %llu at %u of blkaddr %u\n",
> + __func__, vi->nid, ofs, blkaddr);
>
> page = erofs_get_meta_page(inode->i_sb, blkaddr, isdir);
>
> if (IS_ERR(page)) {
> - errln("failed to get inode (nid: %llu) page, err %ld",
> - vi->nid, PTR_ERR(page));
> + pr_err("failed to get inode (nid: %llu) page, err %ld\n",
> + vi->nid, PTR_ERR(page));
> return PTR_ERR(page);
> }
>
> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> index 4ce5991c381f..3833ae713355 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -23,13 +23,10 @@
> #undef pr_fmt
> #define pr_fmt(fmt) "erofs: " fmt
>
> -#define errln(x, ...) pr_err(x "\n", ##__VA_ARGS__)
> -#define infoln(x, ...) pr_info(x "\n", ##__VA_ARGS__)
> #ifdef CONFIG_EROFS_FS_DEBUG
> -#define debugln(x, ...) pr_debug(x "\n", ##__VA_ARGS__)
> +#define DEBUG
> #define DBG_BUGON BUG_ON
> #else
> -#define debugln(x, ...) ((void)0)
> #define DBG_BUGON(x) ((void)(x))
> #endif /* !CONFIG_EROFS_FS_DEBUG */
>
> @@ -108,7 +105,8 @@ struct erofs_sb_info {
>
> #ifdef CONFIG_EROFS_FAULT_INJECTION
> #define erofs_show_injection_info(type) \
> - infoln("inject %s in %s of %pS", erofs_fault_name[type], \
> + pr_info("inject %s in %s of %pS\n", \
> + erofs_fault_name[type], \
> __func__, __builtin_return_address(0))
>
> static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
> diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
> index 8e06526da023..1cba4d471433 100644
> --- a/drivers/staging/erofs/namei.c
> +++ b/drivers/staging/erofs/namei.c
> @@ -233,8 +233,8 @@ static struct dentry *erofs_lookup(struct inode *dir,
> } else if (unlikely(err)) {
> inode = ERR_PTR(err);
> } else {
> - debugln("%s, %s (nid %llu) found, d_type %u", __func__,
> - dentry->d_name.name, nid, d_type);
> + pr_debug("%s: %s (nid %llu) found, d_type %u\n",
> + __func__, dentry->d_name.name, nid, d_type);
> inode = erofs_iget(dir->i_sb, nid, d_type == EROFS_FT_DIR);
> }
> return d_splice_alias(inode, dentry);
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index f65a1ff9f42f..97096bfa5e73 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -75,8 +75,8 @@ static bool check_layout_compatibility(struct super_block *sb,
>
> /* check if current kernel meets all mandatory requirements */
> if (requirements & (~EROFS_ALL_REQUIREMENTS)) {
> - errln("unidentified requirements %x, please upgrade kernel version",
> - requirements & ~EROFS_ALL_REQUIREMENTS);
> + pr_err("unidentified requirements %x, please upgrade kernel version\n",
> + requirements & ~EROFS_ALL_REQUIREMENTS);
> return false;
> }
> return true;
> @@ -93,7 +93,7 @@ static int superblock_read(struct super_block *sb)
> bh = sb_bread(sb, 0);
>
> if (!bh) {
> - errln("cannot read erofs superblock");
> + pr_err("cannot read erofs superblock\n");
> return -EIO;
> }
>
> @@ -103,15 +103,15 @@ static int superblock_read(struct super_block *sb)
>
> ret = -EINVAL;
> if (le32_to_cpu(layout->magic) != EROFS_SUPER_MAGIC_V1) {
> - errln("cannot find valid erofs superblock");
> + pr_err("cannot find valid erofs superblock\n");
> goto out;
> }
>
> blkszbits = layout->blkszbits;
> /* 9(512 bytes) + LOG_SECTORS_PER_BLOCK == LOG_BLOCK_SIZE */
> if (unlikely(blkszbits != LOG_BLOCK_SIZE)) {
> - errln("blksize %u isn't supported on this platform",
> - 1 << blkszbits);
> + pr_err("blksize %u isn't supported on this platform\n",
> + 1 << blkszbits);
> goto out;
> }
>
> @@ -187,7 +187,7 @@ static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
> static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> substring_t *args)
> {
> - infoln("fault_injection options not supported");
> + pr_info("fault_injection options not supported\n");
> return 0;
> }
>
> @@ -205,7 +205,7 @@ static int erofs_build_cache_strategy(struct erofs_sb_info *sbi,
> int err = 0;
>
> if (!cs) {
> - errln("Not enough memory to store cache strategy");
> + pr_err("Not enough memory to store cache strategy\n");
> return -ENOMEM;
> }
>
> @@ -216,7 +216,7 @@ static int erofs_build_cache_strategy(struct erofs_sb_info *sbi,
> } else if (!strcmp(cs, "readaround")) {
> sbi->cache_strategy = EROFS_ZIP_CACHE_READAROUND;
> } else {
> - errln("Unrecognized cache strategy \"%s\"", cs);
> + pr_err("Unrecognized cache strategy \"%s\"\n", cs);
> err = -EINVAL;
> }
> kfree(cs);
> @@ -226,7 +226,7 @@ static int erofs_build_cache_strategy(struct erofs_sb_info *sbi,
> static int erofs_build_cache_strategy(struct erofs_sb_info *sbi,
> substring_t *args)
> {
> - infoln("EROFS compression is disabled, so cache strategy is ignored");
> + pr_info("EROFS compression is disabled, so cache strategy is ignored\n");
> return 0;
> }
> #endif
> @@ -294,10 +294,10 @@ static int parse_options(struct super_block *sb, char *options)
> break;
> #else
> case Opt_user_xattr:
> - infoln("user_xattr options not supported");
> + pr_info("user_xattr options not supported\n");
> break;
> case Opt_nouser_xattr:
> - infoln("nouser_xattr options not supported");
> + pr_info("nouser_xattr options not supported\n");
> break;
> #endif
> #ifdef CONFIG_EROFS_FS_POSIX_ACL
> @@ -309,10 +309,10 @@ static int parse_options(struct super_block *sb, char *options)
> break;
> #else
> case Opt_acl:
> - infoln("acl options not supported");
> + pr_info("acl options not supported\n");
> break;
> case Opt_noacl:
> - infoln("noacl options not supported");
> + pr_info("noacl options not supported\n");
> break;
> #endif
> case Opt_fault_injection:
> @@ -326,7 +326,8 @@ static int parse_options(struct super_block *sb, char *options)
> return err;
> break;
> default:
> - errln("Unrecognized mount option \"%s\" or missing value", p);
> + pr_err("Unrecognized mount option \"%s\" or missing value\n",
> + p);
> return -EINVAL;
> }
> }
> @@ -398,13 +399,13 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent)
> struct erofs_sb_info *sbi;
> int err;
>
> - infoln("fill_super, device -> %s", sb->s_id);
> - infoln("options -> %s", (char *)data);
> + pr_info("%s: device -> %s\n", __func__, sb->s_id);
> + pr_info("options -> %s\n", (char *)data);
>
> sb->s_magic = EROFS_SUPER_MAGIC;
>
> if (unlikely(!sb_set_blocksize(sb, EROFS_BLKSIZ))) {
> - errln("failed to set erofs blksize");
> + pr_err("failed to set erofs blksize\n");
> return -EINVAL;
> }
>
> @@ -434,7 +435,7 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent)
> return err;
>
> if (!silent)
> - infoln("root inode @ nid %llu", ROOT_NID(sbi));
> + pr_info("root inode @ nid %llu\n", ROOT_NID(sbi));
>
> if (test_opt(sbi, POSIX_ACL))
> sb->s_flags |= SB_POSIXACL;
> @@ -451,8 +452,8 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent)
> return PTR_ERR(inode);
>
> if (unlikely(!S_ISDIR(inode->i_mode))) {
> - errln("rootino(nid %llu) is not a directory(i_mode %o)",
> - ROOT_NID(sbi), inode->i_mode);
> + pr_err("rootino(nid %llu) is not a directory(i_mode %o)\n",
> + ROOT_NID(sbi), inode->i_mode);
> iput(inode);
> return -EINVAL;
> }
> @@ -468,7 +469,8 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent)
> return err;
>
> if (!silent)
> - infoln("mounted on %s with opts: %s.", sb->s_id, (char *)data);
> + pr_info("mounted on %s with opts: %s\n",
> + sb->s_id, (char *)data);
> return 0;
> }
>
> @@ -487,7 +489,7 @@ static void erofs_kill_sb(struct super_block *sb)
> struct erofs_sb_info *sbi;
>
> WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
> - infoln("unmounting for %s", sb->s_id);
> + pr_info("unmounting for %s\n", sb->s_id);
>
> kill_block_super(sb);
>
> @@ -526,7 +528,7 @@ static int __init erofs_module_init(void)
> int err;
>
> erofs_check_ondisk_layout_definitions();
> - infoln("initializing erofs " EROFS_VERSION);
> + pr_info("initializing erofs " EROFS_VERSION "\n");
>
> err = erofs_init_inode_cache();
> if (err)
> @@ -544,7 +546,7 @@ static int __init erofs_module_init(void)
> if (err)
> goto fs_err;
>
> - infoln("successfully to initialize erofs");
> + pr_info("successfully initialized erofs\n");
> return 0;
>
> fs_err:
> @@ -563,7 +565,7 @@ static void __exit erofs_module_exit(void)
> z_erofs_exit_zip_subsystem();
> erofs_exit_shrinker();
> erofs_exit_inode_cache();
> - infoln("successfully finalize erofs");
> + pr_info("successfully finalized erofs\n");
> }
>
> /* get filesystem statistics */
> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
> index 289c7850ec96..e774a8c1bfae 100644
> --- a/drivers/staging/erofs/xattr.c
> +++ b/drivers/staging/erofs/xattr.c
> @@ -69,8 +69,8 @@ static int init_inode_xattrs(struct inode *inode)
> * undefined right now (maybe use later with some new sb feature).
> */
> if (vi->xattr_isize == sizeof(struct erofs_xattr_ibody_header)) {
> - errln("xattr_isize %d of nid %llu is not supported yet",
> - vi->xattr_isize, vi->nid);
> + pr_err("xattr_isize %d of nid %llu is not supported yet\n",
> + vi->xattr_isize, vi->nid);
> ret = -ENOTSUPP;
> goto out_unlock;
> } else if (vi->xattr_isize < sizeof(struct erofs_xattr_ibody_header)) {
> diff --git a/drivers/staging/erofs/zdata.c b/drivers/staging/erofs/zdata.c
> index 2d7aaf98f7de..17daf286747e 100644
> --- a/drivers/staging/erofs/zdata.c
> +++ b/drivers/staging/erofs/zdata.c
> @@ -585,7 +585,7 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
> }
>
> /* go ahead the next map_blocks */
> - debugln("%s: [out-of-range] pos %llu", __func__, offset + cur);
> + pr_debug("%s: [out-of-range] pos %llu\n", __func__, offset + cur);
>
> if (z_erofs_collector_end(clt))
> fe->backmost = false;
> @@ -665,8 +665,8 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
> out:
> z_erofs_onlinepage_endio(page);
>
> - debugln("%s, finish page: %pK spiltted: %u map->m_llen %llu",
> - __func__, page, spiltted, map->m_llen);
> + pr_debug("%s: finish page: %pK spiltted: %u map->m_llen %llu\n",
> + __func__, page, spiltted, map->m_llen);
> return err;
>
> /* if some error occurred while processing this page */
> @@ -1308,7 +1308,7 @@ static int z_erofs_vle_normalaccess_readpage(struct file *file,
> (void)z_erofs_collector_end(&f.clt);
>
> if (err) {
> - errln("%s, failed to read, err [%d]", __func__, err);
> + pr_err("%s: failed to read, err [%d]\n", __func__, err);
> goto out;
> }
>
> @@ -1380,8 +1380,8 @@ static int z_erofs_vle_normalaccess_readpages(struct file *filp,
> if (err) {
> struct erofs_vnode *vi = EROFS_V(inode);
>
> - errln("%s, readahead error at page %lu of nid %llu",
> - __func__, page->index, vi->nid);
> + pr_err("%s: readahead error at page %lu of nid %llu\n",
> + __func__, page->index, vi->nid);
> }
> put_page(page);
> }
> diff --git a/drivers/staging/erofs/zdata.h b/drivers/staging/erofs/zdata.h
> index e11fe1959ca2..e96e8ee270d2 100644
> --- a/drivers/staging/erofs/zdata.h
> +++ b/drivers/staging/erofs/zdata.h
> @@ -184,7 +184,7 @@ static inline void z_erofs_onlinepage_endio(struct page *page)
> SetPageUptodate(page);
> unlock_page(page);
> }
> - debugln("%s, page %p value %x", __func__, page, atomic_read(u.o));
> + pr_debug("%s: page %p value %x\n", __func__, page, atomic_read(u.o));
> }
>
> #define Z_EROFS_VMAP_ONSTACK_PAGES \
> diff --git a/drivers/staging/erofs/zmap.c b/drivers/staging/erofs/zmap.c
> index aeed5c674d9e..b2adf531379a 100644
> --- a/drivers/staging/erofs/zmap.c
> +++ b/drivers/staging/erofs/zmap.c
> @@ -66,8 +66,8 @@ static int fill_inode_lazy(struct inode *inode)
> vi->z_algorithmtype[1] = h->h_algorithmtype >> 4;
>
> if (vi->z_algorithmtype[0] >= Z_EROFS_COMPRESSION_MAX) {
> - errln("unknown compression format %u for nid %llu, please upgrade kernel",
> - vi->z_algorithmtype[0], vi->nid);
> + pr_err("unknown compression format %u for nid %llu, please upgrade kernel\n",
> + vi->z_algorithmtype[0], vi->nid);
> err = -ENOTSUPP;
> goto unmap_done;
> }
> @@ -77,8 +77,8 @@ static int fill_inode_lazy(struct inode *inode)
> ((h->h_clusterbits >> 3) & 3);
>
> if (vi->z_physical_clusterbits[0] != LOG_BLOCK_SIZE) {
> - errln("unsupported physical clusterbits %u for nid %llu, please upgrade kernel",
> - vi->z_physical_clusterbits[0], vi->nid);
> + pr_err("unsupported physical clusterbits %u for nid %llu, please upgrade kernel\n",
> + vi->z_physical_clusterbits[0], vi->nid);
> err = -ENOTSUPP;
> goto unmap_done;
> }
> @@ -358,8 +358,8 @@ static int vle_extent_lookback(struct z_erofs_maprecorder *m,
> map->m_la = (lcn << lclusterbits) | m->clusterofs;
> break;
> default:
> - errln("unknown type %u at lcn %lu of nid %llu",
> - m->type, lcn, vi->nid);
> + pr_err("unknown type %u at lcn %lu of nid %llu\n",
> + m->type, lcn, vi->nid);
> DBG_BUGON(1);
> return -EIO;
> }
> @@ -417,8 +417,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
> }
> /* m.lcn should be >= 1 if endoff < m.clusterofs */
> if (unlikely(!m.lcn)) {
> - errln("invalid logical cluster 0 at nid %llu",
> - vi->nid);
> + pr_err("invalid logical cluster 0 at nid %llu\n",
> + vi->nid);
> err = -EIO;
> goto unmap_out;
> }
> @@ -433,8 +433,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
> goto unmap_out;
> break;
> default:
> - errln("unknown type %u at offset %llu of nid %llu",
> - m.type, ofs, vi->nid);
> + pr_err("unknown type %u at offset %llu of nid %llu\n",
> + m.type, ofs, vi->nid);
> err = -EIO;
> goto unmap_out;
> }
> @@ -449,9 +449,9 @@ int z_erofs_map_blocks_iter(struct inode *inode,
> kunmap_atomic(m.kaddr);
>
> out:
> - debugln("%s, m_la %llu m_pa %llu m_llen %llu m_plen %llu m_flags 0%o",
> - __func__, map->m_la, map->m_pa,
> - map->m_llen, map->m_plen, map->m_flags);
> + pr_debug("%s: m_la %llu m_pa %llu m_llen %llu m_plen %llu m_flags 0%o\n",
> + __func__, map->m_la, map->m_pa,
> + map->m_llen, map->m_plen, map->m_flags);
>
> trace_z_erofs_map_blocks_iter_exit(inode, map, flags, err);
>
>
>
On Mon, 2019-08-19 at 13:52 +0800, Gao Xiang wrote:
> Hi Joe,
Hello.
> On Sun, Aug 18, 2019 at 10:28:41PM -0700, Joe Perches wrote:
> > Rename errln, infoln, and debugln to the typical pr_<level> uses
> > to the typical kernel styles of pr_<level>
>
> How about using erofs_err / ... to instead that?
<shrug> I've no opinion.
It seems most fs/*/* filesystems actually do use pr_<level>
sed works well if you want that.
> - I can hardly see directly use pr_<level> for those filesystems in fs/...
just fyi:
There was this one existing pr_<level> use in erofs
drivers/staging/erofs/data.c:366: pr_err("%s, readahead error at page %lu of nid %llu\n",
drivers/staging/erofs/data.c-367- __func__, page->index,
drivers/staging/erofs/data.c-368- EROFS_V(mapping->host)->nid);
Hi Joe,
On Sun, Aug 18, 2019 at 10:47:17PM -0700, Joe Perches wrote:
> On Mon, 2019-08-19 at 13:52 +0800, Gao Xiang wrote:
> > Hi Joe,
>
> Hello.
>
> > On Sun, Aug 18, 2019 at 10:28:41PM -0700, Joe Perches wrote:
> > > Rename errln, infoln, and debugln to the typical pr_<level> uses
> > > to the typical kernel styles of pr_<level>
> >
> > How about using erofs_err / ... to instead that?
>
> <shrug> I've no opinion.
> It seems most fs/*/* filesystems actually do use pr_<level>
> sed works well if you want that.
Sorry, I mainly refer to ext4, ext2, xfs and f2fs...
I didn't notice the other filesystems, you are right.
Okay, I have no opinion as well (maybe we could turn back later to
introduce sb parameter...)
>
> > - I can hardly see directly use pr_<level> for those filesystems in fs/...
>
> just fyi:
>
> There was this one existing pr_<level> use in erofs
That is just because the following piece of code was taken from fs/mpage.c,
I tend to replace it with iomap in the later version after tail-end packing inline is done.
Thanks,
Gao Xiang
>
> drivers/staging/erofs/data.c:366: pr_err("%s, readahead error at page %lu of nid %llu\n",
> drivers/staging/erofs/data.c-367- __func__, page->index,
> drivers/staging/erofs/data.c-368- EROFS_V(mapping->host)->nid);
>
>
>
----- Ursprüngliche Mail -----
> I have made a simple fuzzer to inject messy in inode metadata,
> dir data, compressed indexes and super block,
> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
>
> I am testing with some given dirs and the following script.
> Does it look reasonable?
I think that's a very good start. :-)
Thanks,
//richard
----- Ursprüngliche Mail -----
> On Sun, Aug 18, 2019 at 10:29:38AM -0700, Eric Biggers wrote:
>> Not sure what you're even disagreeing with, as I *do* expect new filesystems to
>> be held to a high standard, and to be written with the assumption that the
>> on-disk data may be corrupted or malicious. We just can't expect the bar to be
>> so high (e.g. no bugs) that it's never been attained by *any* filesystem even
>> after years/decades of active development. If the developers were careful, the
>> code generally looks robust, and they are willing to address such bugs as they
>> are found, realistically that's as good as we can expect to get...
>
> Well, the impression I got from Richards quick look and the reply to it is
> that there is very little attempt to validate the ondisk data structure
> and there is absolutely no priority to do so. Which is very different
> from there is a bug or two here and there.
On the plus side, everything I reported got fixed within hours.
Thanks,
//richard
Hi Richard,
On Mon, Aug 19, 2019 at 09:35:43AM +0200, Richard Weinberger wrote:
> ----- Urspr?ngliche Mail -----
> > I have made a simple fuzzer to inject messy in inode metadata,
> > dir data, compressed indexes and super block,
> > https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
> >
> > I am testing with some given dirs and the following script.
> > Does it look reasonable?
>
> I think that's a very good start. :-)
I have been testing with this tools for hours, it seems strong
against corrupted images without compression.
I'm now struggling with corrupted images with compression,
hopefully most of them can be fixed trivially... I will send
the bunch of fixes later... Let me dig into it more...
Thanks for your reply :-)
Thanks,
Gao Xiang
>
> Thanks,
> //richard
Hi all,
I have fuzzed EROFS for about a day and observed the following
issues due to corrupted compression images by my first fuzzer
(It seems ok for uncompressed images for now). Now it can survive
for 10+ minutes on my PC (Let me send out what I'm done and
I will dig it more deeply...)
All the fixes are trivial.
Note that those have dependency on EFSCORRUPTED, so for-next
is needed and I will manually backport them by hand due to
many cleanup patches...
Thanks,
Gao Xiang
Gao Xiang (6):
staging: erofs: some compressed cluster should be submitted for
corrupted images
staging: erofs: cannot set EROFS_V_Z_INITED_BIT if fill_inode_lazy
fails
staging: erofs: add two missing erofs_workgroup_put for corrupted
images
staging: erofs: avoid loop in submit chains
staging: erofs: detect potential multiref due to corrupted images
staging: erofs: avoid endless loop of invalid lookback distance 0
drivers/staging/erofs/zdata.c | 46 ++++++++++++++++++++++++++---------
drivers/staging/erofs/zmap.c | 9 +++++--
2 files changed, 42 insertions(+), 13 deletions(-)
--
2.17.1
As reported by erofs-utils fuzzer, unsupported compressed
clustersize will make fill_inode_lazy fail, for such case
we cannot set EROFS_V_Z_INITED_BIT since we need return
failure for each z_erofs_map_blocks_iter().
Fixes: 152a333a5895 ("staging: erofs: add compacted compression indexes support")
Cc: <[email protected]> # 5.3+
Signed-off-by: Gao Xiang <[email protected]>
---
drivers/staging/erofs/zmap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/erofs/zmap.c b/drivers/staging/erofs/zmap.c
index b61b9b5950ac..7408e86823a4 100644
--- a/drivers/staging/erofs/zmap.c
+++ b/drivers/staging/erofs/zmap.c
@@ -85,12 +85,11 @@ static int fill_inode_lazy(struct inode *inode)
vi->z_physical_clusterbits[1] = vi->z_logical_clusterbits +
((h->h_clusterbits >> 5) & 7);
+ set_bit(EROFS_V_Z_INITED_BIT, &vi->flags);
unmap_done:
kunmap_atomic(kaddr);
unlock_page(page);
put_page(page);
-
- set_bit(EROFS_V_Z_INITED_BIT, &vi->flags);
out_unlock:
clear_and_wake_up_bit(EROFS_V_BL_Z_BIT, &vi->flags);
return err;
--
2.17.1
On 2019-8-19 18:34, Gao Xiang wrote:
> As reported by erofs-utils fuzzer, unsupported compressed
> clustersize will make fill_inode_lazy fail, for such case
> we cannot set EROFS_V_Z_INITED_BIT since we need return
> failure for each z_erofs_map_blocks_iter().
>
> Fixes: 152a333a5895 ("staging: erofs: add compacted compression indexes support")
> Cc: <[email protected]> # 5.3+
> Signed-off-by: Gao Xiang <[email protected]>
Reviewed-by: Chao Yu <[email protected]>
Thanks,
On Mon, Aug 19, 2019 at 04:14:11AM +0800, Gao Xiang wrote:
> Hi all,
>
> On Mon, Aug 19, 2019 at 02:16:55AM +0800, Gao Xiang wrote:
> > Hi Hch,
> >
> > On Sun, Aug 18, 2019 at 10:47:02AM -0700, Christoph Hellwig wrote:
> > > On Sun, Aug 18, 2019 at 10:29:38AM -0700, Eric Biggers wrote:
> > > > Not sure what you're even disagreeing with, as I *do* expect new filesystems to
> > > > be held to a high standard, and to be written with the assumption that the
> > > > on-disk data may be corrupted or malicious. We just can't expect the bar to be
> > > > so high (e.g. no bugs) that it's never been attained by *any* filesystem even
> > > > after years/decades of active development. If the developers were careful, the
> > > > code generally looks robust, and they are willing to address such bugs as they
> > > > are found, realistically that's as good as we can expect to get...
> > >
> > > Well, the impression I got from Richards quick look and the reply to it is
> > > that there is very little attempt to validate the ondisk data structure
> > > and there is absolutely no priority to do so. Which is very different
> > > from there is a bug or two here and there.
> >
> > As my second reply to Richard, I didn't fuzz all the on-disk fields for EROFS.
> > and as my reply to Richard / Greg, current EROFS is used on the top of dm-verity.
> >
> > I cannot say how well EROFS will be performed on malformed images (and you can
> > also find the bug richard pointed out is a miswritten break->continue by myself).
> >
> > I posted the upstream EROFS post on July 4, 2019 and a month and a half later,
> > no one can tell me (yes, thanks for kind people reply me about their suggestion)
> > what we should do next (you can see these emails, I sent many times) to meet
> > the minimal upstream requirements and rare people can even dip into my code.
> >
> > That is all I want to say. I will work on autofuzz these days, and I want to
> > know how to meet your requirements on this (you can tell us your standard,
> > how well should we do).
> >
> > OK, you don't reply to my post once, I have no idea how to get your first reply.
>
> I have made a simple fuzzer to inject messy in inode metadata,
> dir data, compressed indexes and super block,
> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
>
> I am testing with some given dirs and the following script.
> Does it look reasonable?
>
> # !/bin/bash
>
> mkdir -p mntdir
>
> for ((i=0; i<1000; ++i)); do
> mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
mkfs fuzzes the image? Er....
Over in XFS land we have an xfs debugging tool (xfs_db) that knows how
to dump (and write!) most every field of every metadata type. This
makes it fairly easy to write systematic level 0 fuzzing tests that
check how well the filesystem reacts to garbage data (zeroing,
randomizing, oneing, adding and subtracting small integers) in a field.
(It also knows how to trash entire blocks.)
You might want to write such a debugging tool for erofs so that you can
take apart crashed images to get a better idea of what went wrong, and
to write easy fuzzing tests.
--D
> umount mntdir
> mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
> for j in `find mntdir -type f`; do
> md5sum $j > /dev/null
> done
> done
>
> Thanks,
> Gao Xiang
>
> >
> > Thanks,
> > Gao Xiang
> >
Hi Darrick,
On Mon, Aug 19, 2019 at 09:09:23AM -0700, Darrick J. Wong wrote:
> On Mon, Aug 19, 2019 at 04:14:11AM +0800, Gao Xiang wrote:
> > Hi all,
> >
> > On Mon, Aug 19, 2019 at 02:16:55AM +0800, Gao Xiang wrote:
> > > Hi Hch,
> > >
> > > On Sun, Aug 18, 2019 at 10:47:02AM -0700, Christoph Hellwig wrote:
> > > > On Sun, Aug 18, 2019 at 10:29:38AM -0700, Eric Biggers wrote:
> > > > > Not sure what you're even disagreeing with, as I *do* expect new filesystems to
> > > > > be held to a high standard, and to be written with the assumption that the
> > > > > on-disk data may be corrupted or malicious. We just can't expect the bar to be
> > > > > so high (e.g. no bugs) that it's never been attained by *any* filesystem even
> > > > > after years/decades of active development. If the developers were careful, the
> > > > > code generally looks robust, and they are willing to address such bugs as they
> > > > > are found, realistically that's as good as we can expect to get...
> > > >
> > > > Well, the impression I got from Richards quick look and the reply to it is
> > > > that there is very little attempt to validate the ondisk data structure
> > > > and there is absolutely no priority to do so. Which is very different
> > > > from there is a bug or two here and there.
> > >
> > > As my second reply to Richard, I didn't fuzz all the on-disk fields for EROFS.
> > > and as my reply to Richard / Greg, current EROFS is used on the top of dm-verity.
> > >
> > > I cannot say how well EROFS will be performed on malformed images (and you can
> > > also find the bug richard pointed out is a miswritten break->continue by myself).
> > >
> > > I posted the upstream EROFS post on July 4, 2019 and a month and a half later,
> > > no one can tell me (yes, thanks for kind people reply me about their suggestion)
> > > what we should do next (you can see these emails, I sent many times) to meet
> > > the minimal upstream requirements and rare people can even dip into my code.
> > >
> > > That is all I want to say. I will work on autofuzz these days, and I want to
> > > know how to meet your requirements on this (you can tell us your standard,
> > > how well should we do).
> > >
> > > OK, you don't reply to my post once, I have no idea how to get your first reply.
> >
> > I have made a simple fuzzer to inject messy in inode metadata,
> > dir data, compressed indexes and super block,
> > https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
> >
> > I am testing with some given dirs and the following script.
> > Does it look reasonable?
> >
> > # !/bin/bash
> >
> > mkdir -p mntdir
> >
> > for ((i=0; i<1000; ++i)); do
> > mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
>
> mkfs fuzzes the image? Er....
Thanks for your reply.
First, This is just the first step of erofs fuzzer I wrote yesterday night...
>
> Over in XFS land we have an xfs debugging tool (xfs_db) that knows how
> to dump (and write!) most every field of every metadata type. This
> makes it fairly easy to write systematic level 0 fuzzing tests that
> check how well the filesystem reacts to garbage data (zeroing,
> randomizing, oneing, adding and subtracting small integers) in a field.
> (It also knows how to trash entire blocks.)
Actually, compared with XFS, EROFS has rather simple on-disk format.
What we inject one time is quite deterministic.
The first step just purposely writes some random fuzzed data to
the base inode metadata, compressed indexes, or dir data field
(one round one field) to make it validity and coverability.
>
> You might want to write such a debugging tool for erofs so that you can
> take apart crashed images to get a better idea of what went wrong, and
> to write easy fuzzing tests.
Yes, we will do such a debugging tool of course. Actually Li Guifu is now
developping a erofs-fuse to support old linux versions or other OSes for
archiveing only use, we will base on that code to develop a better fuzzer
tool as well.
Thanks,
Gao Xiang
>
> --D
>
> > umount mntdir
> > mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
> > for j in `find mntdir -type f`; do
> > md5sum $j > /dev/null
> > done
> > done
> >
> > Thanks,
> > Gao Xiang
> >
> > >
> > > Thanks,
> > > Gao Xiang
> > >
[...]
>>> I have made a simple fuzzer to inject messy in inode metadata,
>>> dir data, compressed indexes and super block,
>>> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
>>>
>>> I am testing with some given dirs and the following script.
>>> Does it look reasonable?
>>>
>>> # !/bin/bash
>>>
>>> mkdir -p mntdir
>>>
>>> for ((i=0; i<1000; ++i)); do
>>> mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
>>
>> mkfs fuzzes the image? Er....
>
> Thanks for your reply.
>
> First, This is just the first step of erofs fuzzer I wrote yesterday night...
>
>>
>> Over in XFS land we have an xfs debugging tool (xfs_db) that knows how
>> to dump (and write!) most every field of every metadata type. This
>> makes it fairly easy to write systematic level 0 fuzzing tests that
>> check how well the filesystem reacts to garbage data (zeroing,
>> randomizing, oneing, adding and subtracting small integers) in a field.
>> (It also knows how to trash entire blocks.)
The same tool exists for btrfs, although lacks the write ability, but
that dump is more comprehensive and a great tool to learn the on-disk
format.
And for the fuzzing defending part, just a few kernel releases ago,
there is none for btrfs, and now we have a full static verification
layer to cover (almost) all on-disk data at read and write time.
(Along with enhanced runtime check)
We have covered from vague values inside tree blocks and invalid/missing
cross-ref find at runtime.
Currently the two layered check works pretty fine (well, sometimes too
good to detect older, improper behaved kernel).
- Tree blocks with vague data just get rejected by verification layer
So that all members should fit on-disk format, from alignment to
generation to inode mode.
The error will trigger a good enough (TM) error message for developer
to read, and if we have other copies, we retry other copies just as
we hit a bad copy.
- At runtime, we have much less to check
Only cross-ref related things can be wrong now. since everything
inside a single tree block has already be checked.
In fact, from my respect of view, such read time check should be there
from the very beginning.
It acts kinda of a on-disk format spec. (In fact, by implementing the
verification layer itself, it already exposes a lot of btrfs design
trade-offs)
Even for a fs as complex (buggy) as btrfs, we only take 1K lines to
implement the verification layer.
So I'd like to see every new mainlined fs to have such ability.
>
> Actually, compared with XFS, EROFS has rather simple on-disk format.
> What we inject one time is quite deterministic.
>
> The first step just purposely writes some random fuzzed data to
> the base inode metadata, compressed indexes, or dir data field
> (one round one field) to make it validity and coverability.
>
>>
>> You might want to write such a debugging tool for erofs so that you can
>> take apart crashed images to get a better idea of what went wrong, and
>> to write easy fuzzing tests.
>
> Yes, we will do such a debugging tool of course. Actually Li Guifu is now
> developping a erofs-fuse to support old linux versions or other OSes for
> archiveing only use, we will base on that code to develop a better fuzzer
> tool as well.
Personally speaking, debugging tool is way more important than a running
kernel module/fuse.
It's human trying to write the code, most of time is spent educating
code readers, thus debugging tool is way more important than dead cold code.
Thanks,
Qu
>
> Thanks,
> Gao Xiang
>
>>
>> --D
>>
>>> umount mntdir
>>> mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
>>> for j in `find mntdir -type f`; do
>>> md5sum $j > /dev/null
>>> done
>>> done
>>>
>>> Thanks,
>>> Gao Xiang
>>>
>>>>
>>>> Thanks,
>>>> Gao Xiang
>>>>
Hi Qu,
On Tue, Aug 20, 2019 at 08:55:32AM +0800, Qu Wenruo wrote:
> [...]
> >>> I have made a simple fuzzer to inject messy in inode metadata,
> >>> dir data, compressed indexes and super block,
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
> >>>
> >>> I am testing with some given dirs and the following script.
> >>> Does it look reasonable?
> >>>
> >>> # !/bin/bash
> >>>
> >>> mkdir -p mntdir
> >>>
> >>> for ((i=0; i<1000; ++i)); do
> >>> mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
> >>
> >> mkfs fuzzes the image? Er....
> >
> > Thanks for your reply.
> >
> > First, This is just the first step of erofs fuzzer I wrote yesterday night...
> >
> >>
> >> Over in XFS land we have an xfs debugging tool (xfs_db) that knows how
> >> to dump (and write!) most every field of every metadata type. This
> >> makes it fairly easy to write systematic level 0 fuzzing tests that
> >> check how well the filesystem reacts to garbage data (zeroing,
> >> randomizing, oneing, adding and subtracting small integers) in a field.
> >> (It also knows how to trash entire blocks.)
>
> The same tool exists for btrfs, although lacks the write ability, but
> that dump is more comprehensive and a great tool to learn the on-disk
> format.
>
>
> And for the fuzzing defending part, just a few kernel releases ago,
> there is none for btrfs, and now we have a full static verification
> layer to cover (almost) all on-disk data at read and write time.
> (Along with enhanced runtime check)
>
> We have covered from vague values inside tree blocks and invalid/missing
> cross-ref find at runtime.
>
> Currently the two layered check works pretty fine (well, sometimes too
> good to detect older, improper behaved kernel).
> - Tree blocks with vague data just get rejected by verification layer
> So that all members should fit on-disk format, from alignment to
> generation to inode mode.
>
> The error will trigger a good enough (TM) error message for developer
> to read, and if we have other copies, we retry other copies just as
> we hit a bad copy.
>
> - At runtime, we have much less to check
> Only cross-ref related things can be wrong now. since everything
> inside a single tree block has already be checked.
>
> In fact, from my respect of view, such read time check should be there
> from the very beginning.
> It acts kinda of a on-disk format spec. (In fact, by implementing the
> verification layer itself, it already exposes a lot of btrfs design
> trade-offs)
>
> Even for a fs as complex (buggy) as btrfs, we only take 1K lines to
> implement the verification layer.
> So I'd like to see every new mainlined fs to have such ability.
It's already on our schedule, but we have limited manpower. Rome was
not built in a day, as I mentioned eariler, we are doing our best.
In principle, all the new Linux features on-disk can build their
debugging tools, not only for file systems. You can hardly let your
newborn baby go to university immediately.
We're developping out of our interests for Linux community (our
high level bosses care nothing except for money, you know) and
we hope to better join in and contribute to Linux community, we need
more time to enrich our eco-system in our spare time.
All HUAWEI smartphone products will continue using this filesystem,
and its performance and stability is proven by our 10+ millions
products, and maintaining this filesystem is one of our paid jobs.
>
> >
> > Actually, compared with XFS, EROFS has rather simple on-disk format.
> > What we inject one time is quite deterministic.
> >
> > The first step just purposely writes some random fuzzed data to
> > the base inode metadata, compressed indexes, or dir data field
> > (one round one field) to make it validity and coverability.
> >
> >>
> >> You might want to write such a debugging tool for erofs so that you can
> >> take apart crashed images to get a better idea of what went wrong, and
> >> to write easy fuzzing tests.
> >
> > Yes, we will do such a debugging tool of course. Actually Li Guifu is now
> > developping a erofs-fuse to support old linux versions or other OSes for
> > archiveing only use, we will base on that code to develop a better fuzzer
> > tool as well.
>
> Personally speaking, debugging tool is way more important than a running
> kernel module/fuse.
> It's human trying to write the code, most of time is spent educating
> code readers, thus debugging tool is way more important than dead cold code.
Debugging tools and erofs-fuse share common code, that is to parse
the filesystem. That was the main point that I want to say.
Thanks,
Gao Xiang
>
> Thanks,
> Qu
> >
> > Thanks,
> > Gao Xiang
> >
> >>
> >> --D
> >>
> >>> umount mntdir
> >>> mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
> >>> for j in `find mntdir -type f`; do
> >>> md5sum $j > /dev/null
> >>> done
> >>> done
> >>>
> >>> Thanks,
> >>> Gao Xiang
> >>>
> >>>>
> >>>> Thanks,
> >>>> Gao Xiang
> >>>>
>
On 2019/8/20 8:55, Qu Wenruo wrote:
> [...]
>>>> I have made a simple fuzzer to inject messy in inode metadata,
>>>> dir data, compressed indexes and super block,
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
>>>>
>>>> I am testing with some given dirs and the following script.
>>>> Does it look reasonable?
>>>>
>>>> # !/bin/bash
>>>>
>>>> mkdir -p mntdir
>>>>
>>>> for ((i=0; i<1000; ++i)); do
>>>> mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
>>>
>>> mkfs fuzzes the image? Er....
>>
>> Thanks for your reply.
>>
>> First, This is just the first step of erofs fuzzer I wrote yesterday night...
>>
>>>
>>> Over in XFS land we have an xfs debugging tool (xfs_db) that knows how
>>> to dump (and write!) most every field of every metadata type. This
>>> makes it fairly easy to write systematic level 0 fuzzing tests that
>>> check how well the filesystem reacts to garbage data (zeroing,
>>> randomizing, oneing, adding and subtracting small integers) in a field.
>>> (It also knows how to trash entire blocks.)
>
> The same tool exists for btrfs, although lacks the write ability, but
> that dump is more comprehensive and a great tool to learn the on-disk
> format.
>
>
> And for the fuzzing defending part, just a few kernel releases ago,
> there is none for btrfs, and now we have a full static verification
> layer to cover (almost) all on-disk data at read and write time.
> (Along with enhanced runtime check)
>
> We have covered from vague values inside tree blocks and invalid/missing
> cross-ref find at runtime.
>
> Currently the two layered check works pretty fine (well, sometimes too
> good to detect older, improper behaved kernel).
> - Tree blocks with vague data just get rejected by verification layer
> So that all members should fit on-disk format, from alignment to
> generation to inode mode.
>
> The error will trigger a good enough (TM) error message for developer
> to read, and if we have other copies, we retry other copies just as
> we hit a bad copy.
>
> - At runtime, we have much less to check
> Only cross-ref related things can be wrong now. since everything
> inside a single tree block has already be checked.
>
> In fact, from my respect of view, such read time check should be there
> from the very beginning.
> It acts kinda of a on-disk format spec. (In fact, by implementing the
> verification layer itself, it already exposes a lot of btrfs design
> trade-offs)
>
> Even for a fs as complex (buggy) as btrfs, we only take 1K lines to
> implement the verification layer.
> So I'd like to see every new mainlined fs to have such ability.
Out of curiosity, it looks like every mainstream filesystem has its own
fuzz/injection tool in their tool-set, if it's really such a generic
requirement, why shouldn't there be a common tool to handle that, let specified
filesystem fill the tool's callback to seek a node/block and supported fields
can be fuzzed in inode. It can help to avoid redundant work whenever Linux
welcomes a new filesystem....
Thanks,
>
>>
>> Actually, compared with XFS, EROFS has rather simple on-disk format.
>> What we inject one time is quite deterministic.
>>
>> The first step just purposely writes some random fuzzed data to
>> the base inode metadata, compressed indexes, or dir data field
>> (one round one field) to make it validity and coverability.
>>
>>>
>>> You might want to write such a debugging tool for erofs so that you can
>>> take apart crashed images to get a better idea of what went wrong, and
>>> to write easy fuzzing tests.
>>
>> Yes, we will do such a debugging tool of course. Actually Li Guifu is now
>> developping a erofs-fuse to support old linux versions or other OSes for
>> archiveing only use, we will base on that code to develop a better fuzzer
>> tool as well.
>
> Personally speaking, debugging tool is way more important than a running
> kernel module/fuse.
> It's human trying to write the code, most of time is spent educating
> code readers, thus debugging tool is way more important than dead cold code.
>
> Thanks,
> Qu
>>
>> Thanks,
>> Gao Xiang
>>
>>>
>>> --D
>>>
>>>> umount mntdir
>>>> mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
>>>> for j in `find mntdir -type f`; do
>>>> md5sum $j > /dev/null
>>>> done
>>>> done
>>>>
>>>> Thanks,
>>>> Gao Xiang
>>>>
>>>>>
>>>>> Thanks,
>>>>> Gao Xiang
>>>>>
>
On 2019/8/20 上午10:24, Chao Yu wrote:
> On 2019/8/20 8:55, Qu Wenruo wrote:
>> [...]
>>>>> I have made a simple fuzzer to inject messy in inode metadata,
>>>>> dir data, compressed indexes and super block,
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
>>>>>
>>>>> I am testing with some given dirs and the following script.
>>>>> Does it look reasonable?
>>>>>
>>>>> # !/bin/bash
>>>>>
>>>>> mkdir -p mntdir
>>>>>
>>>>> for ((i=0; i<1000; ++i)); do
>>>>> mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
>>>>
>>>> mkfs fuzzes the image? Er....
>>>
>>> Thanks for your reply.
>>>
>>> First, This is just the first step of erofs fuzzer I wrote yesterday night...
>>>
>>>>
>>>> Over in XFS land we have an xfs debugging tool (xfs_db) that knows how
>>>> to dump (and write!) most every field of every metadata type. This
>>>> makes it fairly easy to write systematic level 0 fuzzing tests that
>>>> check how well the filesystem reacts to garbage data (zeroing,
>>>> randomizing, oneing, adding and subtracting small integers) in a field.
>>>> (It also knows how to trash entire blocks.)
>>
>> The same tool exists for btrfs, although lacks the write ability, but
>> that dump is more comprehensive and a great tool to learn the on-disk
>> format.
>>
>>
>> And for the fuzzing defending part, just a few kernel releases ago,
>> there is none for btrfs, and now we have a full static verification
>> layer to cover (almost) all on-disk data at read and write time.
>> (Along with enhanced runtime check)
>>
>> We have covered from vague values inside tree blocks and invalid/missing
>> cross-ref find at runtime.
>>
>> Currently the two layered check works pretty fine (well, sometimes too
>> good to detect older, improper behaved kernel).
>> - Tree blocks with vague data just get rejected by verification layer
>> So that all members should fit on-disk format, from alignment to
>> generation to inode mode.
>>
>> The error will trigger a good enough (TM) error message for developer
>> to read, and if we have other copies, we retry other copies just as
>> we hit a bad copy.
>>
>> - At runtime, we have much less to check
>> Only cross-ref related things can be wrong now. since everything
>> inside a single tree block has already be checked.
>>
>> In fact, from my respect of view, such read time check should be there
>> from the very beginning.
>> It acts kinda of a on-disk format spec. (In fact, by implementing the
>> verification layer itself, it already exposes a lot of btrfs design
>> trade-offs)
>>
>> Even for a fs as complex (buggy) as btrfs, we only take 1K lines to
>> implement the verification layer.
>> So I'd like to see every new mainlined fs to have such ability.
>
> Out of curiosity, it looks like every mainstream filesystem has its own
> fuzz/injection tool in their tool-set, if it's really such a generic
> requirement, why shouldn't there be a common tool to handle that, let specified
> filesystem fill the tool's callback to seek a node/block and supported fields
> can be fuzzed in inode.
It could be possible for XFS/EXT* to share the same infrastructure
without much hassle.
(If not considering external journal)
But for btrfs, it's like a regular fs on a super large dm-linear, which
further builds its chunks on different dm-raid1/dm-linear/dm-raid56.
So not sure if it's possible for btrfs, as it contains its logical
address layer bytenr (the most common one) along with per-chunk physical
mapping bytenr (in another tree).
It may depends on the granularity. But definitely a good idea to do so
in a generic way.
Currently we depend on super kind student developers/reporters on such
fuzzed images, and developers sometimes get inspired by real world
corruption (or his/her mood) to add some valid but hard-to-hit corner
case check.
Thanks,
Qu
> It can help to avoid redundant work whenever Linux
> welcomes a new filesystem....
>
> Thanks,
>
>>
>>>
>>> Actually, compared with XFS, EROFS has rather simple on-disk format.
>>> What we inject one time is quite deterministic.
>>>
>>> The first step just purposely writes some random fuzzed data to
>>> the base inode metadata, compressed indexes, or dir data field
>>> (one round one field) to make it validity and coverability.
>>>
>>>>
>>>> You might want to write such a debugging tool for erofs so that you can
>>>> take apart crashed images to get a better idea of what went wrong, and
>>>> to write easy fuzzing tests.
>>>
>>> Yes, we will do such a debugging tool of course. Actually Li Guifu is now
>>> developping a erofs-fuse to support old linux versions or other OSes for
>>> archiveing only use, we will base on that code to develop a better fuzzer
>>> tool as well.
>>
>> Personally speaking, debugging tool is way more important than a running
>> kernel module/fuse.
>> It's human trying to write the code, most of time is spent educating
>> code readers, thus debugging tool is way more important than dead cold code.
>>
>> Thanks,
>> Qu
>>>
>>> Thanks,
>>> Gao Xiang
>>>
>>>>
>>>> --D
>>>>
>>>>> umount mntdir
>>>>> mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
>>>>> for j in `find mntdir -type f`; do
>>>>> md5sum $j > /dev/null
>>>>> done
>>>>> done
>>>>>
>>>>> Thanks,
>>>>> Gao Xiang
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Gao Xiang
>>>>>>
>>
on 2019/8/20 at 8:55, Qu Wenruo wrote:
> [...]
>>>> I have made a simple fuzzer to inject messy in inode metadata,
>>>> dir data, compressed indexes and super block,
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
>>>>
>>>> I am testing with some given dirs and the following script.
>>>> Does it look reasonable?
>>>>
>>>> # !/bin/bash
>>>>
>>>> mkdir -p mntdir
>>>>
>>>> for ((i=0; i<1000; ++i)); do
>>>> mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
>>>
>>> mkfs fuzzes the image? Er....
>>
>> Thanks for your reply.
>>
>> First, This is just the first step of erofs fuzzer I wrote yesterday night...
>>
>>>
>>> Over in XFS land we have an xfs debugging tool (xfs_db) that knows how
>>> to dump (and write!) most every field of every metadata type. This
>>> makes it fairly easy to write systematic level 0 fuzzing tests that
>>> check how well the filesystem reacts to garbage data (zeroing,
>>> randomizing, oneing, adding and subtracting small integers) in a field.
>>> (It also knows how to trash entire blocks.)
>
> The same tool exists for btrfs, although lacks the write ability, but
> that dump is more comprehensive and a great tool to learn the on-disk
> format.
>
>
> And for the fuzzing defending part, just a few kernel releases ago,
> there is none for btrfs, and now we have a full static verification
> layer to cover (almost) all on-disk data at read and write time.
> (Along with enhanced runtime check)
>
> We have covered from vague values inside tree blocks and invalid/missing
> cross-ref find at runtime.
>
> Currently the two layered check works pretty fine (well, sometimes too
> good to detect older, improper behaved kernel).
> - Tree blocks with vague data just get rejected by verification layer
> So that all members should fit on-disk format, from alignment to
> generation to inode mode.
>
> The error will trigger a good enough (TM) error message for developer
> to read, and if we have other copies, we retry other copies just as
> we hit a bad copy.
>
> - At runtime, we have much less to check
> Only cross-ref related things can be wrong now. since everything
> inside a single tree block has already be checked.
>
> In fact, from my respect of view, such read time check should be there
> from the very beginning.
> It acts kinda of a on-disk format spec. (In fact, by implementing the
> verification layer itself, it already exposes a lot of btrfs design
> trade-offs)
>
> Even for a fs as complex (buggy) as btrfs, we only take 1K lines to
> implement the verification layer.
> So I'd like to see every new mainlined fs to have such ability.
It is a good idea. In fact, we already have a verification layer which was implemented
as a device mapper sub-module. I think it is enough for a read-only filesystem because
it is simple, flexible and independent(we can modify the filesystem layout without
verification module modification).
>>
>> Actually, compared with XFS, EROFS has rather simple on-disk format.
>> What we inject one time is quite deterministic.
>>
>> The first step just purposely writes some random fuzzed data to
>> the base inode metadata, compressed indexes, or dir data field
>> (one round one field) to make it validity and coverability.
>>
>>>
>>> You might want to write such a debugging tool for erofs so that you can
>>> take apart crashed images to get a better idea of what went wrong, and
>>> to write easy fuzzing tests.
>>
>> Yes, we will do such a debugging tool of course. Actually Li Guifu is now
>> developping a erofs-fuse to support old linux versions or other OSes for
>> archiveing only use, we will base on that code to develop a better fuzzer
>> tool as well.
>
> Personally speaking, debugging tool is way more important than a running
> kernel module/fuse.
> It's human trying to write the code, most of time is spent educating
> code readers, thus debugging tool is way more important than dead cold code.
Agree, Xiang and I have no time to developing this feature now, we are glad very much if you could help
us to do it ;)
Thanks
Miao
>
> Thanks,
> Qu
>>
>> Thanks,
>> Gao Xiang
>>
>>>
>>> --D
>>>
>>>> umount mntdir
>>>> mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
>>>> for j in `find mntdir -type f`; do
>>>> md5sum $j > /dev/null
>>>> done
>>>> done
>>>>
>>>> Thanks,
>>>> Gao Xiang
>>>>
>>>>>
>>>>> Thanks,
>>>>> Gao Xiang
>>>>>
>
On Tue, Aug 20, 2019 at 11:33:51AM +0800, Miao Xie wrote:
>
>
> on 2019/8/20 at 8:55, Qu Wenruo wrote:
> > [...]
> >>>> I have made a simple fuzzer to inject messy in inode metadata,
> >>>> dir data, compressed indexes and super block,
> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
> >>>>
> >>>> I am testing with some given dirs and the following script.
> >>>> Does it look reasonable?
> >>>>
> >>>> # !/bin/bash
> >>>>
> >>>> mkdir -p mntdir
> >>>>
> >>>> for ((i=0; i<1000; ++i)); do
> >>>> mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
> >>>
> >>> mkfs fuzzes the image? Er....
> >>
> >> Thanks for your reply.
> >>
> >> First, This is just the first step of erofs fuzzer I wrote yesterday night...
> >>
> >>>
> >>> Over in XFS land we have an xfs debugging tool (xfs_db) that knows how
> >>> to dump (and write!) most every field of every metadata type. This
> >>> makes it fairly easy to write systematic level 0 fuzzing tests that
> >>> check how well the filesystem reacts to garbage data (zeroing,
> >>> randomizing, oneing, adding and subtracting small integers) in a field.
> >>> (It also knows how to trash entire blocks.)
> >
> > The same tool exists for btrfs, although lacks the write ability, but
> > that dump is more comprehensive and a great tool to learn the on-disk
> > format.
> >
> >
> > And for the fuzzing defending part, just a few kernel releases ago,
> > there is none for btrfs, and now we have a full static verification
> > layer to cover (almost) all on-disk data at read and write time.
> > (Along with enhanced runtime check)
> >
> > We have covered from vague values inside tree blocks and invalid/missing
> > cross-ref find at runtime.
> >
> > Currently the two layered check works pretty fine (well, sometimes too
> > good to detect older, improper behaved kernel).
> > - Tree blocks with vague data just get rejected by verification layer
> > So that all members should fit on-disk format, from alignment to
> > generation to inode mode.
> >
> > The error will trigger a good enough (TM) error message for developer
> > to read, and if we have other copies, we retry other copies just as
> > we hit a bad copy.
> >
> > - At runtime, we have much less to check
> > Only cross-ref related things can be wrong now. since everything
> > inside a single tree block has already be checked.
> >
> > In fact, from my respect of view, such read time check should be there
> > from the very beginning.
> > It acts kinda of a on-disk format spec. (In fact, by implementing the
> > verification layer itself, it already exposes a lot of btrfs design
> > trade-offs)
> >
> > Even for a fs as complex (buggy) as btrfs, we only take 1K lines to
> > implement the verification layer.
> > So I'd like to see every new mainlined fs to have such ability.
>
> It is a good idea. In fact, we already have a verification layer which was implemented
> as a device mapper sub-module. I think it is enough for a read-only filesystem because
> it is simple, flexible and independent(we can modify the filesystem layout without
> verification module modification).
>
>
> >>
> >> Actually, compared with XFS, EROFS has rather simple on-disk format.
> >> What we inject one time is quite deterministic.
> >>
> >> The first step just purposely writes some random fuzzed data to
> >> the base inode metadata, compressed indexes, or dir data field
> >> (one round one field) to make it validity and coverability.
> >>
> >>>
> >>> You might want to write such a debugging tool for erofs so that you can
> >>> take apart crashed images to get a better idea of what went wrong, and
> >>> to write easy fuzzing tests.
> >>
> >> Yes, we will do such a debugging tool of course. Actually Li Guifu is now
> >> developping a erofs-fuse to support old linux versions or other OSes for
> >> archiveing only use, we will base on that code to develop a better fuzzer
> >> tool as well.
> >
> > Personally speaking, debugging tool is way more important than a running
> > kernel module/fuse.
> > It's human trying to write the code, most of time is spent educating
> > code readers, thus debugging tool is way more important than dead cold code.
>
> Agree, Xiang and I have no time to developing this feature now, we are glad very much if you could help
> us to do it ;)
I can speed all my spare time for this...
As I said before, All HUAWEI smartphone products will continue using
this filesystem, and maintaining this filesystem is one of our paid
jobs, but since our Android products is based on dm-verity + EROFS,
it's only on my personal time schedule (bosses care more about Android
and money) and I will do that in my spare time of course.
Thanks,
Gao Xiang
>
> Thanks
> Miao
>
> >
> > Thanks,
> > Qu
> >>
> >> Thanks,
> >> Gao Xiang
> >>
> >>>
> >>> --D
> >>>
> >>>> umount mntdir
> >>>> mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
> >>>> for j in `find mntdir -type f`; do
> >>>> md5sum $j > /dev/null
> >>>> done
> >>>> done
> >>>>
> >>>> Thanks,
> >>>> Gao Xiang
> >>>>
> >>>>>
> >>>>> Thanks,
> >>>>> Gao Xiang
> >>>>>
> >
[...]
>> The same tool exists for btrfs, although lacks the write ability, but
>> that dump is more comprehensive and a great tool to learn the on-disk
>> format.
>>
>>
>> And for the fuzzing defending part, just a few kernel releases ago,
>> there is none for btrfs, and now we have a full static verification
>> layer to cover (almost) all on-disk data at read and write time.
>> (Along with enhanced runtime check)
>>
>> We have covered from vague values inside tree blocks and invalid/missing
>> cross-ref find at runtime.
>>
>> Currently the two layered check works pretty fine (well, sometimes too
>> good to detect older, improper behaved kernel).
>> - Tree blocks with vague data just get rejected by verification layer
>> So that all members should fit on-disk format, from alignment to
>> generation to inode mode.
>>
>> The error will trigger a good enough (TM) error message for developer
>> to read, and if we have other copies, we retry other copies just as
>> we hit a bad copy.
>>
>> - At runtime, we have much less to check
>> Only cross-ref related things can be wrong now. since everything
>> inside a single tree block has already be checked.
>>
>> In fact, from my respect of view, such read time check should be there
>> from the very beginning.
>> It acts kinda of a on-disk format spec. (In fact, by implementing the
>> verification layer itself, it already exposes a lot of btrfs design
>> trade-offs)
>>
>> Even for a fs as complex (buggy) as btrfs, we only take 1K lines to
>> implement the verification layer.
>> So I'd like to see every new mainlined fs to have such ability.
>
> It is a good idea. In fact, we already have a verification layer which was implemented
> as a device mapper sub-module. I think it is enough for a read-only filesystem because
> it is simple, flexible and independent(we can modify the filesystem layout without
> verification module modification).
If you're talking about dm-verity, then IMHO they are with completely
different objective.
For dm-verity it's more like authentication. Without proper key
(authentication), no one can modify the data without being caught.
That's why I hate such thing, it's not open at all, *as bad as locked
bootloader*.
While the tree-checker (the layer in btrfs) is more like a sensitive and
sometimes overreacting detector, find anything wrong, then reject the
offending tree block.
The original objective of tree-checker is to free coder from defensive
coding, providing a centralized verification service, thus we don't need
to verify tree blocks randomly using ugly BUG_ON()s.
(But unfortunately, a lot of BUG_ON()s are still kept as is, as it takes
more time to persuade reviewers that those BUG_ON()s are impossible to
hit anymore)
Tree-checker can not only detect suspicious metadata (either caused by
mem bit flip or poorly crafted image), but also bad *kernel* behavior or
even runtime bitflip. (Well, it only works for RW fs, so not really
helpful for a RO fs).
And performance is another point.
That tree-checker in btrfs is as fast/slow as CRC32.
Not sure how it would be for dm-verity, but I guess it's slower than
CRC32 if using any strong hash.
Anyway, for a RO fs, if it's relying on dm-verify then that's OK for
real-world usage.
But as a standalone fs, even it's RO, a verification layer would be a
great plus.
At least when new student developers try fuzzed images on the fs, it
would be a good surprise other than tons of new bug reports.
>
>
>>>
[...]
>>>
>>> Yes, we will do such a debugging tool of course. Actually Li Guifu is now
>>> developping a erofs-fuse to support old linux versions or other OSes for
>>> archiveing only use, we will base on that code to develop a better fuzzer
>>> tool as well.
>>
>> Personally speaking, debugging tool is way more important than a running
>> kernel module/fuse.
>> It's human trying to write the code, most of time is spent educating
>> code readers, thus debugging tool is way more important than dead cold code.
>
> Agree, Xiang and I have no time to developing this feature now, we are glad very much if you could help
> us to do it ;)
In fact, since the fs is a RO fs, it could be pretty good educational
example for any fs newbies. Thus a debug tool which can show the full
metadata of the fs can really be helpful.
In fact, btrfs-debug-tree (now "btrfs ins dump-tree") leads my way to
btrfs, and still one of my favourite tool to debug.
Thanks,
Qu
Hi Qu,
On Tue, Aug 20, 2019 at 02:04:46PM +0800, Qu Wenruo wrote:
> [...]
>
> And performance is another point.
> That tree-checker in btrfs is as fast/slow as CRC32.
> Not sure how it would be for dm-verity, but I guess it's slower than
> CRC32 if using any strong hash.
Just a word, dm-verity can do simple CRC32 without hash
tree in priciple as well. It is only a time problem.
>
> Anyway, for a RO fs, if it's relying on dm-verify then that's OK for
> real-world usage.
> But as a standalone fs, even it's RO, a verification layer would be a
> great plus.
Yes, again, it's on my schedule at least.
>
> At least when new student developers try fuzzed images on the fs, it
> would be a good surprise other than tons of new bug reports.
>
> >
> >
> >>>
> [...]
> >>>
> >>> Yes, we will do such a debugging tool of course. Actually Li Guifu is now
> >>> developping a erofs-fuse to support old linux versions or other OSes for
> >>> archiveing only use, we will base on that code to develop a better fuzzer
> >>> tool as well.
> >>
> >> Personally speaking, debugging tool is way more important than a running
> >> kernel module/fuse.
> >> It's human trying to write the code, most of time is spent educating
> >> code readers, thus debugging tool is way more important than dead cold code.
> >
> > Agree, Xiang and I have no time to developing this feature now, we are glad very much if you could help
> > us to do it ;)
>
> In fact, since the fs is a RO fs, it could be pretty good educational
> example for any fs newbies. Thus a debug tool which can show the full
> metadata of the fs can really be helpful.
Yes, I think EROFS will be helpful for all fs newbies to understand
the basic fs concepts, and I have received contributions from some
new kernel developers. And again, I will do in my spare time.
I'm dedicated to playing with EROFS in my spare time (rather than
only at work to maintain for our products), so I will do and don't
worry about that.
And I agree with Miao Xie on one word, you can join us as well for
educational reason or whatever if you have some extra time... :)
Thanks,
Gao Xiang
>
> In fact, btrfs-debug-tree (now "btrfs ins dump-tree") leads my way to
> btrfs, and still one of my favourite tool to debug.
>
> Thanks,
> Qu
>
>
> _______________________________________________
> devel mailing list
> [email protected]
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On 2019/8/20 10:38, Qu Wenruo wrote:
>
>
> On 2019/8/20 上午10:24, Chao Yu wrote:
>> On 2019/8/20 8:55, Qu Wenruo wrote:
>>> [...]
>>>>>> I have made a simple fuzzer to inject messy in inode metadata,
>>>>>> dir data, compressed indexes and super block,
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
>>>>>>
>>>>>> I am testing with some given dirs and the following script.
>>>>>> Does it look reasonable?
>>>>>>
>>>>>> # !/bin/bash
>>>>>>
>>>>>> mkdir -p mntdir
>>>>>>
>>>>>> for ((i=0; i<1000; ++i)); do
>>>>>> mkfs/mkfs.erofs -F$i testdir_fsl.fuzz.img testdir_fsl > /dev/null 2>&1
>>>>>
>>>>> mkfs fuzzes the image? Er....
>>>>
>>>> Thanks for your reply.
>>>>
>>>> First, This is just the first step of erofs fuzzer I wrote yesterday night...
>>>>
>>>>>
>>>>> Over in XFS land we have an xfs debugging tool (xfs_db) that knows how
>>>>> to dump (and write!) most every field of every metadata type. This
>>>>> makes it fairly easy to write systematic level 0 fuzzing tests that
>>>>> check how well the filesystem reacts to garbage data (zeroing,
>>>>> randomizing, oneing, adding and subtracting small integers) in a field.
>>>>> (It also knows how to trash entire blocks.)
>>>
>>> The same tool exists for btrfs, although lacks the write ability, but
>>> that dump is more comprehensive and a great tool to learn the on-disk
>>> format.
>>>
>>>
>>> And for the fuzzing defending part, just a few kernel releases ago,
>>> there is none for btrfs, and now we have a full static verification
>>> layer to cover (almost) all on-disk data at read and write time.
>>> (Along with enhanced runtime check)
>>>
>>> We have covered from vague values inside tree blocks and invalid/missing
>>> cross-ref find at runtime.
>>>
>>> Currently the two layered check works pretty fine (well, sometimes too
>>> good to detect older, improper behaved kernel).
>>> - Tree blocks with vague data just get rejected by verification layer
>>> So that all members should fit on-disk format, from alignment to
>>> generation to inode mode.
>>>
>>> The error will trigger a good enough (TM) error message for developer
>>> to read, and if we have other copies, we retry other copies just as
>>> we hit a bad copy.
>>>
>>> - At runtime, we have much less to check
>>> Only cross-ref related things can be wrong now. since everything
>>> inside a single tree block has already be checked.
>>>
>>> In fact, from my respect of view, such read time check should be there
>>> from the very beginning.
>>> It acts kinda of a on-disk format spec. (In fact, by implementing the
>>> verification layer itself, it already exposes a lot of btrfs design
>>> trade-offs)
>>>
>>> Even for a fs as complex (buggy) as btrfs, we only take 1K lines to
>>> implement the verification layer.
>>> So I'd like to see every new mainlined fs to have such ability.
>>
>> Out of curiosity, it looks like every mainstream filesystem has its own
>> fuzz/injection tool in their tool-set, if it's really such a generic
>> requirement, why shouldn't there be a common tool to handle that, let specified
>> filesystem fill the tool's callback to seek a node/block and supported fields
>> can be fuzzed in inode.
>
> It could be possible for XFS/EXT* to share the same infrastructure
> without much hassle.
> (If not considering external journal)
>
> But for btrfs, it's like a regular fs on a super large dm-linear, which
> further builds its chunks on different dm-raid1/dm-linear/dm-raid56.
>
> So not sure if it's possible for btrfs, as it contains its logical
> address layer bytenr (the most common one) along with per-chunk physical
> mapping bytenr (in another tree).
Yeah, it looks like we need searching more levels mapping to find the final
physical block address of inode/node/data in btrfs.
IMO, in a little lazy way, we can reform and reuse existed function in
btrfs-progs which can find the mapping info of inode/node/data according to
specified ino or ino+pg_no.
>
> It may depends on the granularity. But definitely a good idea to do so
> in a generic way.
> Currently we depend on super kind student developers/reporters on such
Yup, I just guess Wen Xu may be interested in working on a generic way to fuzz
filesystem, as I know they dig deep in filesystem code when doing fuzz. BTW,
which impresses me is, constructing checkpoint by injecting one byte, and then
write a correct recalculated checksum value on that checkpoint, making that
checkpoint looks valid...
Thanks,
> fuzzed images, and developers sometimes get inspired by real world
> corruption (or his/her mood) to add some valid but hard-to-hit corner
> case check.
>
> Thanks,
> Qu
>
>> It can help to avoid redundant work whenever Linux
>> welcomes a new filesystem....
>>
>> Thanks,
>>
>>>
>>>>
>>>> Actually, compared with XFS, EROFS has rather simple on-disk format.
>>>> What we inject one time is quite deterministic.
>>>>
>>>> The first step just purposely writes some random fuzzed data to
>>>> the base inode metadata, compressed indexes, or dir data field
>>>> (one round one field) to make it validity and coverability.
>>>>
>>>>>
>>>>> You might want to write such a debugging tool for erofs so that you can
>>>>> take apart crashed images to get a better idea of what went wrong, and
>>>>> to write easy fuzzing tests.
>>>>
>>>> Yes, we will do such a debugging tool of course. Actually Li Guifu is now
>>>> developping a erofs-fuse to support old linux versions or other OSes for
>>>> archiveing only use, we will base on that code to develop a better fuzzer
>>>> tool as well.
>>>
>>> Personally speaking, debugging tool is way more important than a running
>>> kernel module/fuse.
>>> It's human trying to write the code, most of time is spent educating
>>> code readers, thus debugging tool is way more important than dead cold code.
>>>
>>> Thanks,
>>> Qu
>>>>
>>>> Thanks,
>>>> Gao Xiang
>>>>
>>>>>
>>>>> --D
>>>>>
>>>>>> umount mntdir
>>>>>> mount -t erofs -o loop testdir_fsl.fuzz.img mntdir
>>>>>> for j in `find mntdir -type f`; do
>>>>>> md5sum $j > /dev/null
>>>>>> done
>>>>>> done
>>>>>>
>>>>>> Thanks,
>>>>>> Gao Xiang
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Gao Xiang
>>>>>>>
>>>
>
[...]
>
> Yeah, it looks like we need searching more levels mapping to find the final
> physical block address of inode/node/data in btrfs.
>
> IMO, in a little lazy way, we can reform and reuse existed function in
> btrfs-progs which can find the mapping info of inode/node/data according to
> specified ino or ino+pg_no.
Maybe no need to go as deep as ino.
What about just go physical bytenr? E.g. for XFS/EXT* choose a random
bytenr. Then verify if that block is used, if not, try again.
If used, check if it's metadata. If not, try again.
(feel free to corrupt data, in fact btrfs uses some data as space cache,
so it should make some sense)
If metadata, corrupt that bytenr/bytenr range in the metadata block,
regenerate checksum, call it a day and let kernel suffer.
For btrfs, just do extra physical -> logical convert in the first place,
then follow the same workflow.
It should work for any fs as long as it's on single device.
>
>>
>> It may depends on the granularity. But definitely a good idea to do so
>> in a generic way.
>> Currently we depend on super kind student developers/reporters on such
>
> Yup, I just guess Wen Xu may be interested in working on a generic way to fuzz
> filesystem, as I know they dig deep in filesystem code when doing fuzz.
Don't forget Yoon Jungyeon, I see more than one times he reported fuzzed
images with proper reproducer and bugzilla links.
Even using his personal mail address, not school mail address.
Those guys are really awesome!
> BTW,
> which impresses me is, constructing checkpoint by injecting one byte, and then
> write a correct recalculated checksum value on that checkpoint, making that
> checkpoint looks valid...
IIRC F2FS guys may be also investigating a similar mechanism, as they
also got a hard fight against reports from those awesome reporters.
So such fuzzed image is a new trend for fs development.
Thanks,
Qu
>
> Thanks,
>
On Tue, Aug 20, 2019 at 10:24:11AM +0800, Chao Yu wrote:
> Out of curiosity, it looks like every mainstream filesystem has its own
> fuzz/injection tool in their tool-set, if it's really such a generic
> requirement, why shouldn't there be a common tool to handle that, let specified
> filesystem fill the tool's callback to seek a node/block and supported fields
> can be fuzzed in inode. It can help to avoid redundant work whenever Linux
> welcomes a new filesystem....
The reason why there needs to be at least some file system specific
code for fuzz testing is because for efficiency's sake, you don't want
to fuzz every single bit in the file system, but just the ones which
are most interesting (e.g., the metadata blocks). For file systems
which use checksum to protect against accidental corruption, the file
system fuzzer needs to also fix up the checksums (since you can be
sure malicious attackers will do this).
What you *can* do is to make the file system specific portion of the
work as small as possible. Great work in this area is Professor Kim's
Janus[1][2] and Hydra[2] work. (Hydra is about to be published at SOSP 19,
and was partially funded from a Google Faculty Research Work.)
[1] https://taesoo.kim/pubs/2019/xu:janus.pdf
[2] https://github.com/sslab-gatech/janus
[3] https://github.com/sslab-gatech/hydra
> > Personally speaking, debugging tool is way more important than a running
> > kernel module/fuse.
> > It's human trying to write the code, most of time is spent educating
> > code readers, thus debugging tool is way more important than dead cold code.
I personally find that having a tool like e2fsprogs' debugfs program
to be really handy. It's useful for creating regression test images;
it's useful for debugging results from fuzzing systems like Janus;
it's useful for examining broken file systems extracted from busted
Android handsets during dogfood to root cause bugs which escaped
xfstests testing; etc.
Cheers,
- Ted
Hi Ted,
On Tue, Aug 20, 2019 at 11:56:23AM -0400, Theodore Y. Ts'o wrote:
> On Tue, Aug 20, 2019 at 10:24:11AM +0800, Chao Yu wrote:
> > Out of curiosity, it looks like every mainstream filesystem has its own
> > fuzz/injection tool in their tool-set, if it's really such a generic
> > requirement, why shouldn't there be a common tool to handle that, let specified
> > filesystem fill the tool's callback to seek a node/block and supported fields
> > can be fuzzed in inode. It can help to avoid redundant work whenever Linux
> > welcomes a new filesystem....
>
> The reason why there needs to be at least some file system specific
> code for fuzz testing is because for efficiency's sake, you don't want
> to fuzz every single bit in the file system, but just the ones which
> are most interesting (e.g., the metadata blocks). For file systems
> which use checksum to protect against accidental corruption, the file
> system fuzzer needs to also fix up the checksums (since you can be
> sure malicious attackers will do this).
I personally agree with your idea, some fs specfic code needed at least.
>
> What you *can* do is to make the file system specific portion of the
> work as small as possible. Great work in this area is Professor Kim's
> Janus[1][2] and Hydra[2] work. (Hydra is about to be published at SOSP 19,
> and was partially funded from a Google Faculty Research Work.)
For EROFS, it's a special case since it is a RO fs, and erofs mkfs
will generate reproducable images (which means, for one dir trees,
it only generates exact one result except for build time).
For this reason (read only), the fastest way to do fault injection
for EROFS is to modify mkfs and inject some faults when writing
each metadata to the generated images, that is all what I'm done
that night. I think it has similar effect since EROFS is an RO fs
for some given dirs (except for it cannot modify a specfic field
from a given image, but from its source dir). Actually I found 6
issues related with corrupted compressed images, and it can now
run smoothly for about an hour.
https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=experimental-fuzzer
What we will do next is to develop an independent fuzzer, which
need the ability of parsing the filesystem image (which share
common code with erofs-fuse to parse the filesystem) and then
it can inject some faults to the original image.
>
> [1] https://taesoo.kim/pubs/2019/xu:janus.pdf
> [2] https://github.com/sslab-gatech/janus
> [3] https://github.com/sslab-gatech/hydra
>
> > > Personally speaking, debugging tool is way more important than a running
> > > kernel module/fuse.
> > > It's human trying to write the code, most of time is spent educating
> > > code readers, thus debugging tool is way more important than dead cold code.
>
> I personally find that having a tool like e2fsprogs' debugfs program
> to be really handy. It's useful for creating regression test images;
> it's useful for debugging results from fuzzing systems like Janus;
> it's useful for examining broken file systems extracted from busted
> Android handsets during dogfood to root cause bugs which escaped
> xfstests testing; etc.
Yes, what I want to say is that these fses (ext4, xfs, btrfs, ...) do
awesome work by many people for many time. But I personally have 2 hand,
24 hours (including work hours) a day only.
I will refer to these debugfs / xfs_db / btrfs ins dump-tree great work
to get your experience and develop our EROFS tools. I learn a lot from
this topic.
Thanks,
Gao Xiang
>
> Cheers,
>
> - Ted
On Wed, Aug 21, 2019 at 12:35:08AM +0800, Gao Xiang wrote:
>
> For EROFS, it's a special case since it is a RO fs, and erofs mkfs
> will generate reproducable images (which means, for one dir trees,
> it only generates exact one result except for build time).
Agreed, and given that, doing the fuzzing in your mkfs tool makes
perfect sense. I wasn't surprised at all that you chose that path.
Cheers,
- Ted
On Wed, Aug 21, 2019 at 09:34:02AM +0800, Chao Yu wrote:
> On 2019/8/20 23:56, Theodore Y. Ts'o wrote:
> > The reason why there needs to be at least some file system specific
> > code for fuzz testing is because for efficiency's sake, you don't want
> > to fuzz every single bit in the file system, but just the ones which
> > are most interesting (e.g., the metadata blocks). For file systems
> > which use checksum to protect against accidental corruption, the file
> > system fuzzer needs to also fix up the checksums (since you can be
> > sure malicious attackers will do this).
>
> Yup, IMO, if we really want such tool, it needs to:
> - move all generic fuzz codes (trigger random fuzzing in meta/data area) into
> that tool, and
> - make filesystem generic fs_meta/file_node lookup/inject/pack function as a
> callback, such as
> * .find_fs_sb
> * .inject_fs_sb
> * .pack_fs_sb
What about group descriptors? AG headers? The AGFLWTFBBQLOL?
> * .find_fs_bitmap
> * .inject_fs_bitmap
Probably want an find/inject for log blocks too.
Oh, wait, XFS doesn't log blocks like jbd2 does. :) :)
> * .find_fs_inode_bitmap
> * .inject_fs_inode_bitmap
XFS has an inode bitmap? ;)
(This is why there's no generic fuzz tool; every fs is different enough
that doing so would be sort of a mess.)
((Granted, you could also look at how xfstests uses the xfs_db fuzz
command so at least it would be systematic...))
> * .find_inode_by_num
> * .inject_inode
> * .pack_inode
> * .find_tree_node_by_level
> ...
What about the name/value btrees? (Ok, I'll stop now.)
--D
> then specific filesystem can fill the callback to tell how the tool can locate a
> field in inode or a metadata in tree node and then trigger the designed fuzz.
>
> It will be easier to rewrite whole generic fwk for each filesystem, because
> existed filesystem userspace tool should has included above callback's detail
> codes...
>
> > On Tue, Aug 20, 2019 at 10:24:11AM +0800, Chao Yu wrote:
> >> filesystem fill the tool's callback to seek a node/block and supported fields
> >> can be fuzzed in inode.
>
> >
> > What you *can* do is to make the file system specific portion of the
> > work as small as possible. Great work in this area is Professor Kim's
> > Janus[1][2] and Hydra[2] work. (Hydra is about to be published at SOSP 19,
> > and was partially funded from a Google Faculty Research Work.)
> >
> > [1] https://taesoo.kim/pubs/2019/xu:janus.pdf
> > [2] https://github.com/sslab-gatech/janus
> > [3] https://github.com/sslab-gatech/hydra
>
> Thanks for the information!
>
> It looks like janus and hydra alreay have generic compress/decompress function
> across different filesystems, it's really a good job, I do think it may be the
> one once it becomes more generic.
>
> Thanks
>
> >
On 2019/8/20 16:46, Qu Wenruo wrote:
> [...]
>>
>> Yeah, it looks like we need searching more levels mapping to find the final
>> physical block address of inode/node/data in btrfs.
>>
>> IMO, in a little lazy way, we can reform and reuse existed function in
>> btrfs-progs which can find the mapping info of inode/node/data according to
>> specified ino or ino+pg_no.
>
> Maybe no need to go as deep as ino.
>
> What about just go physical bytenr? E.g. for XFS/EXT* choose a random
> bytenr. Then verify if that block is used, if not, try again.
>
> If used, check if it's metadata. If not, try again.
> (feel free to corrupt data, in fact btrfs uses some data as space cache,
> so it should make some sense)
>
> If metadata, corrupt that bytenr/bytenr range in the metadata block,
> regenerate checksum, call it a day and let kernel suffer.
>
> For btrfs, just do extra physical -> logical convert in the first place,
> then follow the same workflow.
> It should work for any fs as long as it's on single device.
Agree, it will be easier to trigger random injection in specific area, and also
I agreed with Ted, some of the time we prefer to do injection in specified field
of meta, it looks developer needs to do more work for that.
>
>>
>>>
>>> It may depends on the granularity. But definitely a good idea to do so
>>> in a generic way.
>>> Currently we depend on super kind student developers/reporters on such
>>
>> Yup, I just guess Wen Xu may be interested in working on a generic way to fuzz
>> filesystem, as I know they dig deep in filesystem code when doing fuzz.
>
> Don't forget Yoon Jungyeon, I see more than one times he reported fuzzed
> images with proper reproducer and bugzilla links.
Of course I remember him. :)
I guess btrfs/f2fs should has improved their stability/robustness a lot due to
Jungyeon and Wen Xu's gret fuzz bug report.
> Even using his personal mail address, not school mail address.
>
> Those guys are really awesome!
>
>> BTW,
>> which impresses me is, constructing checkpoint by injecting one byte, and then
>> write a correct recalculated checksum value on that checkpoint, making that
>> checkpoint looks valid...
>
> IIRC F2FS guys may be also investigating a similar mechanism, as they
> also got a hard fight against reports from those awesome reporters.
Actually, f2fs only support realtime fault injection framework, which allows us
to inject memory exhausting, IO error, lack of free blocks, shutdown... error
during fsstress test.
I do think f2fs needs that kind of tool later.
Thanks,
>
> So such fuzzed image is a new trend for fs development.
>
> Thanks,
> Qu
>
>>
>> Thanks,
>>
>
On 2019/8/20 23:56, Theodore Y. Ts'o wrote:
> The reason why there needs to be at least some file system specific
> code for fuzz testing is because for efficiency's sake, you don't want
> to fuzz every single bit in the file system, but just the ones which
> are most interesting (e.g., the metadata blocks). For file systems
> which use checksum to protect against accidental corruption, the file
> system fuzzer needs to also fix up the checksums (since you can be
> sure malicious attackers will do this).
Yup, IMO, if we really want such tool, it needs to:
- move all generic fuzz codes (trigger random fuzzing in meta/data area) into
that tool, and
- make filesystem generic fs_meta/file_node lookup/inject/pack function as a
callback, such as
* .find_fs_sb
* .inject_fs_sb
* .pack_fs_sb
* .find_fs_bitmap
* .inject_fs_bitmap
* .find_fs_inode_bitmap
* .inject_fs_inode_bitmap
* .find_inode_by_num
* .inject_inode
* .pack_inode
* .find_tree_node_by_level
...
then specific filesystem can fill the callback to tell how the tool can locate a
field in inode or a metadata in tree node and then trigger the designed fuzz.
It will be easier to rewrite whole generic fwk for each filesystem, because
existed filesystem userspace tool should has included above callback's detail
codes...
> On Tue, Aug 20, 2019 at 10:24:11AM +0800, Chao Yu wrote:
>> filesystem fill the tool's callback to seek a node/block and supported fields
>> can be fuzzed in inode.
>
> What you *can* do is to make the file system specific portion of the
> work as small as possible. Great work in this area is Professor Kim's
> Janus[1][2] and Hydra[2] work. (Hydra is about to be published at SOSP 19,
> and was partially funded from a Google Faculty Research Work.)
>
> [1] https://taesoo.kim/pubs/2019/xu:janus.pdf
> [2] https://github.com/sslab-gatech/janus
> [3] https://github.com/sslab-gatech/hydra
Thanks for the information!
It looks like janus and hydra alreay have generic compress/decompress function
across different filesystems, it's really a good job, I do think it may be the
one once it becomes more generic.
Thanks
>
On 2019/8/21 9:48, Darrick J. Wong wrote:
> On Wed, Aug 21, 2019 at 09:34:02AM +0800, Chao Yu wrote:
>> On 2019/8/20 23:56, Theodore Y. Ts'o wrote:
>>> The reason why there needs to be at least some file system specific
>>> code for fuzz testing is because for efficiency's sake, you don't want
>>> to fuzz every single bit in the file system, but just the ones which
>>> are most interesting (e.g., the metadata blocks). For file systems
>>> which use checksum to protect against accidental corruption, the file
>>> system fuzzer needs to also fix up the checksums (since you can be
>>> sure malicious attackers will do this).
>>
>> Yup, IMO, if we really want such tool, it needs to:
>> - move all generic fuzz codes (trigger random fuzzing in meta/data area) into
>> that tool, and
>> - make filesystem generic fs_meta/file_node lookup/inject/pack function as a
>> callback, such as
>> * .find_fs_sb
>> * .inject_fs_sb
>> * .pack_fs_sb
>
> What about group descriptors? AG headers? The AGFLWTFBBQLOL?
>
>> * .find_fs_bitmap
>> * .inject_fs_bitmap
>
> Probably want an find/inject for log blocks too.
>
> Oh, wait, XFS doesn't log blocks like jbd2 does. :) :)
Yes, I admit that I should miss a lot of fs meta type here, but that's just a
simple example here, we should not treat it as a full design.... :)
>
>> * .find_fs_inode_bitmap
>> * .inject_fs_inode_bitmap
>
> XFS has an inode bitmap? ;)
We can leave callback as NULL? ;)
>
> (This is why there's no generic fuzz tool; every fs is different enough
> that doing so would be sort of a mess.)
Yes, I just wonder if there is any possible we can save some redundant work.
>
> ((Granted, you could also look at how xfstests uses the xfs_db fuzz
> command so at least it would be systematic...))
Okay, I will check that.
Thanks,
>
>> * .find_inode_by_num
>> * .inject_inode
>> * .pack_inode
>> * .find_tree_node_by_level
>> ...
>
> What about the name/value btrees? (Ok, I'll stop now.)
>
> --D
>
>> then specific filesystem can fill the callback to tell how the tool can locate a
>> field in inode or a metadata in tree node and then trigger the designed fuzz.
>>
>> It will be easier to rewrite whole generic fwk for each filesystem, because
>> existed filesystem userspace tool should has included above callback's detail
>> codes...
>>
>>> On Tue, Aug 20, 2019 at 10:24:11AM +0800, Chao Yu wrote:
>>>> filesystem fill the tool's callback to seek a node/block and supported fields
>>>> can be fuzzed in inode.
>>
>>>
>>> What you *can* do is to make the file system specific portion of the
>>> work as small as possible. Great work in this area is Professor Kim's
>>> Janus[1][2] and Hydra[2] work. (Hydra is about to be published at SOSP 19,
>>> and was partially funded from a Google Faculty Research Work.)
>>>
>>> [1] https://taesoo.kim/pubs/2019/xu:janus.pdf
>>> [2] https://github.com/sslab-gatech/janus
>>> [3] https://github.com/sslab-gatech/hydra
>>
>> Thanks for the information!
>>
>> It looks like janus and hydra alreay have generic compress/decompress function
>> across different filesystems, it's really a good job, I do think it may be the
>> one once it becomes more generic.
>>
>> Thanks
>>
>>>
> .
>