linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] erofs: move erofs out of staging
@ 2019-08-17  8:23 Gao Xiang
  2019-08-17 21:19 ` Richard Weinberger
  0 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-17  8:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Alexander Viro, linux-fsdevel, devel,
	linux-erofs, LKML
  Cc: Andrew Morton, Stephen Rothwell, Theodore Ts'o, Pavel Machek,
	David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner, Jaegeuk Kim, Jan Kara,
	Richard Weinberger, Linus Torvalds, Chao Yu, Miao Xie, Li Guifu,
	Fang Wei, Gao Xiang

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 <gregkh@linuxfoundation.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Pavel Machek <pavel@denx.de>
Cc: David Sterba <dsterba@suse.cz>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Darrick J . Wong <darrick.wong@oracle.com>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Richard Weinberger <richard@nod.at>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Chao Yu <yuchao0@huawei.com>
Cc: Miao Xie <miaoxie@huawei.com>
Cc: Li Guifu <bluce.liguifu@huawei.com>
Cc: Fang Wei <fangwei1@huawei.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---

Hi,

 This is a formal moving patch based on a previous patch for staging tree
 https://lore.kernel.org/r/20190816071142.8633-1-gaoxiang25@huawei.com/

 The previous related topic is
 https://lore.kernel.org/r/20190815044155.88483-1-gaoxiang25@huawei.com/

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   <linux-erofs@lists.ozlabs.org>
 
-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 <devel@driverdev.osuosl.org>
-
 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 <gaoxiang25@huawei.com>
+M:	Chao Yu <yuchao0@huawei.com>
+L:	linux-erofs@lists.ozlabs.org
+S:	Maintained
+F:	fs/erofs/
+
 ERRSEQ ERROR TRACKING INFRASTRUCTURE
 M:	Jeff Layton <jlayton@kernel.org>
 S:	Maintained
@@ -15215,13 +15222,6 @@ M:	H Hartley Sweeten <hsweeten@visionengravers.com>
 S:	Odd Fixes
 F:	drivers/staging/comedi/
 
-STAGING - EROFS FILE SYSTEM
-M:	Gao Xiang <gaoxiang25@huawei.com>
-M:	Chao Yu <yuchao0@huawei.com>
-L:	linux-erofs@lists.ozlabs.org
-S:	Maintained
-F:	drivers/staging/erofs/
-
 STAGING - FIELDBUS SUBSYSTEM
 M:	Sven Van Asbroeck <TheSven73@gmail.com>
 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 <bluce.liguifu@huawei.com> and
-the co-maintainer Fang Wei <fangwei1@huawei.com> 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   <linux-erofs@lists.ozlabs.org>
-  Gao Xiang                  <gaoxiang25@huawei.com>
-  Chao Yu                    <yuchao0@huawei.com>
-
-Cc: (for linux-kernel upstream patches)
-  Greg Kroah-Hartman         <gregkh@linuxfoundation.org>
-  linux-staging mailing list <devel@driverdev.osuosl.org>
-
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
@@ -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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
@@ -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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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 <gaoxiang25@huawei.com>
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


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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-17  8:23 [PATCH] erofs: move erofs out of staging Gao Xiang
@ 2019-08-17 21:19 ` Richard Weinberger
  2019-08-17 22:07   ` Gao Xiang
  0 siblings, 1 reply; 72+ messages in thread
From: Richard Weinberger @ 2019-08-17 21:19 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds,
	Chao Yu, Miao Xie, Li Guifu, Fang Wei, Gao Xiang

----- Ursprüngliche Mail -----
> Von: "Gao Xiang" <hsiangkao@aol.com>
> An: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Al Viro" <viro@zeniv.linux.org.uk>, "linux-fsdevel"
> <linux-fsdevel@vger.kernel.org>, devel@driverdev.osuosl.org, linux-erofs@lists.ozlabs.org, "linux-kernel"
> <linux-kernel@vger.kernel.org>
> CC: "Andrew Morton" <akpm@linux-foundation.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>, "tytso" <tytso@mit.edu>,
> "Pavel Machek" <pavel@denx.de>, "David Sterba" <dsterba@suse.cz>, "Amir Goldstein" <amir73il@gmail.com>, "Christoph
> Hellwig" <hch@infradead.org>, "Darrick J . Wong" <darrick.wong@oracle.com>, "Dave Chinner" <david@fromorbit.com>,
> "Jaegeuk Kim" <jaegeuk@kernel.org>, "Jan Kara" <jack@suse.cz>, "richard" <richard@nod.at>, "torvalds"
> <torvalds@linux-foundation.org>, "Chao Yu" <yuchao0@huawei.com>, "Miao Xie" <miaoxie@huawei.com>, "Li Guifu"
> <bluce.liguifu@huawei.com>, "Fang Wei" <fangwei1@huawei.com>, "Gao Xiang" <gaoxiang25@huawei.com>
> 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-17 21:19 ` Richard Weinberger
@ 2019-08-17 22:07   ` Gao Xiang
  2019-08-17 23:25     ` Richard Weinberger
  0 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-17 22:07 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds,
	Chao Yu, Miao Xie, Li Guifu, Fang Wei, Gao Xiang

Hi Richard,

On Sat, Aug 17, 2019 at 11:19:50PM +0200, Richard Weinberger wrote:
> ----- Urspr?ngliche Mail -----
> > Von: "Gao Xiang" <hsiangkao@aol.com>
> > An: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Al Viro" <viro@zeniv.linux.org.uk>, "linux-fsdevel"
> > <linux-fsdevel@vger.kernel.org>, devel@driverdev.osuosl.org, linux-erofs@lists.ozlabs.org, "linux-kernel"
> > <linux-kernel@vger.kernel.org>
> > CC: "Andrew Morton" <akpm@linux-foundation.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>, "tytso" <tytso@mit.edu>,
> > "Pavel Machek" <pavel@denx.de>, "David Sterba" <dsterba@suse.cz>, "Amir Goldstein" <amir73il@gmail.com>, "Christoph
> > Hellwig" <hch@infradead.org>, "Darrick J . Wong" <darrick.wong@oracle.com>, "Dave Chinner" <david@fromorbit.com>,
> > "Jaegeuk Kim" <jaegeuk@kernel.org>, "Jan Kara" <jack@suse.cz>, "richard" <richard@nod.at>, "torvalds"
> > <torvalds@linux-foundation.org>, "Chao Yu" <yuchao0@huawei.com>, "Miao Xie" <miaoxie@huawei.com>, "Li Guifu"
> > <bluce.liguifu@huawei.com>, "Fang Wei" <fangwei1@huawei.com>, "Gao Xiang" <gaoxiang25@huawei.com>
> > 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/20190815044155.88483-1-gaoxiang25@huawei.com/

Thanks,
Gao Xiang

> 
> Thanks,
> //richard

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-17 22:07   ` Gao Xiang
@ 2019-08-17 23:25     ` Richard Weinberger
  2019-08-17 23:38       ` Gao Xiang
  0 siblings, 1 reply; 72+ messages in thread
From: Richard Weinberger @ 2019-08-17 23:25 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds, Chao Yu,
	Miao Xie, Li Guifu, Fang Wei, Gao Xiang

----- 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/20190815044155.88483-1-gaoxiang25@huawei.com/

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-17 23:25     ` Richard Weinberger
@ 2019-08-17 23:38       ` Gao Xiang
  2019-08-18  0:04         ` Gao Xiang
  2019-08-18  8:16         ` Richard Weinberger
  0 siblings, 2 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-17 23:38 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds, Chao Yu,
	Miao Xie, Li Guifu, Fang Wei, Gao Xiang

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/20190815044155.88483-1-gaoxiang25@huawei.com/
> 
> 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-17 23:38       ` Gao Xiang
@ 2019-08-18  0:04         ` Gao Xiang
  2019-08-18  0:52           ` Gao Xiang
  2019-08-18  8:16         ` Richard Weinberger
  1 sibling, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-18  0:04 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds, Chao Yu,
	Miao Xie, Li Guifu, Fang Wei, Gao Xiang

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18  0:04         ` Gao Xiang
@ 2019-08-18  0:52           ` Gao Xiang
  0 siblings, 0 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-18  0:52 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds, Chao Yu,
	Miao Xie, Li Guifu, Fang Wei, Gao Xiang

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-17 23:38       ` Gao Xiang
  2019-08-18  0:04         ` Gao Xiang
@ 2019-08-18  8:16         ` Richard Weinberger
  2019-08-18  8:45           ` Gao Xiang
  1 sibling, 1 reply; 72+ messages in thread
From: Richard Weinberger @ 2019-08-18  8:16 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds, Chao Yu,
	Miao Xie, Li Guifu, Fang Wei, Gao Xiang

----- 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18  8:16         ` Richard Weinberger
@ 2019-08-18  8:45           ` Gao Xiang
  2019-08-18  9:03             ` Richard Weinberger
  0 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-18  8:45 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds, Chao Yu,
	Miao Xie, Li Guifu, Fang Wei, Gao Xiang

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18  8:45           ` Gao Xiang
@ 2019-08-18  9:03             ` Richard Weinberger
  2019-08-18  9:09               ` Greg Kroah-Hartman
  2019-08-18  9:28               ` Gao Xiang
  0 siblings, 2 replies; 72+ messages in thread
From: Richard Weinberger @ 2019-08-18  9:03 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds, Chao Yu,
	Miao Xie, Li Guifu, Fang Wei, Gao Xiang

----- 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18  9:03             ` Richard Weinberger
@ 2019-08-18  9:09               ` Greg Kroah-Hartman
  2019-08-18  9:21                 ` Richard Weinberger
  2019-08-18  9:28               ` Gao Xiang
  1 sibling, 1 reply; 72+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-18  9:09 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Gao Xiang, Jan Kara, Chao Yu, Dave Chinner, David Sterba,
	Miao Xie, devel, Stephen Rothwell, Darrick, Christoph Hellwig,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, tytso,
	linux-kernel, Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel,
	Andrew Morton, torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18  9:09               ` Greg Kroah-Hartman
@ 2019-08-18  9:21                 ` Richard Weinberger
  2019-08-18 10:12                   ` Chao Yu
  2019-08-18 15:11                   ` Theodore Y. Ts'o
  0 siblings, 2 replies; 72+ messages in thread
From: Richard Weinberger @ 2019-08-18  9:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Gao Xiang, Jan Kara, Chao Yu, Dave Chinner, David Sterba,
	Miao Xie, devel, Stephen Rothwell, Darrick, Christoph Hellwig,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, tytso,
	linux-kernel, Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel,
	Andrew Morton, torvalds

----- 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18  9:03             ` Richard Weinberger
  2019-08-18  9:09               ` Greg Kroah-Hartman
@ 2019-08-18  9:28               ` Gao Xiang
  2019-08-19  5:28                 ` [PATCH] erofs: Use common kernel logging style Joe Perches
  1 sibling, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-18  9:28 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Greg Kroah-Hartman, Al Viro, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Andrew Morton, Stephen Rothwell, tytso,
	Pavel Machek, David Sterba, Amir Goldstein, Christoph Hellwig,
	Darrick, Dave Chinner, Jaegeuk Kim, Jan Kara, torvalds, Chao Yu,
	Miao Xie, Li Guifu, Fang Wei, Gao Xiang

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18  9:21                 ` Richard Weinberger
@ 2019-08-18 10:12                   ` Chao Yu
  2019-08-18 15:11                   ` Theodore Y. Ts'o
  1 sibling, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-18 10:12 UTC (permalink / raw)
  To: Richard Weinberger, Greg Kroah-Hartman
  Cc: Jan Kara, Amir Goldstein, Dave Chinner, linux-kernel, Miao Xie,
	devel, Stephen Rothwell, Darrick, Christoph Hellwig, Al Viro,
	Jaegeuk Kim, tytso, torvalds, David Sterba, Pavel Machek,
	linux-fsdevel, Andrew Morton, linux-erofs

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
> 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18  9:21                 ` Richard Weinberger
  2019-08-18 10:12                   ` Chao Yu
@ 2019-08-18 15:11                   ` Theodore Y. Ts'o
  2019-08-18 15:58                     ` Christoph Hellwig
                                       ` (2 more replies)
  1 sibling, 3 replies; 72+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-18 15:11 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Greg Kroah-Hartman, Gao Xiang, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Darrick,
	Christoph Hellwig, Amir Goldstein, linux-erofs, Al Viro,
	Jaegeuk Kim, linux-kernel, Li Guifu, Fang Wei, Pavel Machek,
	linux-fsdevel, Andrew Morton, torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 15:11                   ` Theodore Y. Ts'o
@ 2019-08-18 15:58                     ` Christoph Hellwig
  2019-08-18 16:16                       ` Eric Biggers
  2019-08-18 17:43                       ` Theodore Y. Ts'o
  2019-08-18 16:03                     ` Gao Xiang
  2019-08-18 17:06                     ` Richard Weinberger
  2 siblings, 2 replies; 72+ messages in thread
From: Christoph Hellwig @ 2019-08-18 15:58 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Richard Weinberger, Greg Kroah-Hartman,
	Gao Xiang, Jan Kara, Chao Yu, Dave Chinner, David Sterba,
	Miao Xie, devel, Stephen Rothwell, Darrick, Christoph Hellwig,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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.

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 15:11                   ` Theodore Y. Ts'o
  2019-08-18 15:58                     ` Christoph Hellwig
@ 2019-08-18 16:03                     ` Gao Xiang
  2019-08-18 17:06                     ` Richard Weinberger
  2 siblings, 0 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-18 16:03 UTC (permalink / raw)
  To: Theodore Y. Ts'o
  Cc: Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Chao Yu,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Darrick, Christoph Hellwig, Amir Goldstein, linux-erofs, Al Viro,
	Jaegeuk Kim, linux-kernel, Li Guifu, Fang Wei, Pavel Machek,
	linux-fsdevel, Andrew Morton, torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 15:58                     ` Christoph Hellwig
@ 2019-08-18 16:16                       ` Eric Biggers
  2019-08-18 16:22                         ` Christoph Hellwig
  2019-08-18 17:43                       ` Theodore Y. Ts'o
  1 sibling, 1 reply; 72+ messages in thread
From: Eric Biggers @ 2019-08-18 16:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Theodore Y. Ts'o, Richard Weinberger, Greg Kroah-Hartman,
	Gao Xiang, Jan Kara, Chao Yu, Dave Chinner, David Sterba,
	Miao Xie, devel, Stephen Rothwell, Darrick, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 16:16                       ` Eric Biggers
@ 2019-08-18 16:22                         ` Christoph Hellwig
  2019-08-18 16:33                           ` Gao Xiang
  2019-08-18 17:29                           ` Eric Biggers
  0 siblings, 2 replies; 72+ messages in thread
From: Christoph Hellwig @ 2019-08-18 16:22 UTC (permalink / raw)
  To: Christoph Hellwig, Theodore Y. Ts'o, Richard Weinberger,
	Greg Kroah-Hartman, Gao Xiang, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Darrick,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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).

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 16:22                         ` Christoph Hellwig
@ 2019-08-18 16:33                           ` Gao Xiang
  2019-08-18 17:29                           ` Eric Biggers
  1 sibling, 0 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-18 16:33 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Theodore Y. Ts'o, Richard Weinberger, Greg Kroah-Hartman,
	Jan Kara, Chao Yu, Dave Chinner, David Sterba, Miao Xie, devel,
	Stephen Rothwell, Darrick, Amir Goldstein, linux-erofs, Al Viro,
	Jaegeuk Kim, linux-kernel, Li Guifu, Fang Wei, Pavel Machek,
	linux-fsdevel, Andrew Morton, torvalds

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



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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 15:11                   ` Theodore Y. Ts'o
  2019-08-18 15:58                     ` Christoph Hellwig
  2019-08-18 16:03                     ` Gao Xiang
@ 2019-08-18 17:06                     ` Richard Weinberger
  2019-08-18 17:46                       ` Theodore Y. Ts'o
  2 siblings, 1 reply; 72+ messages in thread
From: Richard Weinberger @ 2019-08-18 17:06 UTC (permalink / raw)
  To: tytso
  Cc: Greg Kroah-Hartman, Gao Xiang, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Darrick,
	Christoph Hellwig, Amir Goldstein, linux-erofs, Al Viro,
	Jaegeuk Kim, linux-kernel, Li Guifu, Fang Wei, Pavel Machek,
	linux-fsdevel, Andrew Morton, torvalds

----- 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 16:22                         ` Christoph Hellwig
  2019-08-18 16:33                           ` Gao Xiang
@ 2019-08-18 17:29                           ` Eric Biggers
  2019-08-18 17:47                             ` Christoph Hellwig
  1 sibling, 1 reply; 72+ messages in thread
From: Eric Biggers @ 2019-08-18 17:29 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Theodore Y. Ts'o, Richard Weinberger, Greg Kroah-Hartman,
	Gao Xiang, Jan Kara, Chao Yu, Dave Chinner, David Sterba,
	Miao Xie, devel, Stephen Rothwell, Darrick, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 15:58                     ` Christoph Hellwig
  2019-08-18 16:16                       ` Eric Biggers
@ 2019-08-18 17:43                       ` Theodore Y. Ts'o
  1 sibling, 0 replies; 72+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-18 17:43 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Richard Weinberger, Greg Kroah-Hartman, Gao Xiang, Jan Kara,
	Chao Yu, Dave Chinner, David Sterba, Miao Xie, devel,
	Stephen Rothwell, Darrick, Amir Goldstein, linux-erofs, Al Viro,
	Jaegeuk Kim, linux-kernel, Li Guifu, Fang Wei, Pavel Machek,
	linux-fsdevel, Andrew Morton, torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 17:06                     ` Richard Weinberger
