2024-04-11 03:28:31

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v2 0/4] ubi: Fix serious of resource leaking problems

v1->v2:
Combine Ben[1] and my[2] patches.
Modify second patch in [1], don't drop warning message if debugfs
failed to be initialized.
Modify second patch in [2], move ubiblock_init to the end of ubi_init.
Add new patch to replace IS_ERR_OR_NULL with IS_ERR.

[1] https://lore.kernel.org/linux-mtd/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/T/

Ben Hutchings (1):
mtd: ubi: Ignore all debugfs initialisation failures

Zhihao Cheng (3):
ubi: ubi_init: Fix missed debugfs cleanup in error handling path
ubi: ubi_init: Fix missed ubiblock cleanup in error handling path
ubi: debugfs: Replace IS_ERR_OR_NULL with IS_ERR in error checking
path

drivers/mtd/ubi/build.c | 61 ++++++++++++++++++++---------------------
drivers/mtd/ubi/debug.c | 35 +++++++++--------------
drivers/mtd/ubi/debug.h | 4 +--
3 files changed, 44 insertions(+), 56 deletions(-)

--
2.39.2



2024-04-11 03:28:48

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v2 4/4] ubi: debugfs: Replace IS_ERR_OR_NULL with IS_ERR in error checking path

Function debugfs_create_dir will return error pointer rather than NULL
if it fails, replace IS_ERR_OR_NULL with IS_ERR just like other places
(eg. fault_create_debugfs_attr).

Signed-off-by: Zhihao Cheng <[email protected]>
---
drivers/mtd/ubi/debug.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c
index 989744eb85a5..3bcdf4e70a0a 100644
--- a/drivers/mtd/ubi/debug.c
+++ b/drivers/mtd/ubi/debug.c
@@ -248,11 +248,9 @@ static void dfs_create_fault_entry(struct dentry *parent)
struct dentry *dir;

dir = debugfs_create_dir("fault_inject", parent);
- if (IS_ERR_OR_NULL(dir)) {
- int err = dir ? PTR_ERR(dir) : -ENODEV;
-
+ if (IS_ERR(dir)) {
pr_warn("UBI error: cannot create \"fault_inject\" debugfs directory, error %d\n",
- err);
+ (int)PTR_ERR(dir));
return;
}

@@ -299,11 +297,9 @@ void ubi_debugfs_init(void)
return;

dfs_rootdir = debugfs_create_dir("ubi", NULL);
- if (IS_ERR_OR_NULL(dfs_rootdir)) {
- int err = dfs_rootdir ? PTR_ERR(dfs_rootdir) : -ENODEV;
-
+ if (IS_ERR(dfs_rootdir)) {
pr_warn("UBI error: cannot create \"ubi\" debugfs directory, error %d\n",
- err);
+ (int)PTR_ERR(dfs_rootdir));
return;
}

--
2.39.2


2024-04-11 03:50:27

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v2 3/4] mtd: ubi: Ignore all debugfs initialisation failures

From: Ben Hutchings <[email protected]>

Drivers should never stop initialising or probing because of a failure
to create debugfs entries. Such failures are harmless and could be
the intended result of setting the kernel parameter debugfs=off.

This was partially fixed in 2019, but some error checks were missed
and another one was been added since.

Signed-off-by: Ben Hutchings <[email protected]>
Fixes: 2a734bb8d502 ("UBI: use debugfs for the extra checks knobs")
Fixes: c2d73ba892ea ("mtd: no need to check return value of ...")
Fixes: 6931fb44858c ("ubi: Use the fault injection framework to enhance ...")
Signed-off-by: Zhihao Cheng <[email protected]>
---
drivers/mtd/ubi/build.c | 10 ++--------
drivers/mtd/ubi/debug.c | 25 ++++++++++---------------
drivers/mtd/ubi/debug.h | 4 ++--
3 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index bc63fbf5e947..6d8212a51fec 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -1018,9 +1018,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
if (err)
goto out_detach;

- err = ubi_debugfs_init_dev(ubi);
- if (err)
- goto out_uif;
+ ubi_debugfs_init_dev(ubi);

