linux-kernel - Re: [PATCH] Fix crash in viafb due to 4k stack overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0811091439310.22998@t2.domain.actdsltmp>
Date:	Sun, 9 Nov 2008 14:57:59 -0800 (PST)
From:	Trent Piepho <xyzzy@...akeasy.org>
To:	Bruno Prémont <bonbons@...ux-vserver.org>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Arjan van de Ven <arjan@...radead.org>,
	JosephChan@....com.tw, linux-fbdev-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix crash in viafb due to 4k stack overflow

On Sun, 9 Nov 2008, Bruno Prémont wrote:
> What is preferred, allocating a big block of memory and point various
> variables inside that block or do multiple individual allocations?
>
> e.g.
>  u8 data[CURSOR_SIZE/8]
>  u32 data_bak[CURSOR_SIZE/32]
> =>
>  u8 *data = kzalloc(...)
>  u32 *data_bak = kzalloc(...)
> or
>  u8 *data = kzalloc(CURSOR_SIZE/8 + CURSOR_SIZE/32, ...)
>  u32 *data_bak = (u32*)(data+CURSOR_SIZE/8);
>
> First option is more readable, second should be more efficient...

I like this method:

foo()
{
 	/* anonymous struct, you don't need to think of a name */
 	struct {
 		u8 data[CURSOR_SIZE/8];
 		u32 data_bak[CURSOR_SIZE/32];
 	} *cursor;

 	cursor = kzalloc(sizeof(*cursor), ...);

 	/* for remaining code:
 	 * s/data/cursor->data/
 	 * s/data_bak/cursor->data_bak/
 	 */

 	free(cursor);
}

A number of advantages over multiple allocations:
The alloc code and free code only has to run once.

Likely less memory will be allocated, due to allocation granularity.

Only one pointer is needed to keep track of the allocations instead of two. 
This frees up a pointer's worth of stack space and/or another register for
the compiler to use.

More readable than u32 *data_bak = (u32*)(data+CURSOR_SIZE/8) and so on.

If you check for kzalloc failure, you only need code to check once.  And
you can avoid the need for rolling back the first allocation if the second
fails.

The disadvantage is that you can't free just one of the allocations and big
allocations are harder to satisfy than small ones.  When you get up to the
megabyte range, combining allocations into bigger ones becomes a bad idea.