Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 3 Apr 2000 02:38:58 -0700
From:      Alfred Perlstein <bright@wintelcom.net>
To:        Hellmuth Michaelis <hm@hcs.de>
Cc:        Bruce Evans <bde@zeta.org.au>, garyj@muc.de, freebsd-current@FreeBSD.ORG
Subject:   Re: MLEN and crashes
Message-ID:  <20000403023858.F21029@fw.wintelcom.net>
In-Reply-To: <20000403084330.6A6DC483D@hcswork.hcs.de>; from hm@hcs.de on Mon, Apr 03, 2000 at 10:43:30AM %2B0200
References:  <Pine.BSF.4.21.0004022118510.1442-100000@alphplex.bde.org> <20000403084330.6A6DC483D@hcswork.hcs.de>

next in thread | previous in thread | raw e-mail | index | archive | help
* Hellmuth Michaelis <hm@hcs.de> [000403 02:12] wrote:
> >From the keyboard of Bruce Evans:
> 
> > It's just a bug to allocate big structs on the kernel stack.
> 
> Please specify "big"! :-)

have a look at src/sys/nfs/nfs_vnops.c:

line ~2787:
#ifndef NFS_COMMITBVECSIZ
#define NFS_COMMITBVECSIZ	20
#endif
	struct buf *bvec_on_stack[NFS_COMMITBVECSIZ];
	int bvecsize = 0, bveccount;

I guess 80 bytes is pushing it, some routines are worse, but 2k
is totally out of line.

If you're worried about such things happening then you can use
the pre-processor to catch things that may make your structures
too large.

> I wonder how too "big" can be detected. The code in question is perfectly
> valid syntactically and semantically correct C-code.

There's code that #error (maybe #warning?) when it can detect that the
size of a struct has been unreasonably grown, it's done via the
pre-processor.

> 
> If a piece of code being considered buggy depends on the size of some
> externally defined "thing" (like the kernel stack size in this case)
> something - IMHO - is not in a good condition.
> 
> Perhaps a sentence like "It's just a bug to allocate structs on the
> kernel stack" (note the missing "big") has to be put in style. Or a
> macro like (this is just a cheap shoot in the dark)
> 
> void
> kernel_subr(void)
> {
> 	ALLOCATE_STRUCT_ON_STACK(struct bigstruct bigstr);
> ....
> 
> could be created to check for potentially stack overflow at compile
> time and/or runtime and people are forced in style(9) to use it.

noooooooooooooooooo :)

> > 512 bytes is probably too big for i386's now.  The effective kernel stack
> > size was shrunk by about 2K by the sigset_t changes, so there is only
> > about 5K of kernel stack.
> 
> Then isn't it time to make the kernel stack larger ?
> 
> Please don't misunderstand. I can fully accept accecpt and acknowledge what
> you write (i've converted the piece of code in question to malloc'ing its
> data already), i'm just a bit concerned because its so perfectly legal and
> common to allocate a struct on the stack that i fear just that i and 
> probably other don't think about it if it were not made absolutely clear
> and possibly enforced somehow to never do this in kernel land.

When in doubt, ask.  Design and Implementation clearly explains the
kernel stack program as well as some other OS texts afaik.

Just because it's legal C doesn't mean it's allowed, it's perfectly legal
to do a lot of things in a usermode program that you can't do in a
kernel routine, smashing the kernel stack is one of these things. :)

-- 
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
"I have the heart of a child; I keep it in a jar on my desk."


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000403023858.F21029>