Re: [BUG] Re: mtd_stresstest module bricked my dockstar

Re: [BUG] Re: mtd_stresstest module bricked my dockstar

[Roland Kletzing]

>> > Anyway - what would people think if linux had a kernel module which wipes
>> > /dev/sda1 when loaded ? :)
>
>Yeah, the tests are destructive and they start erasing and torturing
>your MTD device without preserving your data.
>
>And the bad thing is - if you do not pass any parameter, they will
>torture your mtd0.
>
>I am sorry that you bricked your device and I hope you will be able to
>resurrect it.

That will be an exercise....

>This was an oversight. The tests were written for
>validating new drivers and making sure they are robust. So we always
>assumed people run them on a development HW, not on a production HW.
>That was a false implicit assumption.

No problem, i think it will not beggar me. I was a little bit disappointed at 
first, but when re-thinking i think it`s just a result of some carelessnes 
of me (as the user) and also of you developers of that modules (as nobody
expected that it could be accidentally loaded or even exist because of some
foreign kernel .config). nice to see the positive reaction on my report.

>Yes, we should use -1 as the default value, and print a scary warning
>message when the module is inserted without a parameter. We should
>probably also put word "destructive" or something like this to the
>module description.

Great! That sounds good. I think i will happily test if a fix is working, but
most likely not on my other Dockstar. Maybe block2mtd on top of a file-backed
loopdevice will provide a safe test-environment on my x86 box..... :)

regards
Roland


___________________________________________________________
SMS schreiben mit WEB.DE FreeMail - einfach, schnell und
kostenguenstig. Jetzt gleich testen! http://f.web.de/?mc=021192

Re: [BUG] Re: mtd_stresstest module bricked my dockstar

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 4251 bytes --]

On Mon, Oct 24, 2011 at 09:32:42PM +0200, Roland Kletzing wrote:

CCing the mtd-list...

> Hello,
> 
> cc`ing correct email of Adrian Hunter and i found that
> mtd_torturetest (and the
> other modules) have the same issue and there is another author i
> could find the
> email for.
> 
> >If you can't restore it with JTAG, it means the hardware is really dead.
> >JTAG is used when you blank the flash, and is supposed to work. I don't
> >know what the module does, but I fail to see how to could wear the flash
> >that fast. At worst it could have wiped it, or the flash was already bad.
> 
> Yes, i think accidentally insmodding  "mtd_stresstest" has just
> wiped it, not killed.
> The problem is, that it is important stuff for booting and you can`t
> pull it out and
> re-write externally, like a disk. Sorry, i that was probably not
> clearly stated.
> 
> Anyway - what would people think if linux had a kernel module  which wipes
> /dev/sda1 when loaded ? :)
> 
> >I got one Iomega Iconnect with a faulty flash that I got replaced for a
> >good one, so it's more likely the case here.
> 
> Yes, i could give debricking with JTAG a try. But what about the
> cost for the JTAG
> and the work to be spent with it? I could buy another Dockstar for that.....
> 
> >I agree with you. I remember the very old ISA NE2000 driver who used to
> >scan various addresses to find a NIC, causing some of them to reconfigure
> >their address to *none* and definitely stop responding on the bus to any
> >reconfiguration request. That was pretty annoying too. After killing a
> >few, I stopped using Linux on a machine with such a NIC for a long time!
> 
> Oh, that could be an explanation why i had 2 broken NE2000 NICs in
> the nineties.. ;)
> 
> >You should keep it and retry the JTAG adapter. You just need a boot loader
> >so if you manage to find a usable memory location that you can select by
> >soldering two pins together, you could manage to store it and bood from
> >another device.
> 
> I`m sure debricking with JTAG is possible and it would be an
> interesting task.
> Maybe i like to spend some money and time with this one day. For now
> i don`t.
> 
> In the meantime i`d rather being interested in what can be done to add more
> safety to the mtd tests.
> 
> By writing these lines and looking into the source, i started
> getting curious -  i
> see there is dev param with that module(s), but i did not pass a
> device number,
> nor did i pass anything to it.
> 
> in
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/mtd/tests/mtd_stresstest.c;h=63920476b57a24c7e61563303eb3abb773b73fdf;hb=7163cea15f7b362795b858e7c130cd617aecc0aa
> 
> i see:
> 
> static int dev;   <-!
> module_param(dev, int, S_IRUGO);
> MODULE_PARM_DESC(dev, "MTD device number to use");
> 
> static int count = 10000;
> module_param(count, int, S_IRUGO);
> MODULE_PARM_DESC(count, "Number of operations to do (default is 10000)");
> 
> and then
> 
> static int __init mtd_stresstest_init(void)
> {
> 	int err;
> 	int i, op;
> 	uint64_t tmp;
> 
> 	printk(KERN_INFO "\n");
> 	printk(KERN_INFO "=================================================\n");
> 	printk(PRINT_PREF "MTD device: %d\n", dev);
> 
> 	mtd = get_mtd_device(NULL, dev);
> 	if (IS_ERR(mtd)) {
> 		err = PTR_ERR(mtd);
> 		printk(PRINT_PREF "error: cannot get MTD device\n");
> 		return err;
> 	}
> 
> 
> 
> My kernel log showed:
> 
> mtd_stresstest: MTD device: 0
> mtd_stresstest: MTD device size 1048576 etc...
> 
> So, apparenly the module accidentally picked mtd0 instead of exiting
> cleanly (as
> i did not pass a device number)
> 
> I`m not a programmer, but doesn`t look that like an "unitialized
> variable" issue ?
> 
> If yes, then i would call my Dockstar "victim of a bug".

No, that's okay. Check http://en.wikipedia.org/wiki/.bss

I wonder though, if it makes sense to initialize it with -EINVAL or something,
so a user is forced to use the dev-module parameter?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [BUG] Re: mtd_stresstest module bricked my dockstar

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

> > Anyway - what would people think if linux had a kernel module  which wipes
> > /dev/sda1 when loaded ? :)

Yeah, the tests are destructive and they start erasing and torturing
your MTD device without preserving your data.

And the bad thing is - if you do not pass any parameter, they will
torture your mtd0.

I am sorry that you bricked your device and I hope you will be able to
resurrect it. This was an oversight. The tests were written for
validating new drivers and making sure they are robust. So we always
assumed people run them on a development HW, not on a production HW.
That was a false implicit assumption.

Yes, we should use -1 as the default value, and print a scary warning
message when the module is inserted without a parameter. We should
probably also put word "destructive" or something like this to the
module description.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [BUG] Re: mtd_stresstest module bricked my dockstar

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 510 bytes --]


> Yes, we should use -1 as the default value, and print a scary warning
> message when the module is inserted without a parameter. We should
> probably also put word "destructive" or something like this to the
> module description.

I would volunteer to do this tomorrow if nobody is working on it at this
moment...

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [BUG] Re: mtd_stresstest module bricked my dockstar

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

On Mon, 2011-10-24 at 23:24 +0200, Wolfram Sang wrote:
> > Yes, we should use -1 as the default value, and print a scary warning
> > message when the module is inserted without a parameter. We should
> > probably also put word "destructive" or something like this to the
> > module description.
> 
> I would volunteer to do this tomorrow if nobody is working on it at this

Please, go ahead. I'll try to pick them to my l2 mtd tree ASAP so that
they get merged this merge window. I guess we also want to cc
stable@kernel.org.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]