[uml-devel] Little heap overflow in mconsole_kern.c

[uml-devel] Little heap overflow in mconsole_kern.c

[BlaisorBlade]

In that file(releases 2.4.22-3um and -4um at least), we have this code:
[...]

void mconsole_interrupt(int irq, void *dev_id, struct pt_regs *regs)
{
	int fd;
	struct mconsole_entry *new;
	struct mc_request req;

[...]
			new = kmalloc(sizeof(req), GFP_ATOMIC);
			if(new == NULL)
				mconsole_reply(&req, "Out of memory", 1, 0);
			else {
				new->request = req;
				list_add(&new->list, &mc_requests);
			}
[...]
While new points to a mconsole_entry, the code allocates sizeof(mc_request), 
not sizeof(mconsole_entry), which is less than needed. I've looked for any 
reasons for this to be correct(even in the weird Linux list implementation), 
but seems just a typo(not noticed since the actual overflow is unlikely to 
happen, but it's there). Could you change it?(I didn't post a patch because 
it's simpler to edit the code at hand).
-- 
cat <<EOSIGN
Paolo Giarrusso, aka Blaisorblade
Linux Kernel 2.4.21/2.6.0-test on an i686; Linux registered user n. 292729
EOSIGN




-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Little heap overflow in mconsole_kern.c

[Jeff Dike]

blaisorblade_spam@yahoo.it said:
> While new points to a mconsole_entry, the code allocates
> sizeof(mc_request),  not sizeof(mconsole_entry)

Gawd, someone really needs to shoot me before I destroy something...

Needless to say, that is fixed.  Nice spotting.

BTW, It would be a good exercise for someone to go through the code and 
replace instances of
	foo = kmalloc(sizeof(type-name), ...)
with
	foo = kmalloc(sizeof(*foo), ...)

That would have prevented this bug.

				Jeff



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Little heap overflow in mconsole_kern.c

[Geoff Thorpe]

On September 17, 2003 08:22 pm, Jeff Dike wrote:
> blaisorblade_spam@yahoo.it said:
> > While new points to a mconsole_entry, the code allocates
> > sizeof(mc_request),  not sizeof(mconsole_entry)
>
> Gawd, someone really needs to shoot me before I destroy something...

Don't feel bad, I've come down with this bug a few times.

> BTW, It would be a good exercise for someone to go through the code and
> replace instances of
> 	foo = kmalloc(sizeof(type-name), ...)
> with
> 	foo = kmalloc(sizeof(*foo), ...)
>
> That would have prevented this bug.

FWIW: after having this problem one too many times with malloc and 
friends, I started using macros of the form;
  #define MYMALLOC(t,n)  (t *)malloc((n) * sizeof(t))
  #define MYFREE(t,p)    do { \
                             t *_tmp_4567 = (p); \
                             free(_tmp_4567); \
                         } while(0)
These get compiled down to malloc() and free(), respectively. However 
MYMALLOC has the compilation-time advantage of being typesafe according 
to the return value (not void*) and size (which matches the return type). 
The free() variant may seem a bit overkill, but tastes vary. Anyway, if 
you adopt something like this then you can consider any direct use of 
'malloc' and 'free' in the source as bugs and your audit becomes quite a 
bit easier. $0.02, etc.

Cheers,
Geoff

-- 
Geoff Thorpe
geoff@geoffthorpe.net
http://www.geoffthorpe.net/



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel