modutils-2.3.15 'insmod'

modutils-2.3.15 'insmod'

[Richard B. Johnson]

modutils-2.3.15, and probably later, has a bug that can prevent
modules from being loaded from initrd, this results in not
being able to mount a root file-system. The bug assumes that
malloc() will return a valid pointer when given an allocation
size of zero.

When there are no modules loaded, insmod scans for modules
and allocates data using its xmalloc() based upon the number
of modules found. If the number was 0, it attempts to allocate
0 bytes (0 times the size of a structure). If malloc() returns
NULL (and it can, probably should), xmalloc() will write an
"out of memory" diagnostic and call exit().

The most recent `man` pages that RH 9.0 distributes states that
malloc() can return either NULL of a pointer that is valid for
free(). This, of course, depends upon the 'C' runtime library's
malloc() implementation.

#include <stdio.h>
#include <malloc.h>
int main(void);
int main()
{
    printf("%p\n", malloc(0));
    return 0;
}

It is likely that malloc(0) returning a valid pointer is a bug
that has prevented this problem from being observed. Such a
bug in malloc() is probably necessary to keep legacy software
running, but new software shouldn't use such atrocious side-effects.
An allocation of zero needs to be discovered and fixed early
in code design.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: modutils-2.3.15 'insmod'

[Andreas Schwab]

"Richard B. Johnson" <root@chaos.analogic.com> writes:

|> It is likely that malloc(0) returning a valid pointer is a bug
|> that has prevented this problem from being observed.

It's not a bug, it's a behaviour explicitly allowed by the C standard.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: modutils-2.3.15 'insmod'

[Bill Rugolsky Jr.]

On Wed, Jul 09, 2003 at 05:45:22PM +0200, Andreas Schwab wrote:
> "Richard B. Johnson" <root@chaos.analogic.com> writes:
> 
> |> It is likely that malloc(0) returning a valid pointer is a bug
> |> that has prevented this problem from being observed.
> 
> It's not a bug, it's a behaviour explicitly allowed by the C standard.
 
... and has long been used to generate unique cookies of pointer type.

Regards,

	Bill Rugolsky

Re: modutils-2.3.15 'insmod'

[H. Peter Anvin]

Followup to:  <jer84zln59.fsf@sykes.suse.de>
By author:    Andreas Schwab <schwab@suse.de>
In newsgroup: linux.dev.kernel
>
> "Richard B. Johnson" <root@chaos.analogic.com> writes:
> 
> |> It is likely that malloc(0) returning a valid pointer is a bug
> |> that has prevented this problem from being observed.
> 
> It's not a bug, it's a behaviour explicitly allowed by the C standard.
> 

The bug is in xmalloc, meaning that it assumes that returning NULL is
always an error.  Presumably xmalloc should look *either* like:

void *xmalloc(size_t s)
{
	void *p = malloc(s);
	
	if ( !p && s )
		barf();
	else
		return p;
}

... or ...

void *xmalloc(size_t s)
{
	void *p;

	/* Always return a valid allocation */
	if ( s == 0 ) s = 1;
	p = malloc(s);
	
	if ( !p )
		barf();
	else
		return p;
}
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64

Re: modutils-2.3.15 'insmod'

[Richard B. Johnson]

On Wed, 9 Jul 2003, H. Peter Anvin wrote:

> Followup to:  <jer84zln59.fsf@sykes.suse.de>
> By author:    Andreas Schwab <schwab@suse.de>
> In newsgroup: linux.dev.kernel
> >
> > "Richard B. Johnson" <root@chaos.analogic.com> writes:
> >
> > |> It is likely that malloc(0) returning a valid pointer is a bug
> > |> that has prevented this problem from being observed.
> >
> > It's not a bug, it's a behaviour explicitly allowed by the C standard.
> >
>
> The bug is in xmalloc, meaning that it assumes that returning NULL is
> always an error.  Presumably xmalloc should look *either* like:
>
> void *xmalloc(size_t s)
> {
> 	void *p = malloc(s);
>
> 	if ( !p && s )
> 		barf();
> 	else
> 		return p;
> }
>
> ... or ...
>
> void *xmalloc(size_t s)
> {
> 	void *p;
>
> 	/* Always return a valid allocation */
> 	if ( s == 0 ) s = 1;
> 	p = malloc(s);
>
> 	if ( !p )
> 		barf();
> 	else
> 		return p;
> }

You are correct that the bug is in xmalloc(). However, I think the
true bug is that xmalloc() exists! Malloc should be called directly
and any special cases for that specific call should be handled at
that time.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: modutils-2.3.15 'insmod'

[Kurt Wall]

Quoth Richard B. Johnson:
> 
> modutils-2.3.15, and probably later, has a bug that can prevent
> modules from being loaded from initrd, this results in not
> being able to mount a root file-system. The bug assumes that
> malloc() will return a valid pointer when given an allocation
> size of zero.

This isn't a bug. The standard allow returning a non-null pointer
for malloc(0).

> When there are no modules loaded, insmod scans for modules
> and allocates data using its xmalloc() based upon the number
> of modules found. If the number was 0, it attempts to allocate
> 0 bytes (0 times the size of a structure). If malloc() returns
> NULL (and it can, probably should), xmalloc() will write an
> "out of memory" diagnostic and call exit().
> 
> The most recent `man` pages that RH 9.0 distributes states that
> malloc() can return either NULL of a pointer that is valid for
> free(). This, of course, depends upon the 'C' runtime library's
> malloc() implementation.

Perhaps, but IIRC, the rationale in the GNU C library was that
existing programs assume malloc(0) != 0, which allows you to call
realloc on the pointer. Returning NULL only makes sense if the 
malloc() call fails.

> It is likely that malloc(0) returning a valid pointer is a bug
> that has prevented this problem from being observed. Such a
> bug in malloc() is probably necessary to keep legacy software

Not a bug. Bad design perhaps, but not a bug.

> running, but new software shouldn't use such atrocious side-effects.
> An allocation of zero needs to be discovered and fixed early
> in code design.

Kurt
-- 
If you sit down at a poker game and don't see a sucker, get up.  You're
the sucker.

Re: modutils-2.3.15 'insmod'

[Andy Isaacson]

On Wed, Jul 09, 2003 at 12:08:23PM -0400, Kurt Wall wrote:
> Quoth Richard B. Johnson:
> > modutils-2.3.15, and probably later, has a bug that can prevent
> > modules from being loaded from initrd, this results in not
> > being able to mount a root file-system. The bug assumes that
> > malloc() will return a valid pointer when given an allocation
> > size of zero.
> 
> This isn't a bug. The standard allow returning a non-null pointer
> for malloc(0).

It's not literally a bug in libc -- the C standard says it's
implementation-defined whether malloc(0) returns NULL or a cookie -- but
it is definitely a bug (in a portable program) to depend on either
behavior from libc.  See ISO/IEC 9899:1999 7.20.3 paragraph 1.

> > The most recent `man` pages that RH 9.0 distributes states that
> > malloc() can return either NULL of a pointer that is valid for
> > free(). This, of course, depends upon the 'C' runtime library's
> > malloc() implementation.
> 
> Perhaps, but IIRC, the rationale in the GNU C library was that
> existing programs assume malloc(0) != 0, which allows you to call
> realloc on the pointer. Returning NULL only makes sense if the 
> malloc() call fails.

This paragraph is nonsensical, because realloc(malloc(0), 10) is
allowed, regardless of whether malloc(0) returns NULL or a cookie.
realloc(NULL, n) is allowed, and defined to be identical to malloc(n).
7.20.3.4 paragraph 3.

Geez, why does a trivial post about a bug in some program have to turn
into a pile of misleading statements and citations to ISO documents?

-andy

Re: modutils-2.3.15 'insmod'

[Keith Owens]

On Wed, 9 Jul 2003 11:25:11 -0400 (EDT), 
"Richard B. Johnson" <root@chaos.analogic.com> wrote:
>modutils-2.3.15, and probably later, has a bug that can prevent
>modules from being loaded from initrd, this results in not
>being able to mount a root file-system. The bug assumes that
>malloc() will return a valid pointer when given an allocation
>size of zero.

Sigh :(  Fixed over two years ago.

modutils Changelog.  2001-05-06 modutils 2.4.6

	* Do not assume that malloc(0) returns a pointer.  Bug report by
	  Kiichiro Naka, different fix by Keith Owens.

void *
xmalloc(size_t size)
{
  void *ptr = malloc(size ? size : 1);
  if (!ptr)
    {
      error("Out of memory");
      exit(1);
    }
  return ptr;
}