[PATCH] modpost: fix buffer overflow

[PATCH] modpost: fix buffer overflow

[Jiri Benc]

I got SIGABRT in modpost when building a module really deeply nested in
a filesystem (path > 100 chars):

>   Building modules, stage 2.
>   MODPOST
> *** glibc detected *** scripts/mod/modpost: realloc(): invalid next size: 0x0809f588 ***
> [...]

This patch fixes that problem.

Signed-off-by: Jiri Benc <jbenc@suse.cz>

--- linux-2.6.16-rc6.orig/scripts/mod/modpost.c
+++ linux-2.6.16-rc6/scripts/mod/modpost.c
@@ -552,7 +552,7 @@ void __attribute__((format(printf, 2, 3)
 	
 	va_start(ap, fmt);
 	len = vsnprintf(tmp, SZ, fmt, ap);
-	if (buf->size - buf->pos < len + 1) {
+	while (buf->size - buf->pos < len + 1) {
 		buf->size += 128;
 		buf->p = realloc(buf->p, buf->size);
 	}


-- 
Jiri Benc
SUSE Labs

Re: [PATCH] modpost: fix buffer overflow

[Bernd Petrovitsch]

On Wed, 2006-03-15 at 15:44 +0100, Jiri Benc wrote:
> I got SIGABRT in modpost when building a module really deeply nested in
> a filesystem (path > 100 chars):
> 
> >   Building modules, stage 2.
> >   MODPOST
> > *** glibc detected *** scripts/mod/modpost: realloc(): invalid next size: 0x0809f588 ***
> > [...]
> 
> This patch fixes that problem.
> 
> Signed-off-by: Jiri Benc <jbenc@suse.cz>
> 
> --- linux-2.6.16-rc6.orig/scripts/mod/modpost.c
> +++ linux-2.6.16-rc6/scripts/mod/modpost.c
> @@ -552,7 +552,7 @@ void __attribute__((format(printf, 2, 3)
>  	
>  	va_start(ap, fmt);
>  	len = vsnprintf(tmp, SZ, fmt, ap);
> -	if (buf->size - buf->pos < len + 1) {
> +	while (buf->size - buf->pos < len + 1) {
>  		buf->size += 128;
>  		buf->p = realloc(buf->p, buf->size);
>  	}

Are you sure you won't pull the line with realloc() out of the loop and
do it once afterwards?

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: [PATCH] modpost: fix buffer overflow

[Bernd Petrovitsch]

On Wed, 2006-03-15 at 15:44 +0100, Jiri Benc wrote:
> I got SIGABRT in modpost when building a module really deeply nested in
> a filesystem (path > 100 chars):
> 
> >   Building modules, stage 2.
> >   MODPOST
> > *** glibc detected *** scripts/mod/modpost: realloc(): invalid next size: 0x0809f588 ***
> > [...]
> 
> This patch fixes that problem.
> 
> Signed-off-by: Jiri Benc <jbenc@suse.cz>
> 
> --- linux-2.6.16-rc6.orig/scripts/mod/modpost.c
> +++ linux-2.6.16-rc6/scripts/mod/modpost.c
> @@ -552,7 +552,7 @@ void __attribute__((format(printf, 2, 3)
>  	
>  	va_start(ap, fmt);
>  	len = vsnprintf(tmp, SZ, fmt, ap);
> -	if (buf->size - buf->pos < len + 1) {
> +	while (buf->size - buf->pos < len + 1) {
>  		buf->size += 128;
>  		buf->p = realloc(buf->p, buf->size);
>  	}

Silly me. To make it more obvious whatz I really meant was:
----  snip  ----
	if (buf->size - buf->pos < len + 1) {
		while (buf->size - buf->pos < len + 1)
			buf->size += 128;
		buf->p = realloc(buf->p, buf->size);
	}
----  snip  ----

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: [PATCH] modpost: fix buffer overflow

[Jiri Benc]

On Wed, 15 Mar 2006 15:57:28 +0100, Bernd Petrovitsch wrote:
> Silly me. To make it more obvious whatz I really meant was:
> ----  snip  ----
> 	if (buf->size - buf->pos < len + 1) {
> 		while (buf->size - buf->pos < len + 1)
> 			buf->size += 128;
> 		buf->p = realloc(buf->p, buf->size);
> 	}
> ----  snip  ----

Yes, this is probably better. New version of the patch follows.

---->8----

I got SIGABRT in modpost when compiling a module really deeply nested in
a filesystem (path > 100 chars):

>   Building modules, stage 2.
>   MODPOST
> *** glibc detected *** scripts/mod/modpost: realloc(): invalid next size: 0x0809f588 ***
> [...]

This patch fixes that problem.

Signed-off-by: Jiri Benc <jbenc@suse.cz>

--- linux-2.6.16-rc6.orig/scripts/mod/modpost.c
+++ linux-2.6.16-rc6/scripts/mod/modpost.c
@@ -553,7 +553,8 @@ void __attribute__((format(printf, 2, 3)
 	va_start(ap, fmt);
 	len = vsnprintf(tmp, SZ, fmt, ap);
 	if (buf->size - buf->pos < len + 1) {
-		buf->size += 128;
+		while (buf->size - buf->pos < len + 1)
+			buf->size += 128;
 		buf->p = realloc(buf->p, buf->size);
 	}
 	strncpy(buf->p + buf->pos, tmp, len + 1);


-- 
Jiri Benc
SUSE Labs

Re: [PATCH] modpost: fix buffer overflow

[Andreas Schwab]

Jiri Benc <jbenc@suse.cz> writes:

> +		while (buf->size - buf->pos < len + 1)
> +			buf->size += 128;

What's wrong with

                buf->size = buf->pos + len + 1;

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] modpost: fix buffer overflow

[Sam Ravnborg]

On Wed, Mar 15, 2006 at 04:08:58PM +0100, Jiri Benc wrote:
> I got SIGABRT in modpost when compiling a module really deeply nested in
> a filesystem (path > 100 chars):
> 
> >   Building modules, stage 2.
> >   MODPOST
> > *** glibc detected *** scripts/mod/modpost: realloc(): invalid next size: 0x0809f588 ***
> > [...]
> 
> This patch fixes that problem.
> 
> Signed-off-by: Jiri Benc <jbenc@suse.cz>
> 
> --- linux-2.6.16-rc6.orig/scripts/mod/modpost.c
> +++ linux-2.6.16-rc6/scripts/mod/modpost.c
> @@ -553,7 +553,8 @@ void __attribute__((format(printf, 2, 3)
>  	va_start(ap, fmt);
>  	len = vsnprintf(tmp, SZ, fmt, ap);
>  	if (buf->size - buf->pos < len + 1) {
> -		buf->size += 128;
> +		while (buf->size - buf->pos < len + 1)
> +			buf->size += 128;
>  		buf->p = realloc(buf->p, buf->size);
>  	}
>  	strncpy(buf->p + buf->pos, tmp, len + 1);

Hi Jiri.

Can I ask you to make a new patch where you change buf_printf() to use
buf_write. And then change buf_write to allocate in chunks also.
This would be cleanest solution.

	Sam

Re: [PATCH] modpost: fix buffer overflow

[Jiri Benc]

On Wed, 15 Mar 2006 23:51:59 +0100, Sam Ravnborg wrote:
> Can I ask you to make a new patch where you change buf_printf() to use
> buf_write. And then change buf_write to allocate in chunks also.
> This would be cleanest solution.

This probably will be the cleanest solution, but I doubt it would be
acceptable for 2.6.16. And I think the fix should go into 2.6.16.

Thanks,

-- 
Jiri Benc
SUSE Labs

Re: [PATCH] modpost: fix buffer overflow

[Sam Ravnborg]

On Thu, Mar 16, 2006 at 02:21:14PM +0100, Jiri Benc wrote:
> On Wed, 15 Mar 2006 23:51:59 +0100, Sam Ravnborg wrote:
> > Can I ask you to make a new patch where you change buf_printf() to use
> > buf_write. And then change buf_write to allocate in chunks also.
> > This would be cleanest solution.
> 
> This probably will be the cleanest solution, but I doubt it would be
> acceptable for 2.6.16. And I think the fix should go into 2.6.16.

Like this...

	Sam

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 30f3ac8..0b92ddf 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -923,19 +923,14 @@ void __attribute__((format(printf, 2, 3)
 
 	va_start(ap, fmt);
 	len = vsnprintf(tmp, SZ, fmt, ap);
-	if (buf->size - buf->pos < len + 1) {
-		buf->size += 128;
-		buf->p = realloc(buf->p, buf->size);
-	}
-	strncpy(buf->p + buf->pos, tmp, len + 1);
-	buf->pos += len;
+	buf_write(buf, tmp, len);
 	va_end(ap);
 }
 
 void buf_write(struct buffer *buf, const char *s, int len)
 {
 	if (buf->size - buf->pos < len) {
-		buf->size += len;
+		buf->size += len + SZ;
 		buf->p = realloc(buf->p, buf->size);
 	}
 	strncpy(buf->p + buf->pos, s, len);

[PATCH] kbuild: fix buffer overflow in modpost

[Sam Ravnborg]

Hi Linus - please apply to 2.6.16-rc

Jiri Benc <jbenc@suse.cz> reported that modpost would stop with SIGABRT if
used with long filepaths.
The error looked like:
>   Building modules, stage 2.
>   MODPOST
> *** glibc detected *** scripts/mod/modpost: realloc(): invalid next size:
+0x0809f588 ***
> [...]

Following patch fixes this by allocating at least the required
memory + SZ bytes each time. Before we sometimes ended up allocating
too little memory resuting in the glibc detected bug above.
Based on patch originally submitted by: Jiri Benc <jbenc@suse.cz>

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index f70ff13..b8b2a56 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -508,12 +508,7 @@ buf_printf(struct buffer *buf, const cha
 	
 	va_start(ap, fmt);
 	len = vsnprintf(tmp, SZ, fmt, ap);
-	if (buf->size - buf->pos < len + 1) {
-		buf->size += 128;
-		buf->p = realloc(buf->p, buf->size);
-	}
-	strncpy(buf->p + buf->pos, tmp, len + 1);
-	buf->pos += len;
+	buf_write(buf, tmp, len);
 	va_end(ap);
 }
 
@@ -521,7 +516,7 @@ void
 buf_write(struct buffer *buf, const char *s, int len)
 {
 	if (buf->size - buf->pos < len) {
-		buf->size += len;
+		buf->size += len + SZ;
 		buf->p = realloc(buf->p, buf->size);
 	}
 	strncpy(buf->p + buf->pos, s, len);