mmap bug (all archs) w/ fix

mmap bug (all archs) w/ fix

[Michael Galassi]

When we need to update flash on our system we use a utility which opens
/dev/mem, mmaps the region of physical memory which contains flash (top
128meg), and then does all the ugly magic to erase/write/verify.  As we
were implementing this we discovered that it is not possible to mmap a
chunk of physical addresses which ends at the top of the proc's space
(where we (and everyone else on this list) have our boot loader).  I
finally got around to looking at why this might be and found that
whoever implemented mmap got lazy.  The diff which follows is against a
fully patched MontaVista pro 4.0.1 kernel, I just checked and the exact
same fix is applicable to 2.6.20-rc6.

Index: mmap.c
===================================================================
RCS file: /CVSR1/SuperQAM/SW/src/sys/mvl401/mm/mmap.c,v
retrieving revision 1.1
retrieving revision 1.2
diff -w -u -r1.1 -r1.2
--- mmap.c      9 Aug 2006 02:26:09 -0000       1.1
+++ mmap.c      30 Jan 2007 23:42:53 -0000      1.2
@@ -803,7 +803,8 @@
                return -ENOMEM;

        /* offset overflow? */
-       if ((pgoff + (len >> PAGE_SHIFT)) < pgoff)
+       if ((pgoff + (len >> PAGE_SHIFT)) < pgoff &&
+           (pgoff + (len >> PAGE_SHIFT)) != 0)
                return -EOVERFLOW;

        /* Too many mappings? */

This clearly applies to all architectures and all versions (at least 2.4
onward), could someone point me at some document which describes how to
submit patches to "the right place" please?  I really do not want to
support any fixes myself so getting fixes to obvious bugs merged into
the mailine code would be a good thing (tm).

Thanks,

-michael

Re: mmap bug (all archs) w/ fix

[Dan Malek]

On Jan 30, 2007, at 6:54 PM, Michael Galassi wrote:

> When we need to update flash on our system we use a utility which  
> opens
> /dev/mem, mmaps the region of physical memory which contains flash  
> (top
> 128meg), and then does all the ugly magic to erase/write/verify.

MTD does this very nicely for you, and you don't have
to write any special programs......  :-)  It's also much safer.

	-- Dan

Re: mmap bug (all archs) w/ fix

[Michael Galassi]

>MTD does this very nicely for you, and you don't have
>to write any special programs......  :-)

MTD does have the ability to do this.  One might argue about the
philosophical merits of doing in kernel space (hard to debug) what can
trivially be done in user space (trivial to debug).  One might also
argue that the mtd code (I've perused it in some detail) takes a rather
cavalier approach to error checking and is thus not optimal for use in
real systems where reliability, dependability, and availability matter.

While our heroic "one" is busy arguing, I'll take a moment and point out
that the Linux kernel harbors a bug, the fix is trivial.  As long as the
argument still rages, I'll point out that advice from the "don't fix the
bug, work around it" crowd doesn't carry that much weight with the
paranoid amongst us.

>It's also much safer.

Under what circumstance is buggy kernel code safer than buggy user code?

-michael

Re: mmap bug (all archs) w/ fix

[Gerhard Jaeger]

On Mittwoch, 31. Januar 2007, Michael Galassi wrote:
[SNIPSNAP]
> This clearly applies to all architectures and all versions (at least 2.4
> onward), could someone point me at some document which describes how to
> submit patches to "the right place" please?  I really do not want to
> support any fixes myself so getting fixes to obvious bugs merged into
> the mailine code would be a good thing (tm).
> 
IIRC, the problem is not the code that you try to fix, the problem is
the mmap call, which uses a signed 32 bit integer value for the offset
argument. And this one bites you when having addresses to map > 
0x80000000 (I assume that you try to mmap an area somewhere @
0xF8000000-0xFFFFFFFF).

Try to use mmap2 call which overcomes this problem by using the page
size as unit of the offset argument instead of bytes.

So no patch is needed and you should be able ti map the flash.

HTH
Gerhard

-- 
Gerhard Jaeger <gjaeger@sysgo.com>            
SYSGO AG                      Embedded and Real-Time Software
www.sysgo.com | www.elinos.com | www.pikeos.com | www.osek.de