Patchwork [v12,2/4] : mtdoops: Add a maximum MTD partition size

login
register
mail settings
Submitter Simon Kagstrom
Date Oct. 29, 2009, 12:41 p.m.
Message ID <20091029134116.354237ba@marrow.netinsight.se>
Download mbox | patch
Permalink /patch/37179/
State New
Headers show

Comments

Simon Kagstrom - Oct. 29, 2009, 12:41 p.m.
A configurable maximum MTD partition size for mtdoops avoids mistakes
where the user gives e.g., a rootfs partition to mtdoops (which will
happily erase it).

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
 drivers/mtd/Kconfig   |   13 +++++++++++++
 drivers/mtd/mtdoops.c |   11 +++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)
Artem Bityutskiy - Oct. 29, 2009, 3:27 p.m.
On Thu, 2009-10-29 at 13:41 +0100, Simon Kagstrom wrote:
> A configurable maximum MTD partition size for mtdoops avoids mistakes
> where the user gives e.g., a rootfs partition to mtdoops (which will
> happily erase it).
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
>  drivers/mtd/Kconfig   |   13 +++++++++++++
>  drivers/mtd/mtdoops.c |   11 +++++++----
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index ecf90f5..9a69af8 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -315,6 +315,19 @@ config MTD_OOPS
>  	  To use, add console=ttyMTDx to the kernel command line,
>  	  where x is the MTD device number to use.
>  
> +config MTD_OOPS_MAX_MTD_SIZE
> +	int "Maximum MTD oops partition size (kbytes)"
> +	default 4096
> +	range 8 1048576
> +	depends on MTD_OOPS
> +	help
> +	  This parameter sets the maximum MTD partition size for use with
> +	  MTD oops. The default value is used as a safeguard against using
> +	  e.g., a root filesystem partition as a MTDoops device (in which
> +	  case it will be erased).
> +
> +	  No need to change unless you have a very large MTDoops partition.
> +
>  source "drivers/mtd/chips/Kconfig"

Could we please just make it hard limited to something like 64MiB in the
driver itself. This configuration option is not really needed. There are
not many users of this driver, and I do not think any of them would ever
need a big partition for mtdoops, so there is not reason to make things
more complex than they are by introducing this configuration option.

Patch

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index ecf90f5..9a69af8 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -315,6 +315,19 @@  config MTD_OOPS
 	  To use, add console=ttyMTDx to the kernel command line,
 	  where x is the MTD device number to use.
 
+config MTD_OOPS_MAX_MTD_SIZE
+	int "Maximum MTD oops partition size (kbytes)"
+	default 4096
+	range 8 1048576
+	depends on MTD_OOPS
+	help
+	  This parameter sets the maximum MTD partition size for use with
+	  MTD oops. The default value is used as a safeguard against using
+	  e.g., a root filesystem partition as a MTDoops device (in which
+	  case it will be erased).
+
+	  No need to change unless you have a very large MTDoops partition.
+
 source "drivers/mtd/chips/Kconfig"
 
 source "drivers/mtd/maps/Kconfig"
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 209f8f4..dc94a70 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -298,6 +298,12 @@  static void mtdoops_notify_add(struct mtd_info *mtd)
 	if (mtd->index != cxt->mtd_index || cxt->mtd_index < 0)
 		return;
 
+	if (mtd->size > CONFIG_MTD_OOPS_MAX_MTD_SIZE * 1024) {
+		printk(KERN_ERR "mtdoops: MTD partition %d too big for mtdoops (limit %d KiB)\n",
+		       mtd->index, CONFIG_MTD_OOPS_MAX_MTD_SIZE);
+		return;
+	}
+
 	if (mtd->size < mtd->erasesize * 2) {
 		printk(KERN_ERR "mtdoops: MTD partition %d not big enough for mtdoops\n",
 		       mtd->index);
@@ -318,10 +324,7 @@  static void mtdoops_notify_add(struct mtd_info *mtd)
 		return;
 	}
 	cxt->mtd = mtd;
-	if (mtd->size > INT_MAX)
-		cxt->oops_pages = INT_MAX / OOPS_PAGE_SIZE;
-	else
-		cxt->oops_pages = (int)mtd->size / OOPS_PAGE_SIZE;
+	cxt->oops_pages = (int)mtd->size / OOPS_PAGE_SIZE;
 
 	find_next_position(cxt);