Patchwork mtd: mtd_torturetest can cause stack overflows

login
register
mail settings
Submitter Al Cooper
Date Feb. 4, 2013, 9:29 p.m.
Message ID <1360013390-30179-1-git-send-email-alcooperx@gmail.com>
Download mbox | patch
Permalink /patch/218073/
State New
Headers show

Comments

Al Cooper - Feb. 4, 2013, 9:29 p.m.
mtd_torturetest uses the module parm "ebcnt" to control the size of a
stack based array of int's. When "ebcnt" is large, Ex: 1000, it
causes stack overflows on systems with small kernel stacks. The fix
is to move the array from the stack to kmalloc memory.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 drivers/mtd/tests/mtd_torturetest.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
Huang Shijie - Feb. 5, 2013, 2:05 a.m.
于 2013年02月05日 05:29, Al Cooper 写道:
> diff --git a/drivers/mtd/tests/mtd_torturetest.c b/drivers/mtd/tests/mtd_torturetest.c
> index c4cde1e..a777cc8 100644
> --- a/drivers/mtd/tests/mtd_torturetest.c
> +++ b/drivers/mtd/tests/mtd_torturetest.c
> @@ -208,7 +208,7 @@ static inline int write_pattern(int ebnum, void *buf)
>   static int __init tort_init(void)
>   {
>   	int err = 0, i, infinite = !cycles_count;
> -	int bad_ebs[ebcnt];
> +	int *bad_ebs;
>
>   	printk(KERN_INFO "\n");
>   	printk(KERN_INFO "=================================================\n");
> @@ -273,6 +273,12 @@ static int __init tort_init(void)
>   		goto out_patt_FF;
>   	}
>
> +	bad_ebs = kmalloc(sizeof(*bad_ebs) * ebcnt, GFP_KERNEL);
I think it's better to use the kcalloc() here.

thanks
Huang Shijie
Ezequiel Garcia - Feb. 5, 2013, 11:20 a.m.
On Mon, Feb 4, 2013 at 6:29 PM, Al Cooper <alcooperx@gmail.com> wrote:
> mtd_torturetest uses the module parm "ebcnt" to control the size of a
> stack based array of int's. When "ebcnt" is large, Ex: 1000, it
> causes stack overflows on systems with small kernel stacks. The fix
> is to move the array from the stack to kmalloc memory.
>
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
> ---
>  drivers/mtd/tests/mtd_torturetest.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mtd/tests/mtd_torturetest.c b/drivers/mtd/tests/mtd_torturetest.c
> index c4cde1e..a777cc8 100644
> --- a/drivers/mtd/tests/mtd_torturetest.c
> +++ b/drivers/mtd/tests/mtd_torturetest.c
> @@ -208,7 +208,7 @@ static inline int write_pattern(int ebnum, void *buf)
>  static int __init tort_init(void)
>  {
>         int err = 0, i, infinite = !cycles_count;
> -       int bad_ebs[ebcnt];
> +       int *bad_ebs;
>
>         printk(KERN_INFO "\n");
>         printk(KERN_INFO "=================================================\n");
> @@ -273,6 +273,12 @@ static int __init tort_init(void)
>                 goto out_patt_FF;
>         }
>
> +       bad_ebs = kmalloc(sizeof(*bad_ebs) * ebcnt, GFP_KERNEL);
> +       if (!bad_ebs) {
> +               pr_err("error: cannot allocate memory\n");

You don't want to print this error. See:

http://www.spinics.net/lists/newbies/msg48792.html
http://archive.linuxvirtualserver.org/html/lvs-devel/2011-08/msg00001.html

Patch

diff --git a/drivers/mtd/tests/mtd_torturetest.c b/drivers/mtd/tests/mtd_torturetest.c
index c4cde1e..a777cc8 100644
--- a/drivers/mtd/tests/mtd_torturetest.c
+++ b/drivers/mtd/tests/mtd_torturetest.c
@@ -208,7 +208,7 @@  static inline int write_pattern(int ebnum, void *buf)
 static int __init tort_init(void)
 {
 	int err = 0, i, infinite = !cycles_count;
-	int bad_ebs[ebcnt];
+	int *bad_ebs;
 
 	printk(KERN_INFO "\n");
 	printk(KERN_INFO "=================================================\n");
@@ -273,6 +273,12 @@  static int __init tort_init(void)
 		goto out_patt_FF;
 	}
 
+	bad_ebs = kmalloc(sizeof(*bad_ebs) * ebcnt, GFP_KERNEL);
+	if (!bad_ebs) {
+		pr_err("error: cannot allocate memory\n");
+		goto out_check_buf;
+	}
+
 	err = 0;
 
 	/* Initialize patterns */
@@ -394,6 +400,8 @@  out:
 
 	pr_info("finished after %u erase cycles\n",
 	       erase_cycles);
+	kfree(bad_ebs);
+out_check_buf:
 	kfree(check_buf);
 out_patt_FF:
 	kfree(patt_FF);