[PATCH] mtd: Introduce an expert mode for forensics and debugging purposes

[Geert Uytterhoeven]

Hi Miquel,

On Thu, Nov 18, 2021 at 12:47 PM Miquel Raynal
<miquel.raynal at bootlin.com> wrote:
> When developping NAND controller drivers or when debugging filesystem
> corruptions, it is quite common to need hacking locally into the
> MTD/NAND core in order to get access to the content of the bad
> blocks. Instead of having multiple implementations out there let's
> provide a simple yet effective specific MTD-wide debugfs entry to fully
> disable these checks on purpose.
>
> A warning is added to inform the user when this mode gets enabled.
>
> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>

Thanks for your patch, which is now commit 67b967ddd93d0ed5 ("mtd:
Introduce an expert mode for forensics and debugging purposes")
in mtd/next.

> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -2365,6 +2365,14 @@ static struct backing_dev_info * __init mtd_bdi_init(const char *name)
>         return ret ? ERR_PTR(ret) : bdi;
>  }
>
> +char *mtd_expert_analysis_warning =

const

> +       "Bad block checks have been entirely disabled.\n"
> +       "This is only reserved for post-mortem forensics and debug purposes.\n"
> +       "Never enable this mode if you do not know what you are doing!\n";
> +EXPORT_SYMBOL_GPL(mtd_expert_analysis_warning);

Shouldn't this depend on CONFIG_DEBUG_FS?

> +bool mtd_expert_analysis_mode;
> +EXPORT_SYMBOL_GPL(mtd_expert_analysis_mode);

Do you really need to export these two symbols?

> +
>  static struct proc_dir_entry *proc_mtd;
>
>  static int __init init_mtd(void)
=
> --- a/drivers/mtd/nand/core.c
> +++ b/drivers/mtd/nand/core.c
> @@ -21,6 +21,9 @@
>   */
>  bool nanddev_isbad(struct nand_device *nand, const struct nand_pos *pos)
>  {
> +       if (WARN_ONCE(mtd_expert_analysis_mode, mtd_expert_analysis_warning))
> +               return 0;
> +
>         if (nanddev_bbt_is_initialized(nand)) {
>                 unsigned int entry;
>                 int status;
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 3d6c6e880520..b3a9bc08b4bb 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -321,6 +321,9 @@ static int nand_isbad_bbm(struct nand_chip *chip, loff_t ofs)
>         if (nand_region_is_secured(chip, ofs, mtd->erasesize))
>                 return -EIO;
>
> +       if (WARN_ONCE(mtd_expert_analysis_mode, mtd_expert_analysis_warning))
> +               return 0;
> +
>         if (chip->legacy.block_bad)
>                 return chip->legacy.block_bad(chip, ofs);
>
> diff --git a/drivers/mtd/nand/raw/nand_bbt.c b/drivers/mtd/nand/raw/nand_bbt.c
> index b7ad030225f8..ab630af3a309 100644
> --- a/drivers/mtd/nand/raw/nand_bbt.c
> +++ b/drivers/mtd/nand/raw/nand_bbt.c
> @@ -1455,6 +1455,9 @@ int nand_isbad_bbt(struct nand_chip *this, loff_t offs, int allowbbt)
>         pr_debug("nand_isbad_bbt(): bbt info for offs 0x%08x: (block %d) 0x%02x\n",
>                  (unsigned int)offs, block, res);
>
> +       if (WARN_ONCE(mtd_expert_analysis_mode, mtd_expert_analysis_warning))
> +               return 0;
> +

These are all the same.

What about letting drivers/mtd/mtdcore.c export a simple function
mtd_check_expert_analysis_mode() that calls the WARN_ONCE(...) if
CONFIG_DEBUG_FS=y, else providing a dummy?
The backtrace will identify the caller anyway.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds