kernel: improve ubi auto attach code readability

Move the put_mtd_device() called on multiple error conditions to a goto
label to use it later for more error conditions.

The early return on failed open of the mtd device and mismatching mtd
type allows to get rid of one level of indentation. By jumping to the
cleanup code, a refcount bug is fixed for the wrong flash type condition.

While at it, make clear that we only check for the UBI magic if the read
from flash was successful.

Signed-off-by: Mathias Kresin <dev@kresin.me>
This commit is contained in:
Mathias Kresin 2018-07-08 01:38:08 +02:00
parent 354de22bad
commit fdf6760cda
2 changed files with 64 additions and 54 deletions

View file

@ -8,7 +8,7 @@ Signed-off-by: Daniel Golle <daniel@makrotopia.org>
--- a/drivers/mtd/ubi/build.c --- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c +++ b/drivers/mtd/ubi/build.c
@@ -1171,6 +1171,49 @@ static struct mtd_info * __init open_mtd @@ -1171,6 +1171,54 @@ static struct mtd_info * __init open_mtd
return mtd; return mtd;
} }
@ -20,45 +20,50 @@ Signed-off-by: Daniel Golle <daniel@makrotopia.org>
+{ +{
+ int err; + int err;
+ struct mtd_info *mtd; + struct mtd_info *mtd;
+ size_t len;
+ char magic[4];
+ +
+ /* try attaching mtd device named "ubi" or "data" */ + /* try attaching mtd device named "ubi" or "data" */
+ mtd = open_mtd_device("ubi"); + mtd = open_mtd_device("ubi");
+ if (IS_ERR(mtd)) + if (IS_ERR(mtd))
+ mtd = open_mtd_device("data"); + mtd = open_mtd_device("data");
+ +
+ if (!IS_ERR(mtd)) { + if (IS_ERR(mtd))
+ size_t len; + return;
+ char magic[4];
+ +
+ /* check for a valid ubi magic */ + /* check for a valid ubi magic if read from flash was successful */
+ err = mtd_read(mtd, 0, 4, &len, (void *) magic); + err = mtd_read(mtd, 0, 4, &len, (void *) magic);
+ if (!err && len == 4 && strncmp(magic, "UBI#", 4)) { + if (!err && len == 4 && strncmp(magic, "UBI#", 4)) {
+ pr_err("UBI error: no valid UBI magic found inside mtd%d\n", mtd->index); + pr_err("UBI error: no valid UBI magic found inside mtd%d\n", mtd->index);
+ put_mtd_device(mtd); + goto cleanup;
+ return;
+ }
+
+ /* auto-add only media types where UBI makes sense */
+ if (mtd->type == MTD_NANDFLASH ||
+ mtd->type == MTD_NORFLASH ||
+ mtd->type == MTD_DATAFLASH ||
+ mtd->type == MTD_MLCNANDFLASH) {
+ mutex_lock(&ubi_devices_mutex);
+ pr_notice("UBI: auto-attach mtd%d\n", mtd->index);
+ err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO, 0, 0);
+ mutex_unlock(&ubi_devices_mutex);
+ if (err < 0) {
+ pr_err("UBI error: cannot attach mtd%d\n", mtd->index);
+ put_mtd_device(mtd);
+ }
+ }
+ } + }
+
+ /* don't auto-add media types where UBI doesn't makes sense */
+ if (mtd->type != MTD_NANDFLASH &&
+ mtd->type != MTD_NORFLASH &&
+ mtd->type != MTD_DATAFLASH &&
+ mtd->type != MTD_MLCNANDFLASH)
+ goto cleanup;
+
+ mutex_lock(&ubi_devices_mutex);
+ pr_notice("UBI: auto-attach mtd%d\n", mtd->index);
+ err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO, 0, 0);
+ mutex_unlock(&ubi_devices_mutex);
+ if (err < 0) {
+ pr_err("UBI error: cannot attach mtd%d\n", mtd->index);
+ goto cleanup;
+ }
+
+ return;
+
+cleanup:
+ put_mtd_device(mtd);
+} +}
+ +
static int __init ubi_init(void) static int __init ubi_init(void)
{ {
int err, i, k; int err, i, k;
@@ -1254,6 +1297,12 @@ static int __init ubi_init(void) @@ -1254,6 +1302,12 @@ static int __init ubi_init(void)
} }
} }

View file

@ -8,7 +8,7 @@ Signed-off-by: Daniel Golle <daniel@makrotopia.org>
--- a/drivers/mtd/ubi/build.c --- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c +++ b/drivers/mtd/ubi/build.c
@@ -1226,6 +1226,49 @@ static struct mtd_info * __init open_mtd @@ -1226,6 +1226,54 @@ static struct mtd_info * __init open_mtd
return mtd; return mtd;
} }
@ -20,45 +20,50 @@ Signed-off-by: Daniel Golle <daniel@makrotopia.org>
+{ +{
+ int err; + int err;
+ struct mtd_info *mtd; + struct mtd_info *mtd;
+ size_t len;
+ char magic[4];
+ +
+ /* try attaching mtd device named "ubi" or "data" */ + /* try attaching mtd device named "ubi" or "data" */
+ mtd = open_mtd_device("ubi"); + mtd = open_mtd_device("ubi");
+ if (IS_ERR(mtd)) + if (IS_ERR(mtd))
+ mtd = open_mtd_device("data"); + mtd = open_mtd_device("data");
+ +
+ if (!IS_ERR(mtd)) { + if (IS_ERR(mtd))
+ size_t len; + return;
+ char magic[4];
+ +
+ /* check for a valid ubi magic */ + /* check for a valid ubi magic if read from flash was successful */
+ err = mtd_read(mtd, 0, 4, &len, (void *) magic); + err = mtd_read(mtd, 0, 4, &len, (void *) magic);
+ if (!err && len == 4 && strncmp(magic, "UBI#", 4)) { + if (!err && len == 4 && strncmp(magic, "UBI#", 4)) {
+ pr_err("UBI error: no valid UBI magic found inside mtd%d\n", mtd->index); + pr_err("UBI error: no valid UBI magic found inside mtd%d\n", mtd->index);
+ put_mtd_device(mtd); + goto cleanup;
+ return;
+ }
+
+ /* auto-add only media types where UBI makes sense */
+ if (mtd->type == MTD_NANDFLASH ||
+ mtd->type == MTD_NORFLASH ||
+ mtd->type == MTD_DATAFLASH ||
+ mtd->type == MTD_MLCNANDFLASH) {
+ mutex_lock(&ubi_devices_mutex);
+ pr_notice("UBI: auto-attach mtd%d\n", mtd->index);
+ err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO, 0, 0);
+ mutex_unlock(&ubi_devices_mutex);
+ if (err < 0) {
+ pr_err("UBI error: cannot attach mtd%d\n", mtd->index);
+ put_mtd_device(mtd);
+ }
+ }
+ } + }
+
+ /* don't auto-add media types where UBI doesn't makes sense */
+ if (mtd->type != MTD_NANDFLASH &&
+ mtd->type != MTD_NORFLASH &&
+ mtd->type != MTD_DATAFLASH &&
+ mtd->type != MTD_MLCNANDFLASH)
+ goto cleanup;
+
+ mutex_lock(&ubi_devices_mutex);
+ pr_notice("UBI: auto-attach mtd%d\n", mtd->index);
+ err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO, 0, 0);
+ mutex_unlock(&ubi_devices_mutex);
+ if (err < 0) {
+ pr_err("UBI error: cannot attach mtd%d\n", mtd->index);
+ goto cleanup;
+ }
+
+ return;
+
+cleanup:
+ put_mtd_device(mtd);
+} +}
+ +
static int __init ubi_init(void) static int __init ubi_init(void)
{ {
int err, i, k; int err, i, k;
@@ -1309,6 +1352,12 @@ static int __init ubi_init(void) @@ -1309,6 +1357,12 @@ static int __init ubi_init(void)
} }
} }