@ 2019-08-18 17:46                       ` Theodore Y. Ts'o
  2019-08-18 18:00                         ` Richard Weinberger
  0 siblings, 1 reply; 72+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-18 17:46 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Greg Kroah-Hartman, Gao Xiang, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Darrick,
	Christoph Hellwig, Amir Goldstein, linux-erofs, Al Viro,
	Jaegeuk Kim, linux-kernel, Li Guifu, Fang Wei, Pavel Machek,
	linux-fsdevel, Andrew Morton, torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 17:29                           ` Eric Biggers
@ 2019-08-18 17:47                             ` Christoph Hellwig
  2019-08-18 18:16                               ` Gao Xiang
  2019-08-19  7:37                               ` Richard Weinberger
  0 siblings, 2 replies; 72+ messages in thread
From: Christoph Hellwig @ 2019-08-18 17:47 UTC (permalink / raw)
  To: Christoph Hellwig, Theodore Y. Ts'o, Richard Weinberger,
	Greg Kroah-Hartman, Gao Xiang, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Darrick,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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.

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 17:46                       ` Theodore Y. Ts'o
@ 2019-08-18 18:00                         ` Richard Weinberger
  2019-08-18 18:31                           ` Gao Xiang
  0 siblings, 1 reply; 72+ messages in thread
From: Richard Weinberger @ 2019-08-18 18:00 UTC (permalink / raw)
  To: tytso
  Cc: Greg Kroah-Hartman, Gao Xiang, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Darrick,
	Christoph Hellwig, Amir Goldstein, linux-erofs, Al Viro,
	Jaegeuk Kim, linux-kernel, Li Guifu, Fang Wei, Pavel Machek,
	linux-fsdevel, Andrew Morton, torvalds

----- Ursprüngliche Mail -----
> Von: "tytso" <tytso@mit.edu>
> An: "richard" <richard@nod.at>
> CC: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Gao Xiang" <hsiangkao@aol.com>, "Jan Kara" <jack@suse.cz>, "Chao
> Yu" <yuchao0@huawei.com>, "Dave Chinner" <david@fromorbit.com>, "David Sterba" <dsterba@suse.cz>, "Miao Xie"
> <miaoxie@huawei.com>, "devel" <devel@driverdev.osuosl.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>, "Darrick"
> <darrick.wong@oracle.com>, "Christoph Hellwig" <hch@infradead.org>, "Amir Goldstein" <amir73il@gmail.com>,
> "linux-erofs" <linux-erofs@lists.ozlabs.org>, "Al Viro" <viro@zeniv.linux.org.uk>, "Jaegeuk Kim" <jaegeuk@kernel.org>,
> "linux-kernel" <linux-kernel@vger.kernel.org>, "Li Guifu" <bluce.liguifu@huawei.com>, "Fang Wei" <fangwei1@huawei.com>,
> "Pavel Machek" <pavel@denx.de>, "linux-fsdevel" <linux-fsdevel@vger.kernel.org>, "Andrew Morton"
> <akpm@linux-foundation.org>, "torvalds" <torvalds@linux-foundation.org>
> 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 17:47                             ` Christoph Hellwig
@ 2019-08-18 18:16                               ` Gao Xiang
  2019-08-18 20:14                                 ` Gao Xiang
  2019-08-19  7:37                               ` Richard Weinberger
  1 sibling, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-18 18:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Theodore Y. Ts'o, Richard Weinberger, Greg Kroah-Hartman,
	Gao Xiang, Jan Kara, Chao Yu, Dave Chinner, David Sterba,
	Miao Xie, devel, Stephen Rothwell, Darrick, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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


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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 18:00                         ` Richard Weinberger
@ 2019-08-18 18:31                           ` Gao Xiang
  0 siblings, 0 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-18 18:31 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: tytso, Jan Kara, Amir Goldstein, Dave Chinner, linux-kernel,
	Miao Xie, devel, Stephen Rothwell, Darrick, Christoph Hellwig,
	torvalds, Al Viro, Jaegeuk Kim, Greg Kroah-Hartman, David Sterba,
	Pavel Machek, linux-fsdevel, Andrew Morton, linux-erofs

Hi Richard,

On Sun, Aug 18, 2019 at 08:00:28PM +0200, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "tytso" <tytso@mit.edu>
> > An: "richard" <richard@nod.at>
> > CC: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Gao Xiang" <hsiangkao@aol.com>, "Jan Kara" <jack@suse.cz>, "Chao
> > Yu" <yuchao0@huawei.com>, "Dave Chinner" <david@fromorbit.com>, "David Sterba" <dsterba@suse.cz>, "Miao Xie"
> > <miaoxie@huawei.com>, "devel" <devel@driverdev.osuosl.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>, "Darrick"
> > <darrick.wong@oracle.com>, "Christoph Hellwig" <hch@infradead.org>, "Amir Goldstein" <amir73il@gmail.com>,
> > "linux-erofs" <linux-erofs@lists.ozlabs.org>, "Al Viro" <viro@zeniv.linux.org.uk>, "Jaegeuk Kim" <jaegeuk@kernel.org>,
> > "linux-kernel" <linux-kernel@vger.kernel.org>, "Li Guifu" <bluce.liguifu@huawei.com>, "Fang Wei" <fangwei1@huawei.com>,
> > "Pavel Machek" <pavel@denx.de>, "linux-fsdevel" <linux-fsdevel@vger.kernel.org>, "Andrew Morton"
> > <akpm@linux-foundation.org>, "torvalds" <torvalds@linux-foundation.org>
> > 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 18:16                               ` Gao Xiang
@ 2019-08-18 20:14                                 ` Gao Xiang
  2019-08-19  7:35                                   ` Richard Weinberger
  2019-08-19 16:09                                   ` [PATCH] erofs: move erofs out of staging Darrick J. Wong
  0 siblings, 2 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-18 20:14 UTC (permalink / raw)
  To: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger
  Cc: Greg Kroah-Hartman, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Darrick,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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
> 

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

* [PATCH] erofs: Use common kernel logging style
  2019-08-18  9:28               ` Gao Xiang
@ 2019-08-19  5:28                 ` Joe Perches
  2019-08-19  5:52                   ` Gao Xiang
  0 siblings, 1 reply; 72+ messages in thread
From: Joe Perches @ 2019-08-19  5:28 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Greg Kroah-Hartman, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Chao Yu, Miao Xie, Li Guifu, Fang Wei, 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 <joe@perches.com>
---
 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);
 



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

* Re: [PATCH] erofs: Use common kernel logging style
  2019-08-19  5:52                   ` Gao Xiang
@ 2019-08-19  5:47                     ` Joe Perches
  2019-08-19  6:08                       ` Gao Xiang
  0 siblings, 1 reply; 72+ messages in thread
From: Joe Perches @ 2019-08-19  5:47 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Gao Xiang, Greg Kroah-Hartman, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Chao Yu, Miao Xie, Li Guifu, Fang Wei

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);




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

* Re: [PATCH] erofs: Use common kernel logging style
  2019-08-19  5:28                 ` [PATCH] erofs: Use common kernel logging style Joe Perches
@ 2019-08-19  5:52                   ` Gao Xiang
  2019-08-19  5:47                     ` Joe Perches
  0 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-19  5:52 UTC (permalink / raw)
  To: Joe Perches
  Cc: Gao Xiang, Greg Kroah-Hartman, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Chao Yu, Miao Xie, Li Guifu, Fang Wei

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 <joe@perches.com>
> ---
>  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);
>  
> 
> 

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

* Re: [PATCH] erofs: Use common kernel logging style
  2019-08-19  5:47                     ` Joe Perches
@ 2019-08-19  6:08                       ` Gao Xiang
  0 siblings, 0 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-19  6:08 UTC (permalink / raw)
  To: Joe Perches
  Cc: Gao Xiang, Greg Kroah-Hartman, linux-fsdevel, devel, linux-erofs,
	linux-kernel, Chao Yu, Miao Xie, Li Guifu, Fang Wei

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);
> 
> 
> 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 20:14                                 ` Gao Xiang
@ 2019-08-19  7:35                                   ` Richard Weinberger
  2019-08-19  8:02                                     ` Gao Xiang
  2019-08-19 16:09                                   ` [PATCH] erofs: move erofs out of staging Darrick J. Wong
  1 sibling, 1 reply; 72+ messages in thread
From: Richard Weinberger @ 2019-08-19  7:35 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Christoph Hellwig, tytso, Eric Biggers, Greg Kroah-Hartman,
	Jan Kara, Chao Yu, Dave Chinner, David Sterba, Miao Xie, devel,
	Stephen Rothwell, Darrick, Amir Goldstein, linux-erofs, Al Viro,
	Jaegeuk Kim, linux-kernel, Li Guifu, Fang Wei, Pavel Machek,
	linux-fsdevel, Andrew Morton, torvalds

----- 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 17:47                             ` Christoph Hellwig
  2019-08-18 18:16                               ` Gao Xiang
@ 2019-08-19  7:37                               ` Richard Weinberger
  1 sibling, 0 replies; 72+ messages in thread
From: Richard Weinberger @ 2019-08-19  7:37 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: tytso, Greg Kroah-Hartman, Gao Xiang, Jan Kara, Chao Yu,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Darrick, Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim,
	linux-kernel, Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel,
	Andrew Morton, torvalds

----- 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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-19  7:35                                   ` Richard Weinberger
@ 2019-08-19  8:02                                     ` Gao Xiang
  2019-08-19 10:34                                       ` [PATCH 0/6] staging: erofs: first stage of corrupted compressed images Gao Xiang
  0 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-19  8:02 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Gao Xiang, Christoph Hellwig, tytso, Eric Biggers,
	Greg Kroah-Hartman, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Darrick,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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

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

* [PATCH 0/6] staging: erofs: first stage of corrupted compressed images
  2019-08-19  8:02                                     ` Gao Xiang
@ 2019-08-19 10:34                                       ` Gao Xiang
  2019-08-19 10:34                                         ` [PATCH 1/6] staging: erofs: some compressed cluster should be submitted for corrupted images Gao Xiang
                                                           ` (5 more replies)
  0 siblings, 6 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-19 10:34 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei, Gao Xiang

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


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

* [PATCH 1/6] staging: erofs: some compressed cluster should be submitted for corrupted images
  2019-08-19 10:34                                       ` [PATCH 0/6] staging: erofs: first stage of corrupted compressed images Gao Xiang
@ 2019-08-19 10:34                                         ` Gao Xiang
  2019-08-19 14:36                                           ` Chao Yu
  2019-08-19 14:39                                           ` Chao Yu
  2019-08-19 10:34                                         ` [PATCH 2/6] staging: erofs: cannot set EROFS_V_Z_INITED_BIT if fill_inode_lazy fails Gao Xiang
                                                           ` (4 subsequent siblings)
  5 siblings, 2 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-19 10:34 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei,
	Gao Xiang, stable

As reported by erofs_utils fuzzer, a logical page can belong
to at most 2 compressed clusters, if one compressed cluster
is corrupted, but the other has been ready in submitting chain.

The chain needs to submit anyway in order to keep the page
working properly (page unlocked with PG_error set, PG_uptodate
not set).

Let's fix it now.

Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
Cc: <stable@vger.kernel.org> # 4.19+
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/zdata.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/erofs/zdata.c b/drivers/staging/erofs/zdata.c
index 2d7aaf98f7de..87b0c96caf8f 100644
--- a/drivers/staging/erofs/zdata.c
+++ b/drivers/staging/erofs/zdata.c
@@ -1307,19 +1307,18 @@ static int z_erofs_vle_normalaccess_readpage(struct file *file,
 	err = z_erofs_do_read_page(&f, page, &pagepool);
 	(void)z_erofs_collector_end(&f.clt);
 
-	if (err) {
+	/* if some compressed cluster ready, need submit them anyway */
+	z_erofs_submit_and_unzip(inode->i_sb, &f.clt, &pagepool, true);
+
+	if (err)
 		errln("%s, failed to read, err [%d]", __func__, err);
-		goto out;
-	}
 
-	z_erofs_submit_and_unzip(inode->i_sb, &f.clt, &pagepool, true);
-out:
 	if (f.map.mpage)
 		put_page(f.map.mpage);
 
 	/* clean up the remaining free pages */
 	put_pages_list(&pagepool);
-	return 0;
+	return err;
 }
 
 static bool should_decompress_synchronously(struct erofs_sb_info *sbi,
-- 
2.17.1


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

* [PATCH 2/6] staging: erofs: cannot set EROFS_V_Z_INITED_BIT if fill_inode_lazy fails
  2019-08-19 10:34                                       ` [PATCH 0/6] staging: erofs: first stage of corrupted compressed images Gao Xiang
  2019-08-19 10:34                                         ` [PATCH 1/6] staging: erofs: some compressed cluster should be submitted for corrupted images Gao Xiang
@ 2019-08-19 10:34                                         ` Gao Xiang
  2019-08-19 14:43                                           ` Chao Yu
  2019-08-19 10:34                                         ` [PATCH 3/6] staging: erofs: add two missing erofs_workgroup_put for corrupted images Gao Xiang
                                                           ` (3 subsequent siblings)
  5 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-19 10:34 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei,
	Gao Xiang, stable

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: <stable@vger.kernel.org> # 5.3+
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 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


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

* [PATCH 3/6] staging: erofs: add two missing erofs_workgroup_put for corrupted images
  2019-08-19 10:34                                       ` [PATCH 0/6] staging: erofs: first stage of corrupted compressed images Gao Xiang
  2019-08-19 10:34                                         ` [PATCH 1/6] staging: erofs: some compressed cluster should be submitted for corrupted images Gao Xiang
  2019-08-19 10:34                                         ` [PATCH 2/6] staging: erofs: cannot set EROFS_V_Z_INITED_BIT if fill_inode_lazy fails Gao Xiang
@ 2019-08-19 10:34                                         ` Gao Xiang
  2019-08-19 14:40                                           ` Chao Yu
  2019-08-19 10:34                                         ` [PATCH 4/6] staging: erofs: avoid loop in submit chains Gao Xiang
                                                           ` (2 subsequent siblings)
  5 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-19 10:34 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei,
	Gao Xiang, stable

As reported by erofs-utils fuzzer, these error handling
path will be entered to handle corrupted images.

Lack of erofs_workgroup_puts will cause unmounting
unsuccessfully.

Fix these return values to EFSCORRUPTED as well.

Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
Cc: <stable@vger.kernel.org> # 4.19+
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/zdata.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/zdata.c b/drivers/staging/erofs/zdata.c
index 87b0c96caf8f..23283c97fd3b 100644
--- a/drivers/staging/erofs/zdata.c
+++ b/drivers/staging/erofs/zdata.c
@@ -357,14 +357,16 @@ static struct z_erofs_collection *cllookup(struct z_erofs_collector *clt,
 	cl = z_erofs_primarycollection(pcl);
 	if (unlikely(cl->pageofs != (map->m_la & ~PAGE_MASK))) {
 		DBG_BUGON(1);
-		return ERR_PTR(-EIO);
+		erofs_workgroup_put(grp);
+		return ERR_PTR(-EFSCORRUPTED);
 	}
 
 	length = READ_ONCE(pcl->length);
 	if (length & Z_EROFS_PCLUSTER_FULL_LENGTH) {
 		if ((map->m_llen << Z_EROFS_PCLUSTER_LENGTH_BIT) > length) {
 			DBG_BUGON(1);
-			return ERR_PTR(-EIO);
+			erofs_workgroup_put(grp);
+			return ERR_PTR(-EFSCORRUPTED);
 		}
 	} else {
 		unsigned int llen = map->m_llen << Z_EROFS_PCLUSTER_LENGTH_BIT;
-- 
2.17.1


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

* [PATCH 4/6] staging: erofs: avoid loop in submit chains
  2019-08-19 10:34                                       ` [PATCH 0/6] staging: erofs: first stage of corrupted compressed images Gao Xiang
                                                           ` (2 preceding siblings ...)
  2019-08-19 10:34                                         ` [PATCH 3/6] staging: erofs: add two missing erofs_workgroup_put for corrupted images Gao Xiang
@ 2019-08-19 10:34                                         ` Gao Xiang
  2019-08-19 14:50                                           ` Chao Yu
  2019-08-19 10:34                                         ` [PATCH 5/6] staging: erofs: detect potential multiref due to corrupted images Gao Xiang
  2019-08-19 10:34                                         ` [PATCH 6/6] staging: erofs: avoid endless loop of invalid lookback distance 0 Gao Xiang
  5 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-19 10:34 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei, Gao Xiang

As reported by erofs-utils fuzzer, 2 conditions
can happen in corrupted images, which can cause
unexpected behaviors.
 - access the same pcluster one more time;
 - access the tail end pcluster again, e.g.
            _ access again (will trigger tail merging)
           |
     1 2 3 1 2             ->   1 2 3 1
     |_ tail end of the chain    \___/ (unexpected behavior)
Let's detect and avoid them now.

Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/zdata.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/erofs/zdata.c b/drivers/staging/erofs/zdata.c
index 23283c97fd3b..aae2f2b8353f 100644
--- a/drivers/staging/erofs/zdata.c
+++ b/drivers/staging/erofs/zdata.c
@@ -132,7 +132,7 @@ enum z_erofs_collectmode {
 struct z_erofs_collector {
 	struct z_erofs_pagevec_ctor vector;
 
-	struct z_erofs_pcluster *pcl;
+	struct z_erofs_pcluster *pcl, *tailpcl;
 	struct z_erofs_collection *cl;
 	struct page **compressedpages;
 	z_erofs_next_pcluster_t owned_head;
@@ -353,6 +353,11 @@ static struct z_erofs_collection *cllookup(struct z_erofs_collector *clt,
 		return NULL;
 
 	pcl = container_of(grp, struct z_erofs_pcluster, obj);
+	if (clt->owned_head == &pcl->next || pcl == clt->tailpcl) {
+		DBG_BUGON(1);
+		erofs_workgroup_put(grp);
+		return ERR_PTR(-EFSCORRUPTED);
+	}
 
 	cl = z_erofs_primarycollection(pcl);
 	if (unlikely(cl->pageofs != (map->m_la & ~PAGE_MASK))) {
@@ -381,6 +386,9 @@ static struct z_erofs_collection *cllookup(struct z_erofs_collector *clt,
 		}
 	}
 	mutex_lock(&cl->lock);
+	/* used to check tail merging loop due to corrupted images */
+	if (clt->owned_head == Z_EROFS_PCLUSTER_TAIL)
+		clt->tailpcl = pcl;
 	clt->mode = try_to_claim_pcluster(pcl, &clt->owned_head);
 	clt->pcl = pcl;
 	clt->cl = cl;
@@ -434,6 +442,9 @@ static struct z_erofs_collection *clregister(struct z_erofs_collector *clt,
 		kmem_cache_free(pcluster_cachep, pcl);
 		return ERR_PTR(-EAGAIN);
 	}
+	/* used to check tail merging loop due to corrupted images */
+	if (clt->owned_head == Z_EROFS_PCLUSTER_TAIL)
+		clt->tailpcl = pcl;
 	clt->owned_head = &pcl->next;
 	clt->pcl = pcl;
 	clt->cl = cl;
-- 
2.17.1


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

* [PATCH 5/6] staging: erofs: detect potential multiref due to corrupted images
  2019-08-19 10:34                                       ` [PATCH 0/6] staging: erofs: first stage of corrupted compressed images Gao Xiang
                                                           ` (3 preceding siblings ...)
  2019-08-19 10:34                                         ` [PATCH 4/6] staging: erofs: avoid loop in submit chains Gao Xiang
@ 2019-08-19 10:34                                         ` Gao Xiang
  2019-08-19 14:57                                           ` Chao Yu
  2019-08-19 10:34                                         ` [PATCH 6/6] staging: erofs: avoid endless loop of invalid lookback distance 0 Gao Xiang
  5 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-19 10:34 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei,
	Gao Xiang, stable

As reported by erofs-utils fuzzer, currently, multiref
(ondisk deduplication) hasn't been supported for now,
we should forbid it properly.

Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
Cc: <stable@vger.kernel.org> # 4.19+
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/zdata.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/zdata.c b/drivers/staging/erofs/zdata.c
index aae2f2b8353f..5b6fef5181af 100644
--- a/drivers/staging/erofs/zdata.c
+++ b/drivers/staging/erofs/zdata.c
@@ -816,8 +816,16 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
 			pagenr = z_erofs_onlinepage_index(page);
 
 		DBG_BUGON(pagenr >= nr_pages);
-		DBG_BUGON(pages[pagenr]);
 
+		/*
+		 * currently EROFS doesn't support multiref(dedup),
+		 * so here erroring out one multiref page.
+		 */
+		if (unlikely(pages[pagenr])) {
+			DBG_BUGON(1);
+			SetPageError(pages[pagenr]);
+			z_erofs_onlinepage_endio(pages[pagenr]);
+		}
 		pages[pagenr] = page;
 	}
 	z_erofs_pagevec_ctor_exit(&ctor, true);
@@ -849,7 +857,11 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
 			pagenr = z_erofs_onlinepage_index(page);
 
 			DBG_BUGON(pagenr >= nr_pages);
-			DBG_BUGON(pages[pagenr]);
+			if (unlikely(pages[pagenr])) {
+				DBG_BUGON(1);
+				SetPageError(pages[pagenr]);
+				z_erofs_onlinepage_endio(pages[pagenr]);
+			}
 			pages[pagenr] = page;
 
 			overlapped = true;
-- 
2.17.1


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

* [PATCH 6/6] staging: erofs: avoid endless loop of invalid lookback distance 0
  2019-08-19 10:34                                       ` [PATCH 0/6] staging: erofs: first stage of corrupted compressed images Gao Xiang
                                                           ` (4 preceding siblings ...)
  2019-08-19 10:34                                         ` [PATCH 5/6] staging: erofs: detect potential multiref due to corrupted images Gao Xiang
@ 2019-08-19 10:34                                         ` Gao Xiang
  2019-08-19 14:58                                           ` Chao Yu
  5 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-19 10:34 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei,
	Gao Xiang, stable

As reported by erofs-utils fuzzer, Lookback distance should
be a positive number, so it should be actually looked back
rather than spinning.

Fixes: 02827e1796b3 ("staging: erofs: add erofs_map_blocks_iter")
Cc: <stable@vger.kernel.org> # 4.19+
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/zmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/erofs/zmap.c b/drivers/staging/erofs/zmap.c
index 7408e86823a4..774dacbc5b32 100644
--- a/drivers/staging/erofs/zmap.c
+++ b/drivers/staging/erofs/zmap.c
@@ -350,6 +350,12 @@ static int vle_extent_lookback(struct z_erofs_maprecorder *m,
 
 	switch (m->type) {
 	case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
+		if (unlikely(!m->delta[0])) {
+			errln("invalid lookback distance 0 at nid %llu",
+			      vi->nid);
+			DBG_BUGON(1);
+			return -EFSCORRUPTED;
+		}
 		return vle_extent_lookback(m, m->delta[0]);
 	case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
 		map->m_flags &= ~EROFS_MAP_ZIPPED;
-- 
2.17.1


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

* Re: [PATCH 1/6] staging: erofs: some compressed cluster should be submitted for corrupted images
  2019-08-19 10:34                                         ` [PATCH 1/6] staging: erofs: some compressed cluster should be submitted for corrupted images Gao Xiang
@ 2019-08-19 14:36                                           ` Chao Yu
  2019-08-19 14:39                                           ` Chao Yu
  1 sibling, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-19 14:36 UTC (permalink / raw)
  To: Gao Xiang, Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Miao Xie, weidu.du, Fang Wei, stable

On 2019-8-19 18:34, Gao Xiang wrote:
> As reported by erofs_utils fuzzer, a logical page can belong
> to at most 2 compressed clusters, if one compressed cluster
> is corrupted, but the other has been ready in submitting chain.
> 
> The chain needs to submit anyway in order to keep the page
> working properly (page unlocked with PG_error set, PG_uptodate
> not set).
> 
> Let's fix it now.
> 
> Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
> Cc: <stable@vger.kernel.org> # 4.19+
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

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

* Re: [PATCH 1/6] staging: erofs: some compressed cluster should be submitted for corrupted images
  2019-08-19 10:34                                         ` [PATCH 1/6] staging: erofs: some compressed cluster should be submitted for corrupted images Gao Xiang
  2019-08-19 14:36                                           ` Chao Yu
@ 2019-08-19 14:39                                           ` Chao Yu
  1 sibling, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-19 14:39 UTC (permalink / raw)
  To: Gao Xiang, Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Miao Xie, weidu.du, Fang Wei, stable

On 2019-8-19 18:34, Gao Xiang wrote:
> As reported by erofs_utils fuzzer, a logical page can belong
> to at most 2 compressed clusters, if one compressed cluster
> is corrupted, but the other has been ready in submitting chain.
> 
> The chain needs to submit anyway in order to keep the page
> working properly (page unlocked with PG_error set, PG_uptodate
> not set).
> 
> Let's fix it now.
> 
> Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
> Cc: <stable@vger.kernel.org> # 4.19+
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

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

* Re: [PATCH 3/6] staging: erofs: add two missing erofs_workgroup_put for corrupted images
  2019-08-19 10:34                                         ` [PATCH 3/6] staging: erofs: add two missing erofs_workgroup_put for corrupted images Gao Xiang
@ 2019-08-19 14:40                                           ` Chao Yu
  0 siblings, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-19 14:40 UTC (permalink / raw)
  To: Gao Xiang, Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Miao Xie, weidu.du, Fang Wei, stable

On 2019-8-19 18:34, Gao Xiang wrote:
> As reported by erofs-utils fuzzer, these error handling
> path will be entered to handle corrupted images.
> 
> Lack of erofs_workgroup_puts will cause unmounting
> unsuccessfully.
> 
> Fix these return values to EFSCORRUPTED as well.
> 
> Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
> Cc: <stable@vger.kernel.org> # 4.19+
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

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

* Re: [PATCH 2/6] staging: erofs: cannot set EROFS_V_Z_INITED_BIT if fill_inode_lazy fails
  2019-08-19 10:34                                         ` [PATCH 2/6] staging: erofs: cannot set EROFS_V_Z_INITED_BIT if fill_inode_lazy fails Gao Xiang
@ 2019-08-19 14:43                                           ` Chao Yu
  0 siblings, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-19 14:43 UTC (permalink / raw)
  To: Gao Xiang, Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Miao Xie, weidu.du, Fang Wei, stable

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: <stable@vger.kernel.org> # 5.3+
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

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

* Re: [PATCH 4/6] staging: erofs: avoid loop in submit chains
  2019-08-19 10:34                                         ` [PATCH 4/6] staging: erofs: avoid loop in submit chains Gao Xiang
@ 2019-08-19 14:50                                           ` Chao Yu
  0 siblings, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-19 14:50 UTC (permalink / raw)
  To: Gao Xiang, Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Miao Xie, weidu.du, Fang Wei

On 2019-8-19 18:34, Gao Xiang wrote:
> As reported by erofs-utils fuzzer, 2 conditions
> can happen in corrupted images, which can cause
> unexpected behaviors.
>  - access the same pcluster one more time;
>  - access the tail end pcluster again, e.g.
>             _ access again (will trigger tail merging)
>            |
>      1 2 3 1 2             ->   1 2 3 1
>      |_ tail end of the chain    \___/ (unexpected behavior)
> Let's detect and avoid them now.
> 
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

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

* Re: [PATCH 5/6] staging: erofs: detect potential multiref due to corrupted images
  2019-08-19 10:34                                         ` [PATCH 5/6] staging: erofs: detect potential multiref due to corrupted images Gao Xiang
@ 2019-08-19 14:57                                           ` Chao Yu
  2019-08-21  2:19                                             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 72+ messages in thread
From: Chao Yu @ 2019-08-19 14:57 UTC (permalink / raw)
  To: Gao Xiang, Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Miao Xie, weidu.du, Fang Wei, stable

On 2019-8-19 18:34, Gao Xiang wrote:
> As reported by erofs-utils fuzzer, currently, multiref
> (ondisk deduplication) hasn't been supported for now,
> we should forbid it properly.
> 
> Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
> Cc: <stable@vger.kernel.org> # 4.19+
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
> ---
>  drivers/staging/erofs/zdata.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/erofs/zdata.c b/drivers/staging/erofs/zdata.c
> index aae2f2b8353f..5b6fef5181af 100644
> --- a/drivers/staging/erofs/zdata.c
> +++ b/drivers/staging/erofs/zdata.c
> @@ -816,8 +816,16 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
>  			pagenr = z_erofs_onlinepage_index(page);
>  
>  		DBG_BUGON(pagenr >= nr_pages);
> -		DBG_BUGON(pages[pagenr]);
>  
> +		/*
> +		 * currently EROFS doesn't support multiref(dedup),
> +		 * so here erroring out one multiref page.
> +		 */
> +		if (unlikely(pages[pagenr])) {
> +			DBG_BUGON(1);
> +			SetPageError(pages[pagenr]);
> +			z_erofs_onlinepage_endio(pages[pagenr]);

Should set err meanwhile?

> +		}
>  		pages[pagenr] = page;
>  	}
>  	z_erofs_pagevec_ctor_exit(&ctor, true);
> @@ -849,7 +857,11 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
>  			pagenr = z_erofs_onlinepage_index(page);
>  
>  			DBG_BUGON(pagenr >= nr_pages);
> -			DBG_BUGON(pages[pagenr]);
> +			if (unlikely(pages[pagenr])) {
> +				DBG_BUGON(1);
> +				SetPageError(pages[pagenr]);
> +				z_erofs_onlinepage_endio(pages[pagenr]);
> +			}
>  			pages[pagenr] = page;
>  
>  			overlapped = true;
> 

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

* Re: [PATCH 6/6] staging: erofs: avoid endless loop of invalid lookback distance 0
  2019-08-19 10:34                                         ` [PATCH 6/6] staging: erofs: avoid endless loop of invalid lookback distance 0 Gao Xiang
@ 2019-08-19 14:58                                           ` Chao Yu
  0 siblings, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-19 14:58 UTC (permalink / raw)
  To: Gao Xiang, Chao Yu, Greg Kroah-Hartman, devel, linux-fsdevel
  Cc: LKML, linux-erofs, Miao Xie, weidu.du, Fang Wei, stable

On 2019-8-19 18:34, Gao Xiang wrote:
> As reported by erofs-utils fuzzer, Lookback distance should
> be a positive number, so it should be actually looked back
> rather than spinning.
> 
> Fixes: 02827e1796b3 ("staging: erofs: add erofs_map_blocks_iter")
> Cc: <stable@vger.kernel.org> # 4.19+
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-18 20:14                                 ` Gao Xiang
  2019-08-19  7:35                                   ` Richard Weinberger
@ 2019-08-19 16:09                                   ` Darrick J. Wong
  2019-08-19 20:30                                     ` Gao Xiang
  1 sibling, 1 reply; 72+ messages in thread
From: Darrick J. Wong @ 2019-08-19 16:09 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Chao Yu,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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
> > 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-19 16:09                                   ` [PATCH] erofs: move erofs out of staging Darrick J. Wong
@ 2019-08-19 20:30                                     ` Gao Xiang
  2019-08-20  0:55                                       ` Qu Wenruo
  0 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-19 20:30 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Chao Yu,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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
> > > 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-19 20:30                                     ` Gao Xiang
@ 2019-08-20  0:55                                       ` Qu Wenruo
  2019-08-20  1:55                                         ` Gao Xiang
                                                           ` (2 more replies)
  0 siblings, 3 replies; 72+ messages in thread
From: Qu Wenruo @ 2019-08-20  0:55 UTC (permalink / raw)
  To: Gao Xiang, Darrick J. Wong
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Chao Yu,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds


[-- Attachment #1.1: Type: text/plain, Size: 3948 bytes --]

[...]
>>> 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
>>>>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  0:55                                       ` Qu Wenruo
@ 2019-08-20  1:55                                         ` Gao Xiang
  2019-08-20  2:24                                         ` Chao Yu
  2019-08-20  3:33                                         ` Miao Xie
  2 siblings, 0 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-20  1:55 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: Gao Xiang, Darrick J. Wong, Christoph Hellwig,
	Theodore Y. Ts'o, Eric Biggers, Richard Weinberger,
	Greg Kroah-Hartman, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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
> >>>>
> 




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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  0:55                                       ` Qu Wenruo
  2019-08-20  1:55                                         ` Gao Xiang
@ 2019-08-20  2:24                                         ` Chao Yu
  2019-08-20  2:38                                           ` Qu Wenruo
  2019-08-20 15:56                                           ` Theodore Y. Ts'o
  2019-08-20  3:33                                         ` Miao Xie
  2 siblings, 2 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-20  2:24 UTC (permalink / raw)
  To: Qu Wenruo, Gao Xiang, Darrick J. Wong
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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
>>>>>
> 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  2:24                                         ` Chao Yu
@ 2019-08-20  2:38                                           ` Qu Wenruo
  2019-08-20  7:15                                             ` Chao Yu
  2019-08-20 15:56                                           ` Theodore Y. Ts'o
  1 sibling, 1 reply; 72+ messages in thread
From: Qu Wenruo @ 2019-08-20  2:38 UTC (permalink / raw)
  To: Chao Yu, Gao Xiang, Darrick J. Wong
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds


[-- Attachment #1.1: Type: text/plain, Size: 5524 bytes --]



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
>>>>>>
>>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  0:55                                       ` Qu Wenruo
  2019-08-20  1:55                                         ` Gao Xiang
  2019-08-20  2:24                                         ` Chao Yu
@ 2019-08-20  3:33                                         ` Miao Xie
  2019-08-20  3:46                                           ` Gao Xiang
  2019-08-20  6:04                                           ` Qu Wenruo
  2 siblings, 2 replies; 72+ messages in thread
From: Miao Xie @ 2019-08-20  3:33 UTC (permalink / raw)
  To: Qu Wenruo, Gao Xiang, Darrick J. Wong
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Chao Yu,
	Dave Chinner, David Sterba, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds



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
>>>>>
> 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  3:33                                         ` Miao Xie
@ 2019-08-20  3:46                                           ` Gao Xiang
  2019-08-20  6:04                                           ` Qu Wenruo
  1 sibling, 0 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-20  3:46 UTC (permalink / raw)
  To: Miao Xie
  Cc: Qu Wenruo, Gao Xiang, Darrick J. Wong, Christoph Hellwig,
	Theodore Y. Ts'o, Eric Biggers, Richard Weinberger,
	Greg Kroah-Hartman, Jan Kara, Chao Yu, Dave Chinner,
	David Sterba, devel, Stephen Rothwell, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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
> >>>>>
> > 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  3:33                                         ` Miao Xie
  2019-08-20  3:46                                           ` Gao Xiang
@ 2019-08-20  6:04                                           ` Qu Wenruo
  2019-08-20  6:22                                             ` Gao Xiang
  1 sibling, 1 reply; 72+ messages in thread
From: Qu Wenruo @ 2019-08-20  6:04 UTC (permalink / raw)
  To: miaoxie, Gao Xiang, Darrick J. Wong
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Chao Yu,
	Dave Chinner, David Sterba, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds


[-- Attachment #1.1: Type: text/plain, Size: 4557 bytes --]

[...]
>> 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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  6:04                                           ` Qu Wenruo
@ 2019-08-20  6:22                                             ` Gao Xiang
  0 siblings, 0 replies; 72+ messages in thread
From: Gao Xiang @ 2019-08-20  6:22 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: miaoxie, Gao Xiang, Darrick J. Wong, Jan Kara, Christoph Hellwig,
	Amir Goldstein, Dave Chinner, linux-kernel, devel,
	Stephen Rothwell, Richard Weinberger, Eric Biggers, Chao Yu,
	linux-erofs, Al Viro, Jaegeuk Kim, Theodore Y. Ts'o,
	Greg Kroah-Hartman, David Sterba, Li Guifu, Fang Wei,
	Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  2:38                                           ` Qu Wenruo
@ 2019-08-20  7:15                                             ` Chao Yu
  2019-08-20  8:46                                               ` Qu Wenruo
  0 siblings, 1 reply; 72+ messages in thread
From: Chao Yu @ 2019-08-20  7:15 UTC (permalink / raw)
  To: Qu Wenruo, Gao Xiang, Darrick J. Wong
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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
>>>>>>>
>>>
> 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  7:15                                             ` Chao Yu
@ 2019-08-20  8:46                                               ` Qu Wenruo
  2019-08-21  2:12                                                 ` Chao Yu
  0 siblings, 1 reply; 72+ messages in thread
From: Qu Wenruo @ 2019-08-20  8:46 UTC (permalink / raw)
  To: Chao Yu, Gao Xiang, Darrick J. Wong
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds


[-- Attachment #1.1: Type: text/plain, Size: 1975 bytes --]

[...]
> 
> 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,
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  2:24                                         ` Chao Yu
  2019-08-20  2:38                                           ` Qu Wenruo
@ 2019-08-20 15:56                                           ` Theodore Y. Ts'o
  2019-08-20 16:35                                             ` Gao Xiang
  2019-08-21  1:34                                             ` Chao Yu
  1 sibling, 2 replies; 72+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-20 15:56 UTC (permalink / raw)
  To: Chao Yu
  Cc: Qu Wenruo, Gao Xiang, Darrick J. Wong, Christoph Hellwig,
	Eric Biggers, Richard Weinberger, Greg Kroah-Hartman, Jan Kara,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20 15:56                                           ` Theodore Y. Ts'o
@ 2019-08-20 16:35                                             ` Gao Xiang
  2019-08-21  0:51                                               ` Theodore Y. Ts'o
  2019-08-21  1:34                                             ` Chao Yu
  1 sibling, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-20 16:35 UTC (permalink / raw)
  To: Theodore Y. Ts'o
  Cc: Chao Yu, Qu Wenruo, Darrick J. Wong, Christoph Hellwig,
	Eric Biggers, Richard Weinberger, Greg Kroah-Hartman, Jan Kara,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20 16:35                                             ` Gao Xiang
@ 2019-08-21  0:51                                               ` Theodore Y. Ts'o
  0 siblings, 0 replies; 72+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-21  0:51 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Chao Yu, Qu Wenruo, Darrick J. Wong, Christoph Hellwig,
	Eric Biggers, Richard Weinberger, Greg Kroah-Hartman, Jan Kara,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20 15:56                                           ` Theodore Y. Ts'o
  2019-08-20 16:35                                             ` Gao Xiang
@ 2019-08-21  1:34                                             ` Chao Yu
  2019-08-21  1:48                                               ` Darrick J. Wong
  1 sibling, 1 reply; 72+ messages in thread
From: Chao Yu @ 2019-08-21  1:34 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Qu Wenruo, Gao Xiang, Darrick J. Wong,
	Christoph Hellwig, Eric Biggers, Richard Weinberger,
	Greg Kroah-Hartman, Jan Kara, Dave Chinner, David Sterba,
	Miao Xie, devel, Stephen Rothwell, Amir Goldstein, linux-erofs,
	Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu, Fang Wei,
	Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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

> 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-21  1:34                                             ` Chao Yu
@ 2019-08-21  1:48                                               ` Darrick J. Wong
  2019-08-21  1:57                                                 ` Chao Yu
  0 siblings, 1 reply; 72+ messages in thread
From: Darrick J. Wong @ 2019-08-21  1:48 UTC (permalink / raw)
  To: Chao Yu
  Cc: Theodore Y. Ts'o, Qu Wenruo, Gao Xiang, Christoph Hellwig,
	Eric Biggers, Richard Weinberger, Greg Kroah-Hartman, Jan Kara,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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
> 
> > 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-21  1:48                                               ` Darrick J. Wong
@ 2019-08-21  1:57                                                 ` Chao Yu
  0 siblings, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-21  1:57 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Theodore Y. Ts'o, Qu Wenruo, Gao Xiang, Christoph Hellwig,
	Eric Biggers, Richard Weinberger, Greg Kroah-Hartman, Jan Kara,
	Dave Chinner, David Sterba, Miao Xie, devel, Stephen Rothwell,
	Amir Goldstein, linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel,
	Li Guifu, Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton,
	torvalds

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
>>
>>>
> .
> 

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

* Re: [PATCH] erofs: move erofs out of staging
  2019-08-20  8:46                                               ` Qu Wenruo
@ 2019-08-21  2:12                                                 ` Chao Yu
  0 siblings, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-21  2:12 UTC (permalink / raw)
  To: Qu Wenruo, Gao Xiang, Darrick J. Wong
  Cc: Christoph Hellwig, Theodore Y. Ts'o, Eric Biggers,
	Richard Weinberger, Greg Kroah-Hartman, Jan Kara, Dave Chinner,
	David Sterba, Miao Xie, devel, Stephen Rothwell, Amir Goldstein,
	linux-erofs, Al Viro, Jaegeuk Kim, linux-kernel, Li Guifu,
	Fang Wei, Pavel Machek, linux-fsdevel, Andrew Morton, torvalds

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,
>>
> 

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

* Re: [PATCH 5/6] staging: erofs: detect potential multiref due to corrupted images
  2019-08-19 14:57                                           ` Chao Yu
@ 2019-08-21  2:19                                             ` Greg Kroah-Hartman
  2019-08-21 14:01                                               ` [PATCH v2 " Gao Xiang
  0 siblings, 1 reply; 72+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-21  2:19 UTC (permalink / raw)
  To: Chao Yu
  Cc: Gao Xiang, Chao Yu, devel, linux-fsdevel, LKML, linux-erofs,
	Miao Xie, weidu.du, Fang Wei, stable

On Mon, Aug 19, 2019 at 10:57:42PM +0800, Chao Yu wrote:
> On 2019-8-19 18:34, Gao Xiang wrote:
> > As reported by erofs-utils fuzzer, currently, multiref
> > (ondisk deduplication) hasn't been supported for now,
> > we should forbid it properly.
> > 
> > Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
> > Cc: <stable@vger.kernel.org> # 4.19+
> > Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
> > ---
> >  drivers/staging/erofs/zdata.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/erofs/zdata.c b/drivers/staging/erofs/zdata.c
> > index aae2f2b8353f..5b6fef5181af 100644
> > --- a/drivers/staging/erofs/zdata.c
> > +++ b/drivers/staging/erofs/zdata.c
> > @@ -816,8 +816,16 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
> >  			pagenr = z_erofs_onlinepage_index(page);
> >  
> >  		DBG_BUGON(pagenr >= nr_pages);
> > -		DBG_BUGON(pages[pagenr]);
> >  
> > +		/*
> > +		 * currently EROFS doesn't support multiref(dedup),
> > +		 * so here erroring out one multiref page.
> > +		 */
> > +		if (unlikely(pages[pagenr])) {
> > +			DBG_BUGON(1);
> > +			SetPageError(pages[pagenr]);
> > +			z_erofs_onlinepage_endio(pages[pagenr]);
> 
> Should set err meanwhile?

I've skipped this patch in this series for now, and applied the rest.

thanks,

greg k-h

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

* [PATCH v2 5/6] staging: erofs: detect potential multiref due to corrupted images
  2019-08-21  2:19                                             ` Greg Kroah-Hartman
@ 2019-08-21 14:01                                               ` Gao Xiang
  2019-08-21 14:24                                                 ` Chao Yu
  0 siblings, 1 reply; 72+ messages in thread
From: Gao Xiang @ 2019-08-21 14:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Chao Yu, devel, Miao Xie, LKML, weidu.du, linux-fsdevel,
	linux-erofs, Gao Xiang, stable

As reported by erofs-utils fuzzer, currently, multiref
(ondisk deduplication) hasn't been supported for now,
we should forbid it properly.

Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
Cc: <stable@vger.kernel.org> # 4.19+
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---

changelog from v1:
 - change err = -EFSCORRUPTED as well as Chao suggested;
   [ the difference between adding err or not to [PATCH 5/6] is just whether
     we error out the whole compressed cluster or partial of them (since some
     pages could be decompressed successfully), it's an undefined behavior
     for these corrupted compressed images... ]

Hi Chao,
 Could you kindly review it again? Thanks!

Hi Greg,
 This is [PATCH 5/6] of the original patchset, and I fix as what Chao suggested...
 But I'm not sure whether it should be merged right now, it is up to you. :)

Thanks,
Gao Xiang


 drivers/staging/erofs/zdata.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/erofs/zdata.c b/drivers/staging/erofs/zdata.c
index 4d6faaab04f5..60d7c20db87d 100644
--- a/drivers/staging/erofs/zdata.c
+++ b/drivers/staging/erofs/zdata.c
@@ -798,6 +798,7 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
 	for (i = 0; i < nr_pages; ++i)
 		pages[i] = NULL;
 
+	err = 0;
 	z_erofs_pagevec_ctor_init(&ctor, Z_EROFS_NR_INLINE_PAGEVECS,
 				  cl->pagevec, 0);
 
@@ -819,8 +820,17 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
 			pagenr = z_erofs_onlinepage_index(page);
 
 		DBG_BUGON(pagenr >= nr_pages);
-		DBG_BUGON(pages[pagenr]);
 
+		/*
+		 * currently EROFS doesn't support multiref(dedup),
+		 * so here erroring out one multiref page.
+		 */
+		if (unlikely(pages[pagenr])) {
+			DBG_BUGON(1);
+			SetPageError(pages[pagenr]);
+			z_erofs_onlinepage_endio(pages[pagenr]);
+			err = -EFSCORRUPTED;
+		}
 		pages[pagenr] = page;
 	}
 	z_erofs_pagevec_ctor_exit(&ctor, true);
@@ -828,7 +838,6 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
 	overlapped = false;
 	compressed_pages = pcl->compressed_pages;
 
-	err = 0;
 	for (i = 0; i < clusterpages; ++i) {
 		unsigned int pagenr;
 
@@ -852,7 +861,12 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
 			pagenr = z_erofs_onlinepage_index(page);
 
 			DBG_BUGON(pagenr >= nr_pages);
-			DBG_BUGON(pages[pagenr]);
+			if (unlikely(pages[pagenr])) {
+				DBG_BUGON(1);
+				SetPageError(pages[pagenr]);
+				z_erofs_onlinepage_endio(pages[pagenr]);
+				err = -EFSCORRUPTED;
+			}
 			pages[pagenr] = page;
 
 			overlapped = true;
-- 
2.17.1


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

* Re: [PATCH v2 5/6] staging: erofs: detect potential multiref due to corrupted images
  2019-08-21 14:01                                               ` [PATCH v2 " Gao Xiang
@ 2019-08-21 14:24                                                 ` Chao Yu
  0 siblings, 0 replies; 72+ messages in thread
From: Chao Yu @ 2019-08-21 14:24 UTC (permalink / raw)
  To: Gao Xiang, Greg Kroah-Hartman
  Cc: devel, Miao Xie, LKML, weidu.du, linux-fsdevel, linux-erofs, stable

On 2019-8-21 22:01, Gao Xiang wrote:
> As reported by erofs-utils fuzzer, currently, multiref
> (ondisk deduplication) hasn't been supported for now,
> we should forbid it properly.
> 
> Fixes: 3883a79abd02 ("staging: erofs: introduce VLE decompression support")
> Cc: <stable@vger.kernel.org> # 4.19+
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

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

end of thread, other threads:[~2019-08-21 14:24 UTC | newest]

Thread overview: 72+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-17  8:23 [PATCH] erofs: move erofs out of staging Gao Xiang
2019-08-17 21:19 ` Richard Weinberger
2019-08-17 22:07   ` Gao Xiang
2019-08-17 23:25     ` Richard Weinberger
2019-08-17 23:38       ` Gao Xiang
2019-08-18  0:04         ` Gao Xiang
2019-08-18  0:52           ` Gao Xiang
2019-08-18  8:16         ` Richard Weinberger
2019-08-18  8:45           ` Gao Xiang
2019-08-18  9:03             ` Richard Weinberger
2019-08-18  9:09               ` Greg Kroah-Hartman
2019-08-18  9:21                 ` Richard Weinberger
2019-08-18 10:12                   ` Chao Yu
2019-08-18 15:11                   ` Theodore Y. Ts'o
2019-08-18 15:58                     ` Christoph Hellwig
2019-08-18 16:16                       ` Eric Biggers
2019-08-18 16:22                         ` Christoph Hellwig
2019-08-18 16:33                           ` Gao Xiang
2019-08-18 17:29                           ` Eric Biggers
2019-08-18 17:47                             ` Christoph Hellwig
2019-08-18 18:16                               ` Gao Xiang
2019-08-18 20:14                                 ` Gao Xiang
2019-08-19  7:35                                   ` Richard Weinberger
2019-08-19  8:02                                     ` Gao Xiang
2019-08-19 10:34                                       ` [PATCH 0/6] staging: erofs: first stage of corrupted compressed images Gao Xiang
2019-08-19 10:34                                         ` [PATCH 1/6] staging: erofs: some compressed cluster should be submitted for corrupted images Gao Xiang
2019-08-19 14:36                                           ` Chao Yu
2019-08-19 14:39                                           ` Chao Yu
2019-08-19 10:34                                         ` [PATCH 2/6] staging: erofs: cannot set EROFS_V_Z_INITED_BIT if fill_inode_lazy fails Gao Xiang
2019-08-19 14:43                                           ` Chao Yu
2019-08-19 10:34                                         ` [PATCH 3/6] staging: erofs: add two missing erofs_workgroup_put for corrupted images Gao Xiang
2019-08-19 14:40                                           ` Chao Yu
2019-08-19 10:34                                         ` [PATCH 4/6] staging: erofs: avoid loop in submit chains Gao Xiang
2019-08-19 14:50                                           ` Chao Yu
2019-08-19 10:34                                         ` [PATCH 5/6] staging: erofs: detect potential multiref due to corrupted images Gao Xiang
2019-08-19 14:57                                           ` Chao Yu
2019-08-21  2:19                                             ` Greg Kroah-Hartman
2019-08-21 14:01                                               ` [PATCH v2 " Gao Xiang
2019-08-21 14:24                                                 ` Chao Yu
2019-08-19 10:34                                         ` [PATCH 6/6] staging: erofs: avoid endless loop of invalid lookback distance 0 Gao Xiang
2019-08-19 14:58                                           ` Chao Yu
2019-08-19 16:09                                   ` [PATCH] erofs: move erofs out of staging Darrick J. Wong
2019-08-19 20:30                                     ` Gao Xiang
2019-08-20  0:55                                       ` Qu Wenruo
2019-08-20  1:55                                         ` Gao Xiang
2019-08-20  2:24                                         ` Chao Yu
2019-08-20  2:38                                           ` Qu Wenruo
2019-08-20  7:15                                             ` Chao Yu
2019-08-20  8:46                                               ` Qu Wenruo
2019-08-21  2:12                                                 ` Chao Yu
2019-08-20 15:56                                           ` Theodore Y. Ts'o
2019-08-20 16:35                                             ` Gao Xiang
2019-08-21  0:51                                               ` Theodore Y. Ts'o
2019-08-21  1:34                                             ` Chao Yu
2019-08-21  1:48                                               ` Darrick J. Wong
2019-08-21  1:57                                                 ` Chao Yu
2019-08-20  3:33                                         ` Miao Xie
2019-08-20  3:46                                           ` Gao Xiang
2019-08-20  6:04                                           ` Qu Wenruo
2019-08-20  6:22                                             ` Gao Xiang
2019-08-19  7:37                               ` Richard Weinberger
2019-08-18 17:43                       ` Theodore Y. Ts'o
2019-08-18 16:03                     ` Gao Xiang
2019-08-18 17:06                     ` Richard Weinberger
2019-08-18 17:46                       ` Theodore Y. Ts'o
2019-08-18 18:00                         ` Richard Weinberger
2019-08-18 18:31                           ` Gao Xiang
2019-08-18  9:28               ` Gao Xiang
2019-08-19  5:28                 ` [PATCH] erofs: Use common kernel logging style Joe Perches
2019-08-19  5:52                   ` Gao Xiang
2019-08-19  5:47                     ` Joe Perches
2019-08-19  6:08                       ` Gao Xiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).