[CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Dawson Engler]

Hi All,

Here are 37 errors where variables >= 1024 bytes are allocated on a function's
stack.

	Summary for 
		2.4.4ac8-specific errors          = 9
		2.4.4-specific errors 		  = 0
		Common errors 		      	  = 28
		Total 				  = 37

Dawson

# BUGs	|	File Name
5	|	drivers/message/i2o/i2o_proc.c
4	|	drivers/cdrom/cdrom.c
3	|	drivers/block/../../lib/inflate.c
3	|	drivers/acpi/ospm/busmgr/bmpm.c
2	|	drivers/acpi/ospm/busmgr/bmdriver.c
2	|	net/irda/af_irda.c
2	|	fs/jffs2/compr_rtime.c
1	|	drivers/sound/emu10k1/audio.c
1	|	fs/jffs2/zlib.c
1	|	drivers/scsi/qlogicfc.c
1	|	arch/i386/kernel/nmi.c
1	|	drivers/net/wan/cycx_x25.c
1	|	drivers/media/video/w9966.c
1	|	drivers/block/cpqarray.c
1	|	fs/ntfs/super.c
1	|	fs/nfs/nfsroot.c
1	|	arch/i386/kernel/setup.c
1	|	drivers/net/zlib.c
1	|	fs/devfs/base.c
1	|	drivers/net/ewrk3.c
1	|	net/wanrouter/wanmain.c
1	|	net/bridge/br_ioctl.c
1	|	drivers/atm/iphase.c

############################################################
# 2.4.4ac8 specific errors
#
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/media/video/w9966.c:921:w9966_v4l_read: ERROR:VAR:921:921: suspicious var 'tbuf' = 2048 bytes:921 [nbytes=2048]
	}
	
	while(dleft > 0)
	{
		unsigned long tsize = (dleft > W9966_RBUFFER) ? W9966_RBUFFER : dleft;

Error --->
		unsigned char tbuf[W9966_RBUFFER];
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/acpi/ospm/busmgr/bmpm.c:57:bm_get_inferred_power_state: ERROR:VAR:57:57: suspicious var 'pr_list' = 1028 bytes:57 [nbytes=1028]
ACPI_STATUS
bm_get_inferred_power_state (
	BM_DEVICE               *device)
{
	ACPI_STATUS             status = AE_OK;

Error --->
	BM_HANDLE_LIST          pr_list;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/acpi/ospm/busmgr/bmpm.c:205:bm_set_power_state: ERROR:VAR:205:205: suspicious var 'target_list' = 1028 bytes:205 [nbytes=1028]
{
	ACPI_STATUS             status = AE_OK;
	BM_DEVICE		*device = NULL;
	BM_DEVICE		*parent_device = NULL;
	BM_HANDLE_LIST          current_list;

Error --->
	BM_HANDLE_LIST          target_list;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/acpi/ospm/busmgr/bmpm.c:204:bm_set_power_state: ERROR:VAR:204:204: suspicious var 'current_list' = 1028 bytes:204 [nbytes=1028]
	BM_POWER_STATE          state)
{
	ACPI_STATUS             status = AE_OK;
	BM_DEVICE		*device = NULL;
	BM_DEVICE		*parent_device = NULL;

Error --->
	BM_HANDLE_LIST          current_list;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/acpi/ospm/busmgr/bmdriver.c:323:bm_register_driver: ERROR:VAR:323:323: suspicious var 'device_list' = 1028 bytes:323 [nbytes=1028]
bm_register_driver (
	BM_DEVICE_ID		*criteria,
	BM_DRIVER		*driver)
{
	ACPI_STATUS             status = AE_NOT_FOUND;

Error --->
	BM_HANDLE_LIST          device_list;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/acpi/ospm/busmgr/bmdriver.c:407:bm_unregister_driver: ERROR:VAR:407:407: suspicious var 'device_list' = 1028 bytes:407 [nbytes=1028]
bm_unregister_driver (
	BM_DEVICE_ID		*criteria,
	BM_DRIVER		*driver)
{
	ACPI_STATUS             status = AE_NOT_FOUND;

Error --->
	BM_HANDLE_LIST          device_list;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/arch/i386/kernel/nmi.c:47:check_nmi_watchdog: ERROR:VAR:47:47: suspicious var 'tmp' = 1024 bytes:47 [nbytes=1024]
#define P6_EVENT_CPU_CLOCKS_NOT_HALTED	0x79
#define P6_NMI_EVENT		P6_EVENT_CPU_CLOCKS_NOT_HALTED

int __init check_nmi_watchdog (void)
{

Error --->
	irq_cpustat_t tmp[NR_CPUS];
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/fs/jffs2/compr_rtime.c:97:rtime_decompress: ERROR:VAR:97:97: suspicious var 'positions' = 1024 bytes:97 [nbytes=1024]


void rtime_decompress(unsigned char *data_in, unsigned char *cpage_out,
		      __u32 srclen, __u32 destlen)
{

Error --->
	int positions[256];
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/fs/jffs2/compr_rtime.c:57:rtime_compress: ERROR:VAR:57:57: suspicious var 'positions' = 1024 bytes:57 [nbytes=1024]

/* _compress returns the compressed size, -1 if bigger */
int rtime_compress(unsigned char *data_in, unsigned char *cpage_out, 
		   __u32 *sourcelen, __u32 *dstlen)
{

Error --->
	int positions[256];


############################################################
# errors common to both

#
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/sound/emu10k1/audio.c:906:emu10k1_audio_ioctl: ERROR:VAR:906:906: suspicious var 'buf' = 4016 bytes:906 [nbytes=4016]

		break;

	case SNDCTL_COPR_LOAD:
		{

Error --->
			copr_buffer buf;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/message/i2o/i2o_proc.c:955:i2o_proc_read_drivers_stored: ERROR:VAR:955:955: suspicious var 'result' = 3596 bytes:955 [nbytes=3596]
		u8  block_status;
		u8  error_info_size;
		u16 row_count;
		u16 more_flag;
		i2o_driver_store_table dst[MAX_I2O_MODULES];

Error --->
	} result;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/net/ewrk3.c:1639:ewrk3_ioctl: ERROR:VAR:1639:1639: suspicious var 'tmp' = 3072 bytes:1639 [nbytes=3072]
	int i, j, status = 0;
	u_char csr;
	union {
		u_char addr[HASH_TABLE_LEN * ETH_ALEN];
		u_short val[(HASH_TABLE_LEN * ETH_ALEN) >> 1];

Error --->
	} tmp;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/scsi/qlogicfc.c:867:isp2x00_make_portdb: ERROR:VAR:867:867: suspicious var 'temp' = 3072 bytes:867 [nbytes=3072]
static int isp2x00_make_portdb(struct Scsi_Host *host)
{

	short param[8];
	int i, j;

Error --->
	struct id_name_map temp[QLOGICFC_MAX_ID + 1];
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/message/i2o/i2o_proc.c:840:i2o_proc_read_ddm_table: ERROR:VAR:840:840: suspicious var 'result' = 2828 bytes:840 [nbytes=2828]
		u8  block_status;
		u8  error_info_size;
		u16 row_count;
		u16 more_flag;
		i2o_exec_execute_ddm_table ddm_table[MAX_I2O_MODULES];

Error --->
	} result;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/message/i2o/i2o_proc.c:1044:i2o_proc_read_groups: ERROR:VAR:1044:1044: suspicious var 'result' = 2060 bytes:1044 [nbytes=2060]
		u8  block_status;
		u8  error_info_size;
		u16 row_count;
		u16 more_flag;
		i2o_group_info group[256];

Error --->
	} result;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/message/i2o/i2o_proc.c:2261:i2o_proc_read_lan_mcast_addr: ERROR:VAR:2261:2261: suspicious var 'result' = 2060 bytes:2261 [nbytes=2060]
		u8  block_status;
		u8  error_info_size;
		u16 row_count;
		u16 more_flag;
		u8  mc_addr[256][8];

Error --->
	} result;	
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/message/i2o/i2o_proc.c:2492:i2o_proc_read_lan_alt_addr: ERROR:VAR:2492:2492: suspicious var 'result' = 2060 bytes:2492 [nbytes=2060]
		u8  block_status;
		u8  error_info_size;
		u16 row_count;
		u16 more_flag;
		u8  alt_addr[256][8];

Error --->
	} result;	
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/atm/iphase.c:2766:ia_ioctl: ERROR:VAR:2766:2766: suspicious var 'regs_local' = 2048 bytes:2766 [nbytes=2048]
             ia_cmds.status = 0;
             ia_cmds.len = 0x80;
             break;
          case MEMDUMP_FFL:
          {  

Error --->
             ia_regs_t       regs_local;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/fs/ntfs/super.c:352:ntfs_get_free_cluster_count: ERROR:VAR:352:352: suspicious var 'bits' = 2048 bytes:352 [nbytes=2048]

static int nc[16]={4,3,3,2,3,2,2,1,3,2,2,1,2,1,1,0};

int ntfs_get_free_cluster_count(ntfs_inode *bitmap)
{

Error --->
	unsigned char bits[2048];
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/arch/i386/kernel/setup.c:576:sanitize_e820_map: ERROR:VAR:576:576: function stack consumes 1840 bytes:576 [nbytes=1840]
			{
				if (overlap_list[i] == change_point[chgidx]->pbios)
					overlap_list[i] = overlap_list[overlap_entries-1];
			}
			overlap_entries--;

Error --->
		}
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/net/irda/af_irda.c:1981:irda_getsockopt: ERROR:VAR:1981:1981: suspicious var 'ias_opt' = 1356 bytes:1981 [nbytes=1356]
{
	struct sock *sk = sock->sk;
	struct irda_sock *self;
	struct irda_device_list list;
	struct irda_device_info *discoveries;

Error --->
	struct irda_ias_set	ias_opt;	/* IAS get/query params */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/net/irda/af_irda.c:1743:irda_setsockopt: ERROR:VAR:1743:1743: suspicious var 'ias_opt' = 1356 bytes:1743 [nbytes=1356]
static int irda_setsockopt(struct socket *sock, int level, int optname, 
			   char *optval, int optlen)
{
 	struct sock *sk = sock->sk;
	struct irda_sock *self;

Error --->
	struct irda_ias_set	ias_opt;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/block/cpqarray.c:1196:ida_ioctl: ERROR:VAR:1196:1196: suspicious var 'my_io' = 1296 bytes:1196 [nbytes=1296]
	int dsk  = MINOR(inode->i_rdev) >> NWD_SHIFT;
	int error;
	int diskinfo[4];
	struct hd_geometry *geo = (struct hd_geometry *)arg;
	ida_ioctl_t *io = (ida_ioctl_t*)arg;

Error --->
	ida_ioctl_t my_io;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/net/wanrouter/wanmain.c:765:device_new_if: ERROR:VAR:765:765: suspicious var 'conf' = 1272 bytes:765 [nbytes=1272]
 *	o register network interface
 */

static int device_new_if (wan_device_t *wandev, wanif_conf_t *u_conf)
{

Error --->
	wanif_conf_t conf;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/block/../../lib/inflate.c:750:inflate_dynamic: ERROR:VAR:750:750: suspicious var 'll' = 1264 bytes:750 [nbytes=1264]
  unsigned nl;          /* number of literal/length codes */
  unsigned nd;          /* number of distance codes */
#ifdef PKZIP_BUG_WORKAROUND
  unsigned ll[288+32];  /* literal/length and distance code lengths */
#else

Error --->
  unsigned ll[286+30];  /* literal/length and distance code lengths */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/net/zlib.c:4501:inflate_trees_fixed: ERROR:VAR:4501:4501: suspicious var 'c' = 1152 bytes:4501 [nbytes=1152]
{
  /* build fixed tables if not already (multiple overlapped executions ok) */
  if (!fixed_built)
  {
    int k;              /* temporary variable */

Error --->
    unsigned c[288];    /* length list for huft_build */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/block/../../lib/inflate.c:688:inflate_fixed: ERROR:VAR:688:688: suspicious var 'l' = 1152 bytes:688 [nbytes=1152]
  int i;                /* temporary variable */
  struct huft *tl;      /* literal/length code table */
  struct huft *td;      /* distance code table */
  int bl;               /* lookup bits for tl */
  int bd;               /* lookup bits for td */

Error --->
  unsigned l[288];      /* length list for huft_build */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/block/../../lib/inflate.c:301:huft_build: ERROR:VAR:301:301: suspicious var 'v' = 1152 bytes:301 [nbytes=1152]
  int l;                        /* bits per table (returned in m) */
  register unsigned *p;         /* pointer into c[], b[], or v[] */
  register struct huft *q;      /* points to current table */
  struct huft r;                /* table entry for structure assignment */
  struct huft *u[BMAX];         /* table stack */

Error --->
  unsigned v[N_MAX];            /* values in order of bit length */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/fs/jffs2/zlib.c:4216:huft_build: ERROR:VAR:4216:4216: suspicious var 'v' = 1152 bytes:4216 [nbytes=1152]
  int l;                        /* bits per table (returned in m) */
  register uIntf *p;            /* pointer into c[], b[], or v[] */
  inflate_huft *q;              /* points to current table */
  struct inflate_huft_s r;      /* table entry for structure assignment */
  inflate_huft *u[BMAX];        /* table stack */

Error --->
  uInt v[N_MAX];                /* values in order of bit length */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/fs/devfs/base.c:3154:devfsd_read: ERROR:VAR:3154:3154: suspicious var 'info' = 1056 bytes:3154 [nbytes=1056]
			    loff_t *ppos)
{
    int done = FALSE;
    int ival;
    loff_t pos, devname_offset, tlen, rpos;

Error --->
    struct devfsd_notify_struct info;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/cdrom/cdrom.c:815:cdrom_number_of_slots: ERROR:VAR:815:815: suspicious var 'info' = 1032 bytes:815 [nbytes=1032]
 */
int cdrom_number_of_slots(struct cdrom_device_info *cdi) 
{
	int status;
	int nslots = 1;

Error --->
	struct cdrom_changer_info info;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/cdrom/cdrom.c:857:cdrom_select_disc: ERROR:VAR:857:857: suspicious var 'info' = 1032 bytes:857 [nbytes=1032]
	return cdi->ops->generic_packet(cdi, &cgc);
}

int cdrom_select_disc(struct cdrom_device_info *cdi, int slot)
{

Error --->
	struct cdrom_changer_info info;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/cdrom/cdrom.c:1612:cdrom_ioctl: ERROR:VAR:1612:1612: suspicious var 'info' = 1032 bytes:1612 [nbytes=1032]
			cdi->options |= CDO_AUTO_CLOSE | CDO_AUTO_EJECT;
		return 0;
		}

	case CDROM_MEDIA_CHANGED: {

Error --->
		struct cdrom_changer_info info;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/cdrom/cdrom.c:791:cdrom_slot_status: ERROR:VAR:791:791: suspicious var 'info' = 1032 bytes:791 [nbytes=1032]
	return cdo->generic_packet(cdi, &cgc);
}

static int cdrom_slot_status(struct cdrom_device_info *cdi, int slot)
{

Error --->
	struct cdrom_changer_info info;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/fs/nfs/nfsroot.c:238:root_nfs_name: ERROR:VAR:238:238: suspicious var 'buf' = 1024 bytes:238 [nbytes=1024]
/*
 *  Prepare the NFS data structure and parse all options.
 */
static int __init root_nfs_name(char *name)
{

Error --->
	char buf[NFS_MAXPATHLEN];
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/drivers/net/wan/cycx_x25.c:983:hex_dump: ERROR:VAR:983:983: suspicious var 'hex' = 1024 bytes:983 [nbytes=1024]
			 card->devname, cmd->command);
}
#ifdef CYCLOMX_X25_DEBUG
static void hex_dump(char *msg, unsigned char *p, int len)
{

Error --->
	unsigned char hex[1024],
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.4-ac8/net/bridge/br_ioctl.c:86:br_ioctl_device: ERROR:VAR:86:86: suspicious var 'indices' = 1024 bytes:86 [nbytes=1024]
	}

	case BRCTL_GET_PORT_LIST:
	{
		int i;

Error --->
		int indices[256];

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Anton Altaparmakov]

At 22:10 24/05/2001, Dawson Engler wrote:
[snip]
>---------------------------------------------------------
>[BUG]
>/u2/engler/mc/oses/linux/2.4.4-ac8/fs/ntfs/super.c:352:ntfs_get_free_cluster_count: 
>ERROR:VAR:352:352: suspicious var 'bits' = 2048 bytes:352 [nbytes=2048]
>
>static int nc[16]={4,3,3,2,3,2,2,1,3,2,2,1,2,1,1,0};
>
>int ntfs_get_free_cluster_count(ntfs_inode *bitmap)
>{
>
>Error --->
>         unsigned char bits[2048];
>---------------------------------------------------------

Thanks. I was just about to submit a large patch anyway, so I will fix this 
and submit shortly.

Anton



-- 
   "Nothing succeeds like success." - Alexandre Dumas
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://sf.net/projects/linux-ntfs/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andreas Dilger]

Dawson Engler writes:
> Here are 37 errors where variables >= 1024 bytes are allocated on a
> function's stack.

First of all, thanks very much for the work you are doing.  It really
is useful, and a good way to catch those very rare error cases that
would not otherwise be fixed.

I'm curious about this stack checker.  Does it check for a single
stack allocation >= 1024 bytes, or does it also check for several
individual, smaller allocations which total >= 1024 bytes inside
a single function?  That would be equally useful.

On a side note, does anyone know if the kernel does checking if the
stack overflowed at any time?  It is hard to use Dawson's tools to
verify call paths because of interrupts and such, but I wonder what
happens when the kernel stack overflows - OOPS, or silent corruption?

Cheers, Andreas
-- 
Andreas Dilger  \ "If a man ate a pound of pasta and a pound of antipasto,
                 \  would they cancel out, leaving him still hungry?"
http://www-mddsp.enel.ucalgary.ca/People/adilger/               -- Dogbert

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andi Kleen]

On Thu, May 24, 2001 at 05:08:40PM -0600, Andreas Dilger wrote:
> I'm curious about this stack checker.  Does it check for a single
> stack allocation >= 1024 bytes, or does it also check for several
> individual, smaller allocations which total >= 1024 bytes inside
> a single function?  That would be equally useful.

At one time someone had a script to grep objdump -S vmlinux for the
stack allocations generated by gcc and check them. It found a few 
cases. It is easy to rewrite, as they are very regular instruction 
patterns at the beginning of functions (at least when you ignore variable 
length stack arrays, which do not seem to be common in the kernel anyways) 

> 
> On a side note, does anyone know if the kernel does checking if the
> stack overflowed at any time?  It is hard to use Dawson's tools to
> verify call paths because of interrupts and such, but I wonder what
> happens when the kernel stack overflows - OOPS, or silent corruption?

You normally get a silent hang or worse a stack fault exception 
(which linux/x86 without kdb cannot recover from) which gives you instant 
reboot.
The ikd patches contain a stack overflow checker for runtime.


-Andi

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Keith Owens]

On Fri, 25 May 2001 01:33:03 +0200, 
Andi Kleen <ak@suse.de> wrote:
>On Thu, May 24, 2001 at 05:08:40PM -0600, Andreas Dilger wrote:
>> I'm curious about this stack checker.  Does it check for a single
>> stack allocation >= 1024 bytes, or does it also check for several
>> individual, smaller allocations which total >= 1024 bytes inside
>> a single function?  That would be equally useful.
>
>At one time someone had a script to grep objdump -S vmlinux for the
>stack allocations generated by gcc and check them.

ftp://ftp.ocs.com.au/pub/kernel.stack.gz.  ix86 specific, probably gcc
specific and it only picks up code that you compile.  The Stanford
checker is much better.

>> On a side note, does anyone know if the kernel does checking if the
>> stack overflowed at any time?
>
>You normally get a silent hang or worse a stack fault exception 
>(which linux/x86 without kdb cannot recover from) which gives you instant 
>reboot.

You cannot recover from a kernel stack overflow even with kdb.  The
exception handler and kdb use the stack that just overflowed.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andreas Dilger]

Keith Owens writes:
> Andi Kleen <ak@suse.de> wrote:
> >At one time someone had a script to grep objdump -S vmlinux for the
> >stack allocations generated by gcc and check them.
> 
> ftp://ftp.ocs.com.au/pub/kernel.stack.gz.  ix86 specific, probably gcc
> specific and it only picks up code that you compile.  The Stanford
> checker is much better.

I would probably agree.  Dawson said in a separate email that it would
be possible to enhance the checker to follow the call chains to measure
total stack usage.  Combining this with potential interrupt stack usage,
we may be able to eliminate some rare problems otherwise hard to find.

> >> On a side note, does anyone know if the kernel does checking if the
> >> stack overflowed at any time?
> >
> >You normally get a silent hang or worse a stack fault exception 
> >(which linux/x86 without kdb cannot recover from) which gives you instant 
> >reboot.
> 
> You cannot recover from a kernel stack overflow even with kdb.  The
> exception handler and kdb use the stack that just overflowed.

If it at least tells you that the stack has overflowed, and a backtrace
of the stack up to that point, that would at least be useful for fixing
the functions which caused the problem.

Also, allowing a config option to zero the stack at allocation would at
least allow the SysRQ-T code to tell you how much of the stack has
previously been in use so we can get an idea if we are close or not.

Given that the Stanford checker has discovered several individual 3kB+
stack allocations, it would surprise me if we didn't have stack overflows.
It may well be that people get mysterious hangs in these cases and have
no way to even diagnose the problem.  Maybe they are blaming the rare
hangs on hardware instead of software.

Cheers, Andreas
-- 
Andreas Dilger  \ "If a man ate a pound of pasta and a pound of antipasto,
                 \  would they cancel out, leaving him still hungry?"
http://www-mddsp.enel.ucalgary.ca/People/adilger/               -- Dogbert

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Keith Owens]

On Fri, 25 May 2001 00:33:56 -0600 (MDT), 
Andreas Dilger <adilger@turbolinux.com> wrote:
>Keith Owens writes:
>> You cannot recover from a kernel stack overflow even with kdb.  The
>> exception handler and kdb use the stack that just overflowed.
>
>If it at least tells you that the stack has overflowed, and a backtrace
>of the stack up to that point, that would at least be useful for fixing
>the functions which caused the problem.

A small overflow of the kernel stack overwrites the struct task at the
bottom of the stack, recovery is dubious at best because we rely on
data in struct task.  A large overflow of the kernel stack either
corrupts the storage below this task's stack, which could hit anything,
or it gets a stack fault.

If we take a stack fault on ix86, stack_segment() is invoked.  Just
taking the fault and calling the routine uses the kernel stack which
has already overflowed, causing a double fault.  The double fault
handler uses the kernel stack, generating a triple fault.  The machine
is now dead.

The only way to avoid those problems is to move struct task out of the
kernel stack pages and to use a task gate for the stack fault and
double fault handlers, instead of a trap gate (all ix86 specific).
Those methods are expensive, at a minimum they require an extra page
for every process plus an extra stack per cpu.  I have not even
considered the extra cost of using task gates for the interrupts nor
how this method would complicate methods for getting the current struct
task pointer.  It is not worth the bother, we write better kernel code
than that.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andi Kleen]

On Fri, May 25, 2001 at 04:53:47PM +1000, Keith Owens wrote:
> The only way to avoid those problems is to move struct task out of the
> kernel stack pages and to use a task gate for the stack fault and
> double fault handlers, instead of a trap gate (all ix86 specific).
> Those methods are expensive, at a minimum they require an extra page
> for every process plus an extra stack per cpu.  I have not even
> considered the extra cost of using task gates for the interrupts nor
> how this method would complicate methods for getting the current struct
> task pointer.  It is not worth the bother, we write better kernel code
> than that.

When you don't try to handle recursive stack/double faults it only requires
a single static stack per CPU. With some tricks and minor races it is also
possible to handle multiple ones.


-Andi

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Keith Owens]

On Fri, 25 May 2001 10:20:15 +0200, 
Andi Kleen <ak@suse.de> wrote:
>On Fri, May 25, 2001 at 04:53:47PM +1000, Keith Owens wrote:
>> The only way to avoid those problems is to move struct task out of the
>> kernel stack pages and to use a task gate for the stack fault and
>> double fault handlers, instead of a trap gate (all ix86 specific).
>> Those methods are expensive, at a minimum they require an extra page
>> for every process plus an extra stack per cpu.  I have not even
>> considered the extra cost of using task gates for the interrupts nor
>> how this method would complicate methods for getting the current struct
>> task pointer.  It is not worth the bother, we write better kernel code
>> than that.
>
>When you don't try to handle recursive stack/double faults it only requires
>a single static stack per CPU. With some tricks and minor races it is also
>possible to handle multiple ones.

That is exactly what I said above, a separate fault task with its own
stack for every cpu.  But there is no point in doing this to detect a
hardware stack overflow when the overflow has already corrupted the
struct task which is at the bottom of the stack segment.

To get any benefit from hardware detection of kernel stack overflow you
must also dedicate the stack segment to hold only stack data.  That
means moving struct task to yet another page, adding an extra page per
task.  It is just too expensive, we write better code than that.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andi Kleen]

On Fri, May 25, 2001 at 06:31:20PM +1000, Keith Owens wrote:
> That is exactly what I said above, a separate fault task with its own
> stack for every cpu.  But there is no point in doing this to detect a
> hardware stack overflow when the overflow has already corrupted the
> struct task which is at the bottom of the stack segment.

You can at least get a backtrace, which is useful enough.

> 
> To get any benefit from hardware detection of kernel stack overflow you
> must also dedicate the stack segment to hold only stack data.  That
> means moving struct task to yet another page, adding an extra page per
> task.  It is just too expensive, we write better code than that.

Ah this was a misunderstanding. I was just talking about plain recovery
from stack faults; not from hardware stack overflow detection. Even without
wather tight overflow detection they happen often enough and usually not
too long after a stack page overflow.




-Andi

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Oliver Neukum]

> A small overflow of the kernel stack overwrites the struct task at the
> bottom of the stack, recovery is dubious at best because we rely on
> data in struct task.  A large overflow of the kernel stack either
> corrupts the storage below this task's stack, which could hit anything,
> or it gets a stack fault.

Is there a reason for the task structure to be at the bottom rather than the 
top of these two pages ?

	Regards
		Oliver

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andi Kleen]

On Fri, May 25, 2001 at 04:03:57PM +0200, Oliver Neukum wrote:
> > A small overflow of the kernel stack overwrites the struct task at the
> > bottom of the stack, recovery is dubious at best because we rely on
> > data in struct task.  A large overflow of the kernel stack either
> > corrupts the storage below this task's stack, which could hit anything,
> > or it gets a stack fault.
> 
> Is there a reason for the task structure to be at the bottom rather than the 
> top of these two pages ?

This way you save one addition for every current access; which adds to 
quite a few KB over the complete kernel. 

-Andi

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[dean gaudet]

On Fri, 25 May 2001, Andi Kleen wrote:

> On Fri, May 25, 2001 at 04:03:57PM +0200, Oliver Neukum wrote:
> > Is there a reason for the task structure to be at the bottom rather than the
> > top of these two pages ?
>
> This way you save one addition for every current access; which adds to
> quite a few KB over the complete kernel.

hrm, really?

i think it really depends on how you use current -- here's an alternative
usage which can fold the extra addition into the structure offset
calculations, and moves the task struct to the top of the stack.

not that this really solves anything, 'cause a stack underflow will just
trash something else rather than the task struct :)

-dean

% cat task.c
struct task {
        int a;
        int b;
};

#define current(p)      (((struct task *)(((unsigned)p | 0x1fff)+1))-1)
int foo(void *p)
{
        return current(p)->a + current(p)->b;
}
% gcc -O -c task.c
% objdump -dr task.o

task.o:     file format elf32-i386

Disassembly of section .text:

00000000 <foo>:
   0:   55                      push   %ebp
   1:   89 e5                   mov    %esp,%ebp
   3:   8b 55 08                mov    0x8(%ebp),%edx
   6:   81 ca ff 1f 00 00       or     $0x1fff,%edx
   c:   8b 42 fd                mov    0xfffffffd(%edx),%eax
   f:   03 42 f9                add    0xfffffff9(%edx),%eax
  12:   c9                      leave
  13:   c3                      ret

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Jonathan Lundell]

At 8:45 AM -0700 2001-05-25, dean gaudet wrote:
>i think it really depends on how you use current -- here's an alternative
>usage which can fold the extra addition into the structure offset
>calculations, and moves the task struct to the top of the stack.
>
>not that this really solves anything, 'cause a stack underflow will just
>trash something else rather than the task struct :)

It would open the door for putting a guard page (which only occupies 
virtual space, after all) below the stack. I have no idea whether 
that's practical, given other constraints, but it's a potential 
benefit of having the stack at the bottom rather than the top of a 
page.
-- 
/Jonathan Lundell.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[dean gaudet]

On Fri, 25 May 2001, Jonathan Lundell wrote:

> At 8:45 AM -0700 2001-05-25, dean gaudet wrote:
> >i think it really depends on how you use current -- here's an alternative
> >usage which can fold the extra addition into the structure offset
> >calculations, and moves the task struct to the top of the stack.
> >
> >not that this really solves anything, 'cause a stack underflow will just
> >trash something else rather than the task struct :)
>
> It would open the door for putting a guard page (which only occupies
> virtual space, after all) below the stack. I have no idea whether
> that's practical, given other constraints, but it's a potential
> benefit of having the stack at the bottom rather than the top of a
> page.

somewhere else in the thread someone indicated this was a hard thing to
do.

also you don't need the task struct at the top to do this -- you just
allocate 16k instead of 8k, put the task struct on page 0 of the
allocation, unmap page 1, and put the stack frame on pages 2 and 3.
(you'd probably have to do a 16k allocation regardless to get the guard
page.)

-dean

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Jeff Dike]

dean-list-linux-kernel@arctic.org said:
> also you don't need the task struct at the top to do this -- you just
> allocate 16k instead of 8k, put the task struct on page 0 of the
> allocation, unmap page 1, and put the stack frame on pages 2 and 3.
> (you'd probably have to do a 16k allocation regardless to get the
> guard page.)

This is exactly what UML does.  My stacks are larger than the host's stacks 
and I was having stack overflow problems when the Linux started saving SSE 
state on signal frames.

So I doubled the stack size and put in a guard page while I was at it.

				Jeff

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[David Welch]

On Fri, May 25, 2001 at 03:20:20PM +1000, Keith Owens wrote:
> 
> >> On a side note, does anyone know if the kernel does checking if the
> >> stack overflowed at any time?
> >
> >You normally get a silent hang or worse a stack fault exception 
> >(which linux/x86 without kdb cannot recover from) which gives you instant 
> >reboot.
> 
> You cannot recover from a kernel stack overflow even with kdb.  The
> exception handler and kdb use the stack that just overflowed.
> 
Why not use a task gate for the double fault handler points to a 
per-processor TSS with a seperate stack. This would allow limited recovery
from a kernel stack overlay.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Keith Owens]

On Fri, 25 May 2001 08:11:07 +0100, 
David Welch <david.welch@st-edmund-hall.oxford.ac.uk> wrote:
>Why not use a task gate for the double fault handler points to a 
>per-processor TSS with a seperate stack. This would allow limited recovery
>from a kernel stack overlay.

It is far too late by then.  struct task is at the bottom of the kernel
stack, a stack overflow would corrupt the task data long before the
hardware was involved.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[dean gaudet]

another possibility for a debugging mode for the kernel would be to hack
gcc to emit something like the following in the prologue of every function
(after the frame is allocated):

	movl %esp,%edx
	andl %edx,0x1fff
	cmpl %edx,sizeof(struct task)+512
	jbe stack_overflow

where stack_overflow is a no_return routine... the 512 is just some fudge
factor where if we get that low on the stack we probably want to know
about it (perhaps compile time tuneable).

-dean

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Keith Owens]

On Fri, 25 May 2001 08:31:24 -0700 (PDT), 
dean gaudet <dean-list-linux-kernel@arctic.org> wrote:
>another possibility for a debugging mode for the kernel would be to hack
>gcc to emit something like the following in the prologue of every function
>(after the frame is allocated):

IKD already does that, via the CONFIG_DEBUG_KSTACK, CONFIG_KSTACK_METER
and CONFIG_KSTACK_THRESHOLD options.  No need to hack gcc.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[dean gaudet]

On Sat, 26 May 2001, Keith Owens wrote:

> On Fri, 25 May 2001 08:31:24 -0700 (PDT),
> dean gaudet <dean-list-linux-kernel@arctic.org> wrote:
> >another possibility for a debugging mode for the kernel would be to hack
> >gcc to emit something like the following in the prologue of every function
> >(after the frame is allocated):
>
> IKD already does that, via the CONFIG_DEBUG_KSTACK, CONFIG_KSTACK_METER
> and CONFIG_KSTACK_THRESHOLD options.  No need to hack gcc.

oh nice!  you hook in through gcc -pg mcount stuff.

-dean

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andi Kleen]

On Fri, May 25, 2001 at 03:20:20PM +1000, Keith Owens wrote:
> >> On a side note, does anyone know if the kernel does checking if the
> >> stack overflowed at any time?
> >
> >You normally get a silent hang or worse a stack fault exception 
> >(which linux/x86 without kdb cannot recover from) which gives you instant 
> >reboot.
> 
> You cannot recover from a kernel stack overflow even with kdb.  The
> exception handler and kdb use the stack that just overflowed.

Hmm, I thought it used an own stack using an appropiate gate.
At least on x86-64 I implemented it this way using a static 4K array.


-Andi

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Keith Owens]

On Fri, 25 May 2001 10:14:57 +0200, 
Andi Kleen <ak@suse.de> wrote:
>On Fri, May 25, 2001 at 03:20:20PM +1000, Keith Owens wrote:
>> You cannot recover from a kernel stack overflow even with kdb.  The
>> exception handler and kdb use the stack that just overflowed.
>
>Hmm, I thought it used an own stack using an appropiate gate.
>At least on x86-64 I implemented it this way using a static 4K array.

Nothing in arch/i386/kernel/traps.c uses a task gate, they are all
interrupt, trap, system or call gates.  I guarantee that kdb on ix86
and ia64 uses the same kernel stack as the failing task, the starting
point for the kdb backtrace is itself and it does not follow segment
switches.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andi Kleen]

On Fri, May 25, 2001 at 06:25:57PM +1000, Keith Owens wrote:
> Nothing in arch/i386/kernel/traps.c uses a task gate, they are all
> interrupt, trap, system or call gates.  I guarantee that kdb on ix86
> and ia64 uses the same kernel stack as the failing task, the starting
> point for the kdb backtrace is itself and it does not follow segment
> switches.

I would consider this a bug in kdb then.

-Andi

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Keith Owens]

On Fri, 25 May 2001 10:27:53 +0200, 
Andi Kleen <ak@suse.de> wrote:
>On Fri, May 25, 2001 at 06:25:57PM +1000, Keith Owens wrote:
>> Nothing in arch/i386/kernel/traps.c uses a task gate, they are all
>> interrupt, trap, system or call gates.  I guarantee that kdb on ix86
>> and ia64 uses the same kernel stack as the failing task, the starting
>> point for the kdb backtrace is itself and it does not follow segment
>> switches.
>
>I would consider this a bug in kdb then.

No more of a bug than panic(), show_stack(), printk() and all the other
routines that get called during a kernel problem.  They all use the
current kernel stack and they work almost all the time.  Kernel stack
overflows are symptoms of bad code, so fix the code, not the recovery
routines.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andi Kleen]

On Fri, May 25, 2001 at 03:20:20PM +1000, Keith Owens wrote:
> ftp://ftp.ocs.com.au/pub/kernel.stack.gz.  ix86 specific, probably gcc
> specific and it only picks up code that you compile.  The Stanford
> checker is much better.

I have no complete understanding of the stanford checker, but I was assuming
that it was a hacked version of gcc. If yes it has to use the preprocessor
and also will only catch stuff you compile. I don't see how you can do 
meaningfull lint like checks without a preprocessor ;)

-Andi

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Brian Gerst]

Andi Kleen wrote:
> 
> On Thu, May 24, 2001 at 05:08:40PM -0600, Andreas Dilger wrote:
> > I'm curious about this stack checker.  Does it check for a single
> > stack allocation >= 1024 bytes, or does it also check for several
> > individual, smaller allocations which total >= 1024 bytes inside
> > a single function?  That would be equally useful.
> 
> At one time someone had a script to grep objdump -S vmlinux for the
> stack allocations generated by gcc and check them. It found a few
> cases. It is easy to rewrite, as they are very regular instruction
> patterns at the beginning of functions (at least when you ignore variable
> length stack arrays, which do not seem to be common in the kernel anyways)
> 
> >
> > On a side note, does anyone know if the kernel does checking if the
> > stack overflowed at any time?  It is hard to use Dawson's tools to
> > verify call paths because of interrupts and such, but I wonder what
> > happens when the kernel stack overflows - OOPS, or silent corruption?
> 
> You normally get a silent hang or worse a stack fault exception
> (which linux/x86 without kdb cannot recover from) which gives you instant
> reboot.
> The ikd patches contain a stack overflow checker for runtime.

Actually, you will never get a stack fault exception, since with a flat
stack segment you can never get a limit violation.  All you will do is
corrupt the data in task struct and cause an oops later on when the
kernel tries to use the task struct.  There are only two ways to
properly trap a kernel stack overflow:

- Make the stack segment non-flat, putting the limit just above the task
struct.  Ugly, because we want to stay away from segmentation.  The
stack fault handler would have to be a task gate.  This also causes
problems because pointers accessed through %ebp also use the stack
segment by default.  We would either need to leave frame pointers turned
on or teach GCC to use %ds overrides when using %ebp as a pointer.

- Add a not-present guard page at the bottom of the stack.  This means
the stack would have to live in vmalloc'ed memory, which I don't think
the kernel can handle at this time (with lazy vmalloc mapping).  The
task struct would have to be moved elsewhere or it would still get
overwritten.  Then a double fault task would be able to detect this and
kill the task.

In other words, with the current x86 architecture, there isn't really
much we can do to handle stack overflows without sacrificing
performance.  Good discipline is the best we have.

-- 

						Brian Gerst

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andi Kleen]

On Fri, May 25, 2001 at 07:52:02AM -0400, Brian Gerst wrote:
> Actually, you will never get a stack fault exception, since with a flat
> stack segment you can never get a limit violation.  All you will do is
> corrupt the data in task struct and cause an oops later on when the
> kernel tries to use the task struct.  There are only two ways to
> properly trap a kernel stack overflow:

In my experience the stack pointer eventually gets corrupted and starts 
pointing to some unmapped area, which gives you a stack fault (admittedly
a backtrace is a bit hard after that)

-Andi

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Brian Gerst]

Andi Kleen wrote:
> 
> On Fri, May 25, 2001 at 07:52:02AM -0400, Brian Gerst wrote:
> > Actually, you will never get a stack fault exception, since with a flat
> > stack segment you can never get a limit violation.  All you will do is
> > corrupt the data in task struct and cause an oops later on when the
> > kernel tries to use the task struct.  There are only two ways to
> > properly trap a kernel stack overflow:
> 
> In my experience the stack pointer eventually gets corrupted and starts
> pointing to some unmapped area, which gives you a stack fault (admittedly
> a backtrace is a bit hard after that)

You mean a double fault.  #SS is only called when there is a limit
violation on the stack segment or the stack segment is not present.  I
guess you could get a limit violation if it tried to wrap around below
0, but you would get a page fault first.  An unmapped page will always
cause a page fault, but since the stack is still invalid, it would then
cause a double fault.

-- 

						Brian Gerst

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Andrew Morton]

Andreas Dilger wrote:
> 
> Dawson Engler writes:
> > Here are 37 errors where variables >= 1024 bytes are allocated on a
> > function's stack.
> 
> First of all, thanks very much for the work you are doing.  It really
> is useful, and a good way to catch those very rare error cases that
> would not otherwise be fixed.
> 
> I'm curious about this stack checker.  Does it check for a single
> stack allocation >= 1024 bytes, or does it also check for several
> individual, smaller allocations which total >= 1024 bytes inside
> a single function?  That would be equally useful.
> 
> On a side note, does anyone know if the kernel does checking if the
> stack overflowed at any time?

There's a little bit of code in show_task() which calculates
how close this task ever got to overrunning its kernel stack:

        {
                unsigned long * n = (unsigned long *) (p+1);
                while (!*n)
                        n++;
                free = (unsigned long) n - (unsigned long)(p+1);
        }
        printk("%5lu %5d %6d ", free, p->pid, p->p_pptr->pid);

SYSRQ-T will trigger this.

However it doesn't work, because do_fork() doesn't zero
out the stack pages when they're created.

-

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Mikael Pettersson]

On Thu, 24 May 2001 14:10:00 -0700 (PDT), Dawson Engler wrote:

