[PATCH] ARM: ioremap: fix boundary check when reusing static mapping

[linux@arm.linux.org.uk (Russell King - ARM Linux)]

On Sun, Jan 29, 2012 at 03:33:35PM +0100, Joachim Eastwood wrote:
> <1>ioremap: pfn=fffff phys=fffff000 offset=400 size=1000
> <1>ioremap: area c3ffdfc0: phys_addr=200000 pfn=200 size=4000
> <1>ioremap: found: addr fef74000 => fed73000 => fed73400

Ok, first thing is that patch 7304/1 is utter crap.  The reason is simple -
'size' here:

                if (__phys_to_pfn(area->phys_addr) > pfn ||
                    __pfn_to_phys(pfn) + offset + size-1 >
                    area->phys_addr + area->size-1)
                        continue;

_already_ contains 'offset', as we've only _just_ done this a few lines
above the loop:

        size = PAGE_ALIGN(offset + size);

So, 'size' includes the requested offset *already*.  So, Nicolas' original:

             if (__phys_to_pfn(area->phys_addr) > pfn ||
                 __pfn_to_phys(pfn) + size-1 > area->phys_addr + area->size-1)
                     continue;

was correct in the first place.  In any case, let's see what's going on
here:

                if (__phys_to_pfn(area->phys_addr) > pfn ||
                    __pfn_to_phys(pfn) + offset + size-1 >
                    area->phys_addr + area->size-1)
                        continue;

we have:

area->phys_addr = 0x00200000, __phys_to_pfn(area->phys_addr) = 0x00200
area->size = 0x4000, pfn = 0xfffff, offset = 0x400, size = 0x1000

So, this if condition becomes:

		if (0x00200 > 0xfffff ||
		    0xfffff000 + 0x400 + 0x1000-1 > 0x00200000 + 0x4000-1)
and therefore:
		if (0x00200 > 0xfffff ||
		    0x000003ff > 0x00203fff)

So, the if condition fails, and so we _believe_ that the SRAM mapping fits
our request.  Clearly that's totally bogus and crap.

Unfortunately, Pawel doesn't say what the problem he was trying to fix was,
all we have to go on is this totally crap commit comment:

    The condition checking boundaries of the requested and existing
    mappings didn't take in-page offset into consideration though,
    which lead to obscure and hard to debug problems when requested
    mapping crossed end of the static one.

In any case, I don't think it matters, and I'm going to schedule a revert
of that crap commit.  So, Joachim, well spotted.