flashrom: Fix error -EINVAL on mmap()

[Segher Boessenkool]

Don't calculate "flash_baseaddr" until the final value of "size"
is known, otherwise we end up trying to map a page right after
end of memory.

Fixes #112.

Signed-off-by: Segher Boessenkool 
---
 flashrom.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

Index: flashrom/flashrom.c
===================================================================
--- flashrom.orig/flashrom.c
+++ flashrom/flashrom.c
@@ -116,14 +116,6 @@

size = flash->total_size * 1024;

-#ifdef TS5300
-		// FIXME: Wrong place for this decision
-		// FIXME: This should be autodetected. It is trivial.
-		flash_baseaddr = 0x9400000;
-#else
-		flash_baseaddr = (0xffffffff - size + 1);
-#endif
-
    	/* If getpagesize() > size -> 
    	 * "Can't mmap memory using /dev/mem: Invalid argument"
    	 * This should never happen as we don't support any flash chips
@@ -139,6 +131,14 @@
    		size = getpagesize();
    	}

+#ifdef TS5300
+		// FIXME: Wrong place for this decision
+		// FIXME: This should be autodetected. It is trivial.
+		flash_baseaddr = 0x9400000;
+#else
+		flash_baseaddr = (0xffffffff - size + 1);
+#endif
+
    	bios = mmap(0, size, PROT_WRITE | PROT_READ, MAP_SHARED,
    		    fd_mem, (off_t) flash_baseaddr);
    	if (bios == MAP_FAILED) {

[ron minnich]

if somebody has tested this, and it looks good to me, I'll ack it. Do
we have one test? I know you were discussing it on IRC.

Thanks Segher!

ron

[Segher Boessenkool]

> if somebody has tested this, and it looks good to me, I'll ack it. Do
> we have one test? I know you were discussing it on IRC.

The reporter of the bug tested it, and it fixed the problem.

So, _does_ it look good to you?  ;-)

Segher

[Sean Nelson]

Segher Boessenkool wrote:
> > if somebody has tested this, and it looks good to me, I'll ack it. Do
> > we have one test? I know you were discussing it on IRC.
>
> The reporter of the bug tested it, and it fixed the problem.
>
> So, _does_ it look good to you? ;-)
>
> Segher

looks good, and flashrom still works for me.

Acked-by: Sean Nelson

[Paul Menzel]

Dear list,

Am Dienstag, den 12.08.2008, 07:53 +0200 schrieb Segher Boessenkool:
> Don't calculate "flash_baseaddr" until the final value of "size"
> is known, otherwise we end up trying to map a page right after
> end of memory.
> Index: flashrom/flashrom.c
>
> --- flashrom.orig/flashrom.c
> +++ flashrom/flashrom.c
> @@ -116,14 +116,6 @@
> size = flash->total_size * 1024;
>
> -#ifdef TS5300
> // FIXME: Wrong place for this decision
> // FIXME: This should be autodetected. It is trivial.
> flash_baseaddr = 0x9400000;
>
> -#else
> flash_baseaddr = (0xffffffff - size + 1);
>
> -#endif
> /* If getpagesize() > size ->
> "Can't mmap memory using /dev/mem: Invalid argument"
> This should never happen as we don't support any flash chips
>
> @@ -139,6 +131,14 @@
>    		size = getpagesize();
>    	}
>
> +#ifdef TS5300
> // FIXME: Wrong place for this decision
> // FIXME: This should be autodetected. It is trivial.
> flash_baseaddr = 0x9400000;
>
> +#else
> flash_baseaddr = (0xffffffff - size + 1);
>
> +#endif
> bios = mmap(0, size, PROT_WRITE | PROT_READ, MAP_SHARED,
>   	    fd_mem, (off_t) flash_baseaddr);
>   if (bios == MAP_FAILED) {

How do I apply it?

1. Check out flashrom [1].
2. Save the text under including the line with Index: of your message in
fix.diff in flashrom/.
3. flashrom$ patch -p1 <fix.diff 
patching file flashrom.c
Hunk #1 FAILED at 116.
1 out of 1 hunk FAILED -- saving rejects to file flashrom.c.rej
missing header for unified diff at line 21 of patch
can't find file to patch at input line 21
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|flash chips
--------------------------
File to patch:

Sorry for my ignorance.

Paul

[1] http://www.coreboot.org/Flashrom#Installation

[Stefan Reinauer]

Segher Boessenkool wrote:
> Don't calculate "flash_baseaddr" until the final value of "size"
> is known, otherwise we end up trying to map a page right after
> end of memory.
>
> Fixes #112.
>
> Signed-off-by: Segher Boessenkool

Acked-by: Stefan Reinauer
> flashrom.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> Index: flashrom/flashrom.c
>
> --- flashrom.orig/flashrom.c
> +++ flashrom/flashrom.c
> @@ -116,14 +116,6 @@
> size = flash->total_size * 1024;
>
> -#ifdef TS5300
> // FIXME: Wrong place for this decision
> // FIXME: This should be autodetected. It is trivial.
> flash_baseaddr = 0x9400000;
>
> -#else
> flash_baseaddr = (0xffffffff - size + 1);
>
> -#endif
> /* If getpagesize() > size ->
> "Can't mmap memory using /dev/mem: Invalid argument"
> This should never happen as we don't support any flash chips
>
> @@ -139,6 +131,14 @@
>    		size = getpagesize();
>    	}
>
> +#ifdef TS5300
> // FIXME: Wrong place for this decision
> // FIXME: This should be autodetected. It is trivial.
> flash_baseaddr = 0x9400000;
>
> +#else
> flash_baseaddr = (0xffffffff - size + 1);
>
> +#endif
> bios = mmap(0, size, PROT_WRITE | PROT_READ, MAP_SHARED,
>   	    fd_mem, (off_t) flash_baseaddr);
>   if (bios == MAP_FAILED) {

[Segher Boessenkool]

> > Don't calculate "flash_baseaddr" until the final value of "size"
> > is known, otherwise we end up trying to map a page right after
> > end of memory.
> >
> > Fixes #112.
> >
> > Signed-off-by: Segher Boessenkool
>
> Acked-by: Stefan Reinauer

Committed, r3502.

Segher