>[BUG]
>/u2/engler/mc/oses/linux/2.4.4-ac8/arch/i386/kernel/nmi.c:47:check_nmi_watchdog: ERROR:VAR:47:47: suspicious var 'tmp' = 1024 bytes:47 [nbytes=1024]
>#define P6_EVENT_CPU_CLOCKS_NOT_HALTED	0x79
>#define P6_NMI_EVENT		P6_EVENT_CPU_CLOCKS_NOT_HALTED
>
>int __init check_nmi_watchdog (void)
>{
>
>Error --->
>	irq_cpustat_t tmp[NR_CPUS];

Nice work, but I think this is a false positive.

check_nmi_watchdog() is __init and we know exactly when it's called.
The interesting cases (SMP kernel, since for UP NR_CPUS==1) are:

start_kernel -> smp_init -> smp_boot_cpus -> APIC_init_uniprocessor
-> check_nmi_watchdog

and

start_kernel -> smp_init -> smp_boot_cpus -> setup_IO_APIC ->
check_timer -> check_nmi_watchdog

In neither case is the stack large enough that the 1KB blob in
check_nmi_watchdog should pose a problem, I think. (Don't we have
something like 8KB-sizeof(struct task_struct) stack to play with?)

IMHO the checker tool should take call paths into consideration
when trying to detect stack overflow problems. Does it do that?
(I.e. is it polyvariant or monovariant?)

I could write a patch to make 'tmp' __initdata instead, which would
silence the checker tool, but I don't really want to do that unless
someone can convince me that there is a real problem here.

/Mikael

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Dawson Engler]

> check_nmi_watchdog() is __init and we know exactly when it's called.
> The interesting cases (SMP kernel, since for UP NR_CPUS==1) are:

Ah, nice --- I keep meaning to tell the checker to demote its warning
about NULL bugs or large stack vars in __init routines and/or routines
that have the substring "init" in them ;-)


> IMHO the checker tool should take call paths into consideration
> when trying to detect stack overflow problems. Does it do that?
> (I.e. is it polyvariant or monovariant?)

The var checker is more "really stupid".  It just does a flow
insensitive pass looking for big variables.  I could make it follow
call chains without too much work (other checkers do do this.)

> I could write a patch to make 'tmp' __initdata instead, which would
> silence the checker tool, but I don't really want to do that unless
> someone can convince me that there is a real problem here.

No need.  Once it's marked as an FP the checker won't warn about it
anymore.

Thanks for post-mortem.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Alexander Viro]

On Thu, 24 May 2001, Dawson Engler wrote:

> > check_nmi_watchdog() is __init and we know exactly when it's called.
> > The interesting cases (SMP kernel, since for UP NR_CPUS==1) are:
> 
> Ah, nice --- I keep meaning to tell the checker to demote its warning
> about NULL bugs or large stack vars in __init routines and/or routines
> that have the substring "init" in them ;-)

Please, don't. These functions are often used from/as init_module(),
so they must handle the case when allocation fails. They can be
called long after the boot.

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Dawson Engler]

> > Ah, nice --- I keep meaning to tell the checker to demote its warning
> > about NULL bugs or large stack vars in __init routines and/or routines
> > that have the substring "init" in them ;-)
> 
> Please, don't. These functions are often used from/as init_module(),
> so they must handle the case when allocation fails. They can be
> called long after the boot.

I meant "demote"  to mean "reducing the ranking of these errors during
sorting" rather than "eliminate from the error logs".  

RE: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[Dunlap, Randy]

> From: Andrew Morton [mailto:andrewm@uow.edu.au]
> 
> Andreas Dilger wrote:
> > 
> > On a side note, does anyone know if the kernel does checking if the
> > stack overflowed at any time?
> 
> There's a little bit of code in show_task() which calculates
> how close this task ever got to overrunning its kernel stack:
> 
>         {
>                 unsigned long * n = (unsigned long *) (p+1);
>                 while (!*n)
>                         n++;
>                 free = (unsigned long) n - (unsigned long)(p+1);
>         }
>         printk("%5lu %5d %6d ", free, p->pid, p->p_pptr->pid);
> 
> SYSRQ-T will trigger this.
> 
> However it doesn't work, because do_fork() doesn't zero
> out the stack pages when they're created.

If do_fork() performance is an issue, at least clearing the stack
pages as a build option would be nice for some of us.

~Randy

Re: [CHECKER] large stack variables (>=1K) in 2.4.4 and 2.4.4-ac8

[VDA]

On Fri, May 25, 2001 at 07:52:02AM -0400, Brian Gerst wrote:
> Actually, you will never get a stack fault exception, since with a flat
> stack segment you can never get a limit violation. All you will do is
> corrupt the data in task struct and cause an oops later on when the
> kernel tries to use the task struct. There are only two ways to
> properly trap a kernel stack overflow:

> - Make the stack segment non-flat, putting the limit just above the task
> struct. Ugly, because we want to stay away from segmentation. The
> stack fault handler would have to be a task gate. This also causes
> problems because pointers accessed through %ebp also use the stack
> segment by default. We would either need to leave frame pointers turned
> on or teach GCC to use %ds overrides when using %ebp as a pointer.

> - Add a not-present guard page at the bottom of the stack. This means
> the stack would have to live in vmalloc'ed memory, which I don't think
> the kernel can handle at this time (with lazy vmalloc mapping). The
> task struct would have to be moved elsewhere or it would still get
> overwritten. Then a double fault task would be able to detect this and
> kill the task.

You're talking about something like this?

0.............|taskstruct|guardpage(s)|stack space............FFFFFFFF
^^^^^^^^^^^^              ^^^^^^^^^^^^
This is not               these cause
a stack space             a page fault
                          -> double fault
                          thru task gate

I don't see why task struct have to be moved elsewhere. A couple of
(not-present) pages and some support from MM subsystem is needed as
well as double fault handling code.

> In other words, with the current x86 architecture, there isn't really
> much we can do to handle stack overflows without sacrificing
> performance. Good discipline is the best we have.

It could be useful as a debug feature for testing experimental kernels
(selectable by make config).

Pls CC me. I'm not on the list
--
vda@port.imtp.ilyichevsk.odessa.ua