[PATCH 1/2] mtdpart: Avoid divide-by-zero on out-of-reach path

[Atsushi Nemoto]

On Tue, 17 Jun 2008 18:46:29 +0200, Jörn Engel <joern at logfs.org> wrote:
> > No, the second loop go forwards.  It searches maximum erasesize of the
> > partition.
> 
> Indeed, it does.  And if I change the order in the conditions, those
> make sense even to me.  Hm.
> 
> But what purpose does the 'i--' serve?  Why would we want to check an
> eraseregion _before_ the start of the partition?

The decrement is there because when the first loop stoped 'i' points
the second region in the partition.

> Next, you could simply add code like this below the second loop:
> 	/* In case no eraseregion matched. */
> 	if (slave->mtd.erasesize == 0)
> 		slave->mtd.erasesize = master->erasesize;

Yes, it should work.

> But under which conditions would we ever run into this?  It appears as
> if those conditions would be better served with either 'BUG();' or
> 'return -EINVAL;'.
> 
> I am still rather confused by all this.

The keyword is 'out-of-reach path'.  I found this when I expected 8MB
flash and try to split into two 4MB chip, but 2MB chip was actually
mounted on the board socket.

The mtdpart detect this condition and disable the partition by setting
zero to mtd.size.  And fall through to code we are talking about.

	if (slave->offset >= master->size) {
			/* let's register it anyway to preserve ordering */
		slave->offset = 0;
		slave->mtd.size = 0;
		printk ("mtd: partition \"%s\" is out of reach -- disabled\n",
			parts[i].name);
	}

Then, the body of the second loop we are talking about will never
executed.  And finally 'slave->offset % slave->mtd.erasesize' will
cause division-by-zero.

Putting 'goto somewhere' or 'slave->mtd.erasesize = master->erasesize'
in the above 'if' block can be an alternative fix.

---
Atsushi Nemoto