ubi->bgt_thread = kthread_create(ubi_thread, ubi, "%s", ubi->bgt_name);
if (IS_ERR(ubi->bgt_thread)) {
@@ -1064,7 +1062,6 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,

out_debugfs:
ubi_debugfs_exit_dev(ubi);
-out_uif:
uif_close(ubi);
out_detach:
ubi_wl_close(ubi);
@@ -1369,9 +1366,7 @@ static int __init ubi_init(void)
goto out_dev_unreg;
}

- err = ubi_debugfs_init();
- if (err)
- goto out_slab;
+ ubi_debugfs_init();

register_mtd_user(&ubi_mtd_notifier);

@@ -1397,7 +1392,6 @@ static int __init ubi_init(void)
out_mtd_notifier:
unregister_mtd_user(&ubi_mtd_notifier);
ubi_debugfs_exit();
-out_slab:
kmem_cache_destroy(ubi_wl_entry_slab);
out_dev_unreg:
misc_deregister(&ubi_ctrl_cdev);
diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c
index d57f52bd2ff3..989744eb85a5 100644
--- a/drivers/mtd/ubi/debug.c
+++ b/drivers/mtd/ubi/debug.c
@@ -291,28 +291,25 @@ static void dfs_create_fault_entry(struct dentry *parent)
/**
* ubi_debugfs_init - create UBI debugfs directory.
*
- * Create UBI debugfs directory. Returns zero in case of success and a negative
- * error code in case of failure.
+ * Try to create UBI debugfs directory.
*/
-int ubi_debugfs_init(void)
+void ubi_debugfs_init(void)
{
if (!IS_ENABLED(CONFIG_DEBUG_FS))
- return 0;
+ return;

dfs_rootdir = debugfs_create_dir("ubi", NULL);
if (IS_ERR_OR_NULL(dfs_rootdir)) {
int err = dfs_rootdir ? PTR_ERR(dfs_rootdir) : -ENODEV;

- pr_err("UBI error: cannot create \"ubi\" debugfs directory, error %d\n",
- err);
- return err;
+ pr_warn("UBI error: cannot create \"ubi\" debugfs directory, error %d\n",
+ err);
+ return;
}

#ifdef CONFIG_MTD_UBI_FAULT_INJECTION
dfs_create_fault_entry(dfs_rootdir);
#endif
-
- return 0;
}

/**
@@ -585,10 +582,9 @@ static const struct file_operations eraseblk_count_fops = {
* ubi_debugfs_init_dev - initialize debugfs for an UBI device.
* @ubi: UBI device description object
*
- * This function creates all debugfs files for UBI device @ubi. Returns zero in
- * case of success and a negative error code in case of failure.
+ * This function tries to create all debugfs files for UBI device @ubi.
*/
-int ubi_debugfs_init_dev(struct ubi_device *ubi)
+void ubi_debugfs_init_dev(struct ubi_device *ubi)
{
unsigned long ubi_num = ubi->ubi_num;
struct ubi_debug_info *d = &ubi->dbg;
@@ -596,13 +592,13 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi)
int n;

if (!IS_ENABLED(CONFIG_DEBUG_FS))
- return 0;
+ return;

n = snprintf(d->dfs_dir_name, UBI_DFS_DIR_LEN + 1, UBI_DFS_DIR_NAME,
ubi->ubi_num);
if (n > UBI_DFS_DIR_LEN) {
/* The array size is too small */
- return -EINVAL;
+ return;
}

d->dfs_dir = debugfs_create_dir(d->dfs_dir_name, dfs_rootdir);
@@ -653,7 +649,6 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi)
(void *)ubi_num,
&dfs_fops);
#endif
- return 0;
}

/**
diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h
index b2fd97548808..1dc430d3f482 100644
--- a/drivers/mtd/ubi/debug.h
+++ b/drivers/mtd/ubi/debug.h
@@ -47,9 +47,9 @@ void ubi_dump_aeb(const struct ubi_ainf_peb *aeb, int type);
void ubi_dump_mkvol_req(const struct ubi_mkvol_req *req);
int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset,
int len);
-int ubi_debugfs_init(void);
+void ubi_debugfs_init(void);
void ubi_debugfs_exit(void);
-int ubi_debugfs_init_dev(struct ubi_device *ubi);
+void ubi_debugfs_init_dev(struct ubi_device *ubi);
void ubi_debugfs_exit_dev(struct ubi_device *ubi);

/**
--
2.39.2