From 7296c8af6a341e30b517afc1ccd107cf10926d03 Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Mon, 20 Sep 2021 09:05:55 +0200 Subject: [PATCH 01/11] ubifs: Fix spelling mistakes Found with `codespell -i 3 -w fs/ubifs/**` and proof reading that parts. Signed-off-by: Alexander Dahl Signed-off-by: Richard Weinberger --- fs/ubifs/dir.c | 4 ++-- fs/ubifs/replay.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 7c61d0ec0159..dbe72f664abf 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -1207,7 +1207,7 @@ static int ubifs_symlink(struct user_namespace *mnt_userns, struct inode *dir, * @inode1: first inode * @inode2: second inode * @inode3: third inode - * @inode4: fouth inode + * @inode4: fourth inode * * This function is used for 'ubifs_rename()' and @inode1 may be the same as * @inode2 whereas @inode3 and @inode4 may be %NULL. @@ -1233,7 +1233,7 @@ static void lock_4_inodes(struct inode *inode1, struct inode *inode2, * @inode1: first inode * @inode2: second inode * @inode3: third inode - * @inode4: fouth inode + * @inode4: fourth inode */ static void unlock_4_inodes(struct inode *inode1, struct inode *inode2, struct inode *inode3, struct inode *inode4) diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c index 5260d3e531bb..4211e4456b1e 100644 --- a/fs/ubifs/replay.c +++ b/fs/ubifs/replay.c @@ -106,7 +106,7 @@ static int set_bud_lprops(struct ubifs_info *c, struct bud_entry *b) * property values should be @lp->free == @c->leb_size and * @lp->dirty == 0, but that is not the case. The reason is that * the LEB had been garbage collected before it became the bud, - * and there was not commit inbetween. The garbage collector + * and there was no commit in between. The garbage collector * resets the free and dirty space without recording it * anywhere except lprops, so if there was no commit then * lprops does not have that information. From bc7849e280434289936b5cbe1b3701336741aba9 Mon Sep 17 00:00:00 2001 From: Kai Song Date: Tue, 5 Oct 2021 14:56:55 +0800 Subject: [PATCH 02/11] ubi: Fix a mistake in comment Fixes: 2a734bb8d502 ("UBI: use debugfs for the extra checks knobs") There is a mistake in docstrings, it should be ubi_debugfs_exit_dev instead of dbg_debug_exit_dev. Signed-off-by: Kai Song Signed-off-by: Richard Weinberger --- drivers/mtd/ubi/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c index 3c0c8eca4d51..31d427ee191a 100644 --- a/drivers/mtd/ubi/debug.c +++ b/drivers/mtd/ubi/debug.c @@ -562,7 +562,7 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi) } /** - * dbg_debug_exit_dev - free all debugfs files corresponding to device @ubi + * ubi_debugfs_exit_dev - free all debugfs files corresponding to device @ubi * @ubi: UBI device description object */ void ubi_debugfs_exit_dev(struct ubi_device *ubi) From d98c6c35c881b944933edd1bbc438aa1f12e3c47 Mon Sep 17 00:00:00 2001 From: Cai Huoqing Date: Thu, 21 Oct 2021 16:43:43 +0800 Subject: [PATCH 03/11] ubifs: Make use of the helper macro kthread_run() Repalce kthread_create/wake_up_process() with kthread_run() to simplify the code. Signed-off-by: Cai Huoqing Signed-off-by: Richard Weinberger --- fs/ubifs/super.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index f0fb25727d96..0e7f206c43cf 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1367,7 +1367,7 @@ static int mount_ubifs(struct ubifs_info *c) sprintf(c->bgt_name, BGT_NAME_PATTERN, c->vi.ubi_num, c->vi.vol_id); if (!c->ro_mount) { /* Create background thread */ - c->bgt = kthread_create(ubifs_bg_thread, c, "%s", c->bgt_name); + c->bgt = kthread_run(ubifs_bg_thread, c, "%s", c->bgt_name); if (IS_ERR(c->bgt)) { err = PTR_ERR(c->bgt); c->bgt = NULL; @@ -1375,7 +1375,6 @@ static int mount_ubifs(struct ubifs_info *c) c->bgt_name, err); goto out_wbufs; } - wake_up_process(c->bgt); } err = ubifs_read_master(c); @@ -1780,7 +1779,7 @@ static int ubifs_remount_rw(struct ubifs_info *c) goto out; /* Create background thread */ - c->bgt = kthread_create(ubifs_bg_thread, c, "%s", c->bgt_name); + c->bgt = kthread_run(ubifs_bg_thread, c, "%s", c->bgt_name); if (IS_ERR(c->bgt)) { err = PTR_ERR(c->bgt); c->bgt = NULL; @@ -1788,7 +1787,6 @@ static int ubifs_remount_rw(struct ubifs_info *c) c->bgt_name, err); goto out; } - wake_up_process(c->bgt); c->orph_buf = vmalloc(c->leb_size); if (!c->orph_buf) { From 3fea4d9d160186617ff40490ae01f4f4f36b28ff Mon Sep 17 00:00:00 2001 From: Petr Cvachoucek Date: Mon, 30 Aug 2021 21:20:37 +0200 Subject: [PATCH 04/11] ubifs: Error path in ubifs_remount_rw() seems to wrongly free write buffers it seems freeing the write buffers in the error path of the ubifs_remount_rw() is wrong. It leads later to a kernel oops like this: [10016.431274] UBIFS (ubi0:0): start fixing up free space [10090.810042] UBIFS (ubi0:0): free space fixup complete [10090.814623] UBIFS error (ubi0:0 pid 512): ubifs_remount_fs: cannot spawn "ubifs_bgt0_0", error -4 [10101.915108] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" started, PID 517 [10105.275498] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000030 [10105.284352] Mem abort info: [10105.287160] ESR = 0x96000006 [10105.290252] EC = 0x25: DABT (current EL), IL = 32 bits [10105.295592] SET = 0, FnV = 0 [10105.298652] EA = 0, S1PTW = 0 [10105.301848] Data abort info: [10105.304723] ISV = 0, ISS = 0x00000006 [10105.308573] CM = 0, WnR = 0 [10105.311564] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000f03d1000 [10105.318034] [0000000000000030] pgd=00000000f6cee003, pud=00000000f4884003, pmd=0000000000000000 [10105.326783] Internal error: Oops: 96000006 [#1] PREEMPT SMP [10105.332355] Modules linked in: ath10k_pci ath10k_core ath mac80211 libarc4 cfg80211 nvme nvme_core cryptodev(O) [10105.342468] CPU: 3 PID: 518 Comm: touch Tainted: G O 5.4.3 #1 [10105.349517] Hardware name: HYPEX CPU (DT) [10105.353525] pstate: 40000005 (nZcv daif -PAN -UAO) [10105.358324] pc : atomic64_try_cmpxchg_acquire.constprop.22+0x8/0x34 [10105.364596] lr : mutex_lock+0x1c/0x34 [10105.368253] sp : ffff000075633aa0 [10105.371563] x29: ffff000075633aa0 x28: 0000000000000001 [10105.376874] x27: ffff000076fa80c8 x26: 0000000000000004 [10105.382185] x25: 0000000000000030 x24: 0000000000000000 [10105.387495] x23: 0000000000000000 x22: 0000000000000038 [10105.392807] x21: 000000000000000c x20: ffff000076fa80c8 [10105.398119] x19: ffff000076fa8000 x18: 0000000000000000 [10105.403429] x17: 0000000000000000 x16: 0000000000000000 [10105.408741] x15: 0000000000000000 x14: fefefefefefefeff [10105.414052] x13: 0000000000000000 x12: 0000000000000fe0 [10105.419364] x11: 0000000000000fe0 x10: ffff000076709020 [10105.424675] x9 : 0000000000000000 x8 : 00000000000000a0 [10105.429986] x7 : ffff000076fa80f4 x6 : 0000000000000030 [10105.435297] x5 : 0000000000000000 x4 : 0000000000000000 [10105.440609] x3 : 0000000000000000 x2 : ffff00006f276040 [10105.445920] x1 : ffff000075633ab8 x0 : 0000000000000030 [10105.451232] Call trace: [10105.453676] atomic64_try_cmpxchg_acquire.constprop.22+0x8/0x34 [10105.459600] ubifs_garbage_collect+0xb4/0x334 [10105.463956] ubifs_budget_space+0x398/0x458 [10105.468139] ubifs_create+0x50/0x180 [10105.471712] path_openat+0x6a0/0x9b0 [10105.475284] do_filp_open+0x34/0x7c [10105.478771] do_sys_open+0x78/0xe4 [10105.482170] __arm64_sys_openat+0x1c/0x24 [10105.486180] el0_svc_handler+0x84/0xc8 [10105.489928] el0_svc+0x8/0xc [10105.492808] Code: 52800013 17fffffb d2800003 f9800011 (c85ffc05) [10105.498903] ---[ end trace 46b721d93267a586 ]--- To reproduce the problem: 1. Filesystem initially mounted read-only, free space fixup flag set. 2. mount -o remount,rw 3. it takes some time (free space fixup running) ... try to terminate running mount by CTRL-C ... does not respond, only after free space fixup is complete ... then "ubifs_remount_fs: cannot spawn "ubifs_bgt0_0", error -4" 4. mount -o remount,rw ... now finished instantly (fixup already done). 5. Create file or just unmount the filesystem and we get the oops. Cc: Fixes: b50b9f408502 ("UBIFS: do not free write-buffers when in R/O mode") Signed-off-by: Petr Cvachoucek Signed-off-by: Richard Weinberger --- fs/ubifs/super.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 0e7f206c43cf..fbec8fcc9a3c 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1851,7 +1851,6 @@ static int ubifs_remount_rw(struct ubifs_info *c) kthread_stop(c->bgt); c->bgt = NULL; } - free_wbufs(c); kfree(c->write_reserve_buf); c->write_reserve_buf = NULL; vfree(c->ileb_buf); From 2e3cbf425804fb44a005e252f88f93dff108c911 Mon Sep 17 00:00:00 2001 From: Stefan Schaeckeler Date: Sat, 9 Oct 2021 21:22:39 -0700 Subject: [PATCH 05/11] ubifs: Export filesystem error counters Not all ubifs filesystem errors are propagated to userspace. Export bad magic, bad node and crc errors via sysfs. This allows userspace to notice filesystem errors: /sys/fs/ubifs/ubiX_Y/errors_magic /sys/fs/ubifs/ubiX_Y/errors_node /sys/fs/ubifs/ubiX_Y/errors_crc The counters are reset to 0 with a remount. Signed-off-by: Stefan Schaeckeler Signed-off-by: Richard Weinberger --- fs/ubifs/Makefile | 2 +- fs/ubifs/io.c | 21 +++++++ fs/ubifs/super.c | 16 ++++- fs/ubifs/sysfs.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++ fs/ubifs/ubifs.h | 35 +++++++++++ 5 files changed, 225 insertions(+), 2 deletions(-) create mode 100644 fs/ubifs/sysfs.c diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile index 5c4b845754a7..314c80b24a76 100644 --- a/fs/ubifs/Makefile +++ b/fs/ubifs/Makefile @@ -5,7 +5,7 @@ ubifs-y += shrinker.o journal.o file.o dir.o super.o sb.o io.o ubifs-y += tnc.o master.o scan.o replay.o log.o commit.o gc.o orphan.o ubifs-y += budget.o find.o tnc_commit.o compress.o lpt.o lprops.o ubifs-y += recovery.o ioctl.o lpt_commit.o tnc_misc.o debug.o -ubifs-y += misc.o +ubifs-y += misc.o sysfs.o ubifs-$(CONFIG_FS_ENCRYPTION) += crypto.o ubifs-$(CONFIG_UBIFS_FS_XATTR) += xattr.o ubifs-$(CONFIG_UBIFS_FS_AUTHENTICATION) += auth.o diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c index 00b61dba62b7..789a7813f3fa 100644 --- a/fs/ubifs/io.c +++ b/fs/ubifs/io.c @@ -194,6 +194,24 @@ int ubifs_is_mapped(const struct ubifs_info *c, int lnum) return err; } +static void record_magic_error(struct ubifs_stats_info *stats) +{ + if (stats) + stats->magic_errors++; +} + +static void record_node_error(struct ubifs_stats_info *stats) +{ + if (stats) + stats->node_errors++; +} + +static void record_crc_error(struct ubifs_stats_info *stats) +{ + if (stats) + stats->crc_errors++; +} + /** * ubifs_check_node - check node. * @c: UBIFS file-system description object @@ -238,6 +256,7 @@ int ubifs_check_node(const struct ubifs_info *c, const void *buf, int len, if (!quiet) ubifs_err(c, "bad magic %#08x, expected %#08x", magic, UBIFS_NODE_MAGIC); + record_magic_error(c->stats); err = -EUCLEAN; goto out; } @@ -246,6 +265,7 @@ int ubifs_check_node(const struct ubifs_info *c, const void *buf, int len, if (type < 0 || type >= UBIFS_NODE_TYPES_CNT) { if (!quiet) ubifs_err(c, "bad node type %d", type); + record_node_error(c->stats); goto out; } @@ -270,6 +290,7 @@ int ubifs_check_node(const struct ubifs_info *c, const void *buf, int len, if (!quiet) ubifs_err(c, "bad CRC: calculated %#08x, read %#08x", crc, node_crc); + record_crc_error(c->stats); err = -EUCLEAN; goto out; } diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index fbec8fcc9a3c..aa7a1381c457 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1264,6 +1264,10 @@ static int mount_ubifs(struct ubifs_info *c) if (err) return err; + err = ubifs_sysfs_register(c); + if (err) + goto out_debugging; + err = check_volume_empty(c); if (err) goto out_free; @@ -1640,6 +1644,8 @@ static int mount_ubifs(struct ubifs_info *c) vfree(c->sbuf); kfree(c->bottom_up_buf); kfree(c->sup_node); + ubifs_sysfs_unregister(c); +out_debugging: ubifs_debugging_exit(c); return err; } @@ -1683,6 +1689,7 @@ static void ubifs_umount(struct ubifs_info *c) kfree(c->bottom_up_buf); kfree(c->sup_node); ubifs_debugging_exit(c); + ubifs_sysfs_unregister(c); } /** @@ -2433,14 +2440,20 @@ static int __init ubifs_init(void) dbg_debugfs_init(); + err = ubifs_sysfs_init(); + if (err) + goto out_dbg; + err = register_filesystem(&ubifs_fs_type); if (err) { pr_err("UBIFS error (pid %d): cannot register file system, error %d", current->pid, err); - goto out_dbg; + goto out_sysfs; } return 0; +out_sysfs: + ubifs_sysfs_exit(); out_dbg: dbg_debugfs_exit(); ubifs_compressors_exit(); @@ -2459,6 +2472,7 @@ static void __exit ubifs_exit(void) WARN_ON(atomic_long_read(&ubifs_clean_zn_cnt) != 0); dbg_debugfs_exit(); + ubifs_sysfs_exit(); ubifs_compressors_exit(); unregister_shrinker(&ubifs_shrinker_info); diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c new file mode 100644 index 000000000000..0eb3d7d12450 --- /dev/null +++ b/fs/ubifs/sysfs.c @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * This file is part of UBIFS. + * + * Copyright (C) 2021 Cisco Systems + * + * Author: Stefan Schaeckeler + */ + + +#include +#include "ubifs.h" + +enum attr_id_t { + attr_errors_magic, + attr_errors_node, + attr_errors_crc, +}; + +struct ubifs_attr { + struct attribute attr; + enum attr_id_t attr_id; +}; + +#define UBIFS_ATTR(_name, _mode, _id) \ +static struct ubifs_attr ubifs_attr_##_name = { \ + .attr = {.name = __stringify(_name), .mode = _mode }, \ + .attr_id = attr_##_id, \ +} + +#define UBIFS_ATTR_FUNC(_name, _mode) UBIFS_ATTR(_name, _mode, _name) + +UBIFS_ATTR_FUNC(errors_magic, 0444); +UBIFS_ATTR_FUNC(errors_crc, 0444); +UBIFS_ATTR_FUNC(errors_node, 0444); + +#define ATTR_LIST(name) (&ubifs_attr_##name.attr) + +static struct attribute *ubifs_attrs[] = { + ATTR_LIST(errors_magic), + ATTR_LIST(errors_node), + ATTR_LIST(errors_crc), + NULL, +}; + +static ssize_t ubifs_attr_show(struct kobject *kobj, + struct attribute *attr, char *buf) +{ + struct ubifs_info *sbi = container_of(kobj, struct ubifs_info, + kobj); + + struct ubifs_attr *a = container_of(attr, struct ubifs_attr, attr); + + switch (a->attr_id) { + case attr_errors_magic: + return sysfs_emit(buf, "%u\n", sbi->stats->magic_errors); + case attr_errors_node: + return sysfs_emit(buf, "%u\n", sbi->stats->node_errors); + case attr_errors_crc: + return sysfs_emit(buf, "%u\n", sbi->stats->crc_errors); + } + return 0; +}; + +static void ubifs_sb_release(struct kobject *kobj) +{ + struct ubifs_info *c = container_of(kobj, struct ubifs_info, kobj); + + complete(&c->kobj_unregister); +} + +static const struct sysfs_ops ubifs_attr_ops = { + .show = ubifs_attr_show, +}; + +static struct kobj_type ubifs_sb_ktype = { + .default_attrs = ubifs_attrs, + .sysfs_ops = &ubifs_attr_ops, + .release = ubifs_sb_release, +}; + +static struct kobj_type ubifs_ktype = { + .sysfs_ops = &ubifs_attr_ops, +}; + +static struct kset ubifs_kset = { + .kobj = {.ktype = &ubifs_ktype}, +}; + +int ubifs_sysfs_register(struct ubifs_info *c) +{ + int ret, n; + char dfs_dir_name[UBIFS_DFS_DIR_LEN+1]; + + c->stats = kzalloc(sizeof(struct ubifs_stats_info), GFP_KERNEL); + if (!c->stats) { + ret = -ENOMEM; + goto out_last; + } + n = snprintf(dfs_dir_name, UBIFS_DFS_DIR_LEN + 1, UBIFS_DFS_DIR_NAME, + c->vi.ubi_num, c->vi.vol_id); + + if (n == UBIFS_DFS_DIR_LEN) { + /* The array size is too small */ + ret = -EINVAL; + goto out_free; + } + + c->kobj.kset = &ubifs_kset; + init_completion(&c->kobj_unregister); + + ret = kobject_init_and_add(&c->kobj, &ubifs_sb_ktype, NULL, + "%s", dfs_dir_name); + if (ret) + goto out_put; + + return 0; + +out_put: + kobject_put(&c->kobj); + wait_for_completion(&c->kobj_unregister); +out_free: + kfree(c->stats); +out_last: + ubifs_err(c, "cannot create sysfs entry for ubifs%d_%d, error %d\n", + c->vi.ubi_num, c->vi.vol_id, ret); + return ret; +} + +void ubifs_sysfs_unregister(struct ubifs_info *c) +{ + kobject_del(&c->kobj); + kobject_put(&c->kobj); + wait_for_completion(&c->kobj_unregister); + + kfree(c->stats); +} + +int __init ubifs_sysfs_init(void) +{ + int ret; + + kobject_set_name(&ubifs_kset.kobj, "ubifs"); + ubifs_kset.kobj.parent = fs_kobj; + ret = kset_register(&ubifs_kset); + + return ret; +} + +void ubifs_sysfs_exit(void) +{ + kset_unregister(&ubifs_kset); +} diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index c38066ce9ab0..f55828c0a300 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include #include #include @@ -155,6 +157,13 @@ #define UBIFS_HMAC_ARR_SZ 0 #endif +/* + * The UBIFS sysfs directory name pattern and maximum name length (3 for "ubi" + * + 1 for "_" and plus 2x2 for 2 UBI numbers and 1 for the trailing zero byte. + */ +#define UBIFS_DFS_DIR_NAME "ubi%d_%d" +#define UBIFS_DFS_DIR_LEN (3 + 1 + 2*2 + 1) + /* * Lockdep classes for UBIFS inode @ui_mutex. */ @@ -990,6 +999,18 @@ struct ubifs_budg_info { int dent_budget; }; +/** + * ubifs_stats_info - per-FS statistics information. + * @magic_errors: number of bad magic numbers (will be reset with a new mount). + * @node_errors: number of bad nodes (will be reset with a new mount). + * @crc_errors: number of bad crcs (will be reset with a new mount). + */ +struct ubifs_stats_info { + unsigned int magic_errors; + unsigned int node_errors; + unsigned int crc_errors; +}; + struct ubifs_debug_info; /** @@ -1251,6 +1272,10 @@ struct ubifs_debug_info; * @mount_opts: UBIFS-specific mount options * * @dbg: debugging-related information + * @stats: statistics exported over sysfs + * + * @kobj: kobject for /sys/fs/ubifs/ + * @kobj_unregister: completion to unregister sysfs kobject */ struct ubifs_info { struct super_block *vfs_sb; @@ -1286,6 +1311,9 @@ struct ubifs_info { spinlock_t cs_lock; wait_queue_head_t cmt_wq; + struct kobject kobj; + struct completion kobj_unregister; + unsigned int big_lpt:1; unsigned int space_fixup:1; unsigned int double_hash:1; @@ -1493,6 +1521,7 @@ struct ubifs_info { struct ubifs_mount_opts mount_opts; struct ubifs_debug_info *dbg; + struct ubifs_stats_info *stats; }; extern struct list_head ubifs_infos; @@ -2072,6 +2101,12 @@ void ubifs_compress(const struct ubifs_info *c, const void *in_buf, int in_len, int ubifs_decompress(const struct ubifs_info *c, const void *buf, int len, void *out, int *out_len, int compr_type); +/* sysfs.c */ +int ubifs_sysfs_init(void); +void ubifs_sysfs_exit(void); +int ubifs_sysfs_register(struct ubifs_info *c); +void ubifs_sysfs_unregister(struct ubifs_info *c); + #include "debug.h" #include "misc.h" #include "key.h" From 58225631cf9ae45f98ea04b38b7986e3e7646ee2 Mon Sep 17 00:00:00 2001 From: Stefan Schaeckeler Date: Thu, 28 Oct 2021 00:44:04 -0700 Subject: [PATCH 06/11] ubifs: Document sysfs nodes Add documentation for the new sysfs nodes /sys/fs/ubifs/ubiX_Y/errors_magic /sys/fs/ubifs/ubiX_Y/errors_node /sys/fs/ubifs/ubiX_Y/errors_crc Signed-off-by: Stefan Schaeckeler Signed-off-by: Richard Weinberger --- Documentation/ABI/testing/sysfs-fs-ubifs | 35 ++++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 36 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-fs-ubifs diff --git a/Documentation/ABI/testing/sysfs-fs-ubifs b/Documentation/ABI/testing/sysfs-fs-ubifs new file mode 100644 index 000000000000..af5afda30220 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-fs-ubifs @@ -0,0 +1,35 @@ +What: /sys/fs/ubifsX_Y/error_magic +Date: October 2021 +KernelVersion: 5.16 +Contact: linux-mtd@lists.infradead.org +Description: + Exposes magic errors: every node starts with a magic number. + + This counter keeps track of the number of accesses of nodes + with a corrupted magic number. + + The counter is reset to 0 with a remount. + +What: /sys/fs/ubifsX_Y/error_node +Date: October 2021 +KernelVersion: 5.16 +Contact: linux-mtd@lists.infradead.org +Description: + Exposes node errors. Every node embeds its type. + + This counter keeps track of the number of accesses of nodes + with a corrupted node type. + + The counter is reset to 0 with a remount. + +What: /sys/fs/ubifsX_Y/error_crc +Date: October 2021 +KernelVersion: 5.16 +Contact: linux-mtd@lists.infradead.org +Description: + Exposes crc errors: every node embeds a crc checksum. + + This counter keeps track of the number of accesses of nodes + with a bad crc checksum. + + The counter is reset to 0 with a remount. diff --git a/MAINTAINERS b/MAINTAINERS index 8912b2c1260c..27cb3353d209 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -19444,6 +19444,7 @@ S: Supported W: http://www.linux-mtd.infradead.org/doc/ubifs.html T: git git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubifs.git next T: git git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubifs.git fixes +F: Documentation/ABI/testing/sysfs-fs-ubifs F: Documentation/filesystems/ubifs-authentication.rst F: Documentation/filesystems/ubifs.rst F: fs/ubifs/ From d3de970bcba0fb171e6aceaa0723d2cd842dc25c Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 9 Nov 2021 14:50:51 +0300 Subject: [PATCH 07/11] ubifs: fix snprintf() length check The snprintf() function returns the number of bytes (not including the NUL terminator) which would have been printed if there were enough space. So it can be greater than UBIFS_DFS_DIR_LEN. And actually if it equals UBIFS_DFS_DIR_LEN then that's okay so this check is too strict. Fixes: 9a620291fc01 ("ubifs: Export filesystem error counters") Signed-off-by: Dan Carpenter Signed-off-by: Richard Weinberger --- fs/ubifs/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c index 0eb3d7d12450..7acc5a74e5fa 100644 --- a/fs/ubifs/sysfs.c +++ b/fs/ubifs/sysfs.c @@ -100,7 +100,7 @@ int ubifs_sysfs_register(struct ubifs_info *c) n = snprintf(dfs_dir_name, UBIFS_DFS_DIR_LEN + 1, UBIFS_DFS_DIR_NAME, c->vi.ubi_num, c->vi.vol_id); - if (n == UBIFS_DFS_DIR_LEN) { + if (n > UBIFS_DFS_DIR_LEN) { /* The array size is too small */ ret = -EINVAL; goto out_free; From 88618feecf44e774e03cf49872567398b0177d25 Mon Sep 17 00:00:00 2001 From: Baokun Li Date: Mon, 15 Nov 2021 09:31:42 +0800 Subject: [PATCH 08/11] ubifs: fix slab-out-of-bounds in ubifs_change_lp Hulk Robot reported a KASAN report about slab-out-of-bounds: ================================================================== BUG: KASAN: slab-out-of-bounds in ubifs_change_lp+0x3a9/0x1390 [ubifs] Read of size 8 at addr ffff888101c961f8 by task fsstress/1068 [...] Call Trace: check_memory_region+0x1c1/0x1e0 ubifs_change_lp+0x3a9/0x1390 [ubifs] ubifs_change_one_lp+0x170/0x220 [ubifs] ubifs_garbage_collect+0x7f9/0xda0 [ubifs] ubifs_budget_space+0xfe4/0x1bd0 [ubifs] ubifs_write_begin+0x528/0x10c0 [ubifs] Allocated by task 1068: kmemdup+0x25/0x50 ubifs_lpt_lookup_dirty+0x372/0xb00 [ubifs] ubifs_update_one_lp+0x46/0x260 [ubifs] ubifs_tnc_end_commit+0x98b/0x1720 [ubifs] do_commit+0x6cb/0x1950 [ubifs] ubifs_run_commit+0x15a/0x2b0 [ubifs] ubifs_budget_space+0x1061/0x1bd0 [ubifs] ubifs_write_begin+0x528/0x10c0 [ubifs] [...] ================================================================== In ubifs_garbage_collect(), if ubifs_find_dirty_leb returns an error, lp is an uninitialized variable. But lp.num might be used in the out branch, which is a random value. If the value is -1 or another value that can pass the check, soob may occur in the ubifs_change_lp() in the following procedure. To solve this problem, we initialize lp.lnum to -1, and then initialize it correctly in ubifs_find_dirty_leb, which is not equal to -1, and ubifs_return_leb is executed only when lp.lnum != -1. if find a retained or indexing LEB and continue to next loop, but break before find another LEB, the "taken" flag of this LEB will be cleaned in ubi_return_lebi(). This bug has also been fixed in this patch. Reported-by: Hulk Robot Signed-off-by: Baokun Li Signed-off-by: Richard Weinberger --- fs/ubifs/gc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index dc3e26e9ed7b..05e1eeae8457 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -692,6 +692,9 @@ int ubifs_garbage_collect(struct ubifs_info *c, int anyway) for (i = 0; ; i++) { int space_before, space_after; + /* Maybe continue after find and break before find */ + lp.lnum = -1; + cond_resched(); /* Give the commit an opportunity to run */ @@ -843,7 +846,8 @@ int ubifs_garbage_collect(struct ubifs_info *c, int anyway) ubifs_wbuf_sync_nolock(wbuf); ubifs_ro_mode(c, ret); mutex_unlock(&wbuf->io_mutex); - ubifs_return_leb(c, lp.lnum); + if (lp.lnum != -1) + ubifs_return_leb(c, lp.lnum); return ret; } From 0d76502172d83e1e09aedbdced3d8be0ef1abcb5 Mon Sep 17 00:00:00 2001 From: Baokun Li Date: Mon, 15 Nov 2021 09:31:43 +0800 Subject: [PATCH 09/11] ubifs: fix double return leb in ubifs_garbage_collect If ubifs_garbage_collect_leb() returns -EAGAIN and enters the "out" branch, ubifs_return_leb will execute twice on the same lnum. This can cause data loss in concurrency situations. Reported-by: Hulk Robot Signed-off-by: Baokun Li Signed-off-by: Richard Weinberger --- fs/ubifs/gc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 05e1eeae8457..1f74a127fe3a 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -758,6 +758,8 @@ int ubifs_garbage_collect(struct ubifs_info *c, int anyway) err = ubifs_return_leb(c, lp.lnum); if (err) ret = err; + /* Maybe double return LEB if goto out */ + lp.lnum = -1; break; } goto out; From 50cb4373254433ad015dd50a061194c693b37c16 Mon Sep 17 00:00:00 2001 From: Baokun Li Date: Mon, 15 Nov 2021 09:31:44 +0800 Subject: [PATCH 10/11] ubifs: read-only if LEB may always be taken in ubifs_garbage_collect If ubifs_garbage_collect_leb() returns -EAGAIN and ubifs_return_leb returns error, a LEB will always has a "taken" flag. In this case, set the ubifs to read-only to prevent a worse situation. Signed-off-by: Baokun Li Signed-off-by: Richard Weinberger --- fs/ubifs/gc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 1f74a127fe3a..3134d070fcc0 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -756,8 +756,17 @@ int ubifs_garbage_collect(struct ubifs_info *c, int anyway) * caller instead of the original '-EAGAIN'. */ err = ubifs_return_leb(c, lp.lnum); - if (err) + if (err) { ret = err; + /* + * An LEB may always be "taken", + * so setting ubifs to read-only, + * and then executing sync wbuf will + * return -EROFS and enter the "out" + * error branch. + */ + ubifs_ro_mode(c, ret); + } /* Maybe double return LEB if goto out */ lp.lnum = -1; break; From aa39cc675799bc92da153af9a13d6f969c348e82 Mon Sep 17 00:00:00 2001 From: Kyeong Yoo Date: Tue, 4 Jul 2017 16:22:38 +1200 Subject: [PATCH 11/11] jffs2: GC deadlock reading a page that is used in jffs2_write_begin() GC task can deadlock in read_cache_page() because it may attempt to release a page that is actually allocated by another task in jffs2_write_begin(). The reason is that in jffs2_write_begin() there is a small window a cache page is allocated for use but not set Uptodate yet. This ends up with a deadlock between two tasks: 1) A task (e.g. file copy) - jffs2_write_begin() locks a cache page - jffs2_write_end() tries to lock "alloc_sem" from jffs2_reserve_space() <-- STUCK 2) GC task (jffs2_gcd_mtd3) - jffs2_garbage_collect_pass() locks "alloc_sem" - try to lock the same cache page in read_cache_page() <-- STUCK So to avoid this deadlock, hold "alloc_sem" in jffs2_write_begin() while reading data in a cache page. Signed-off-by: Kyeong Yoo Signed-off-by: Richard Weinberger --- fs/jffs2/file.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c index 4fc8cd698d1a..bd7d58d27bfc 100644 --- a/fs/jffs2/file.c +++ b/fs/jffs2/file.c @@ -136,20 +136,15 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, struct page *pg; struct inode *inode = mapping->host; struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode); + struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); pgoff_t index = pos >> PAGE_SHIFT; uint32_t pageofs = index << PAGE_SHIFT; int ret = 0; - pg = grab_cache_page_write_begin(mapping, index, flags); - if (!pg) - return -ENOMEM; - *pagep = pg; - jffs2_dbg(1, "%s()\n", __func__); if (pageofs > inode->i_size) { /* Make new hole frag from old EOF to new page */ - struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); struct jffs2_raw_inode ri; struct jffs2_full_dnode *fn; uint32_t alloc_len; @@ -160,7 +155,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len, ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE); if (ret) - goto out_page; + goto out_err; mutex_lock(&f->sem); memset(&ri, 0, sizeof(ri)); @@ -190,7 +185,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ret = PTR_ERR(fn); jffs2_complete_reservation(c); mutex_unlock(&f->sem); - goto out_page; + goto out_err; } ret = jffs2_add_full_dnode_to_inode(c, f, fn); if (f->metadata) { @@ -205,13 +200,26 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, jffs2_free_full_dnode(fn); jffs2_complete_reservation(c); mutex_unlock(&f->sem); - goto out_page; + goto out_err; } jffs2_complete_reservation(c); inode->i_size = pageofs; mutex_unlock(&f->sem); } + /* + * While getting a page and reading data in, lock c->alloc_sem until + * the page is Uptodate. Otherwise GC task may attempt to read the same + * page in read_cache_page(), which causes a deadlock. + */ + mutex_lock(&c->alloc_sem); + pg = grab_cache_page_write_begin(mapping, index, flags); + if (!pg) { + ret = -ENOMEM; + goto release_sem; + } + *pagep = pg; + /* * Read in the page if it wasn't already present. Cannot optimize away * the whole page write case until jffs2_write_end can handle the @@ -221,15 +229,17 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, mutex_lock(&f->sem); ret = jffs2_do_readpage_nolock(inode, pg); mutex_unlock(&f->sem); - if (ret) - goto out_page; + if (ret) { + unlock_page(pg); + put_page(pg); + goto release_sem; + } } jffs2_dbg(1, "end write_begin(). pg->flags %lx\n", pg->flags); - return ret; -out_page: - unlock_page(pg); - put_page(pg); +release_sem: + mutex_unlock(&c->alloc_sem); +out_err: return ret; }