[CHECKER] 37 stack variables >= 1K in 2.4.17

[CHECKER] 37 stack variables >= 1K in 2.4.17

[Dawson Engler]

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

Dawson

# BUGs	|	File Name
4	|	/drivers/cdrom.c
4	|	/message/i2o_proc.c
3	|	/net/airo.c
3	|	/../inflate.c
2	|	/fs/zlib.c
2	|	/drivers/zlib.c
2	|	/drivers/cpqfcTScontrol.c
2	|	/fs/compr_rtime.c
1	|	/char/sidewinder.c
1	|	/drivers/ide-cs.c
1	|	/drivers/iphase.c
1	|	/drivers/ixj_pcmcia.c
1	|	/i386/nmi.c
1	|	/drivers/parport_cs.c
1	|	/net/br_ioctl.c
1	|	/drivers/qlogicfc.c
1	|	/drivers/cpqarray.c
1	|	/drivers/optcd.c
1	|	/net/wanmain.c
1	|	/i386/setup.c
1	|	/fs/nfsroot.c
1	|	/net/cycx_x25.c
1	|	/fs/ioctl.c


############################################################
# 2.4.17 specific errors

#
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/scsi/qlogicfc.c:840:isp2x00_make_portdb: ERROR:VAR:840:840: suspicious var 'temp' = 3072 bytes:840 [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];
	struct isp2x00_hostdata *hostdata;

	isp2x00_disable_irqs(host);
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/message/i2o/i2o_proc.c:838:i2o_proc_read_ddm_table: ERROR:VAR:838:838: suspicious var 'result' = 2828 bytes:838 [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;

	i2o_exec_execute_ddm_table ddm_table;

---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/cdrom/optcd.c:1625:cdromread: ERROR:VAR:1625:1625: suspicious var 'buf' = 2646 bytes:1625 [nbytes=2646]

static int cdromread(unsigned long arg, int blocksize, int cmd)
{
	int status;
	struct cdrom_msf msf;

Error --->
	char buf[CD_FRAMESIZE_RAWER];

	status = verify_area(VERIFY_WRITE, (void *) arg, blocksize);
	if (status)
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/message/i2o/i2o_proc.c:2266:i2o_proc_read_lan_mcast_addr: ERROR:VAR:2266:2266: suspicious var 'result' = 2060 bytes:2266 [nbytes=2060]
		u8  block_status;
		u8  error_info_size;
		u16 row_count;
		u16 more_flag;
		u8  mc_addr[256][8];

Error --->
	} result;	

	spin_lock(&i2o_proc_lock);	
	len = 0;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/message/i2o/i2o_proc.c:2497:i2o_proc_read_lan_alt_addr: ERROR:VAR:2497:2497: suspicious var 'result' = 2060 bytes:2497 [nbytes=2060]
		u8  block_status;
		u8  error_info_size;
		u16 row_count;
		u16 more_flag;
		u8  alt_addr[256][8];

Error --->
	} result;	

	spin_lock(&i2o_proc_lock);	
	len = 0;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/message/i2o/i2o_proc.c:1049:i2o_proc_read_groups: ERROR:VAR:1049:1049: suspicious var 'result' = 2060 bytes:1049 [nbytes=2060]
		u8  block_status;
		u8  error_info_size;
		u16 row_count;
		u16 more_flag;
		i2o_group_info group[256];

Error --->
	} result;

	spin_lock(&i2o_proc_lock);

---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/scsi/cpqfcTScontrol.c:721:CpqTsProcessIMQEntry: ERROR:VAR:721:721: suspicious var 'ulFibreFrame' = 2048 bytes:721 [nbytes=2048]
  int iStatus;
  USHORT i, RPCset, DPCset;
  ULONG x_ID;
  ULONG ulBuff, dwStatus;
  TachFCHDR_GCMND* fchs;

Error --->
  ULONG ulFibreFrame[2048/4];  // max number of DWORDS in incoming Fibre Frame
  UCHAR ucInboundMessageType;  // Inbound CM, dword 3 "type" field

  ENTER("ProcessIMQEntry");
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/atm/iphase.c:2772:ia_ioctl: ERROR:VAR:2772:2772: suspicious var 'regs_local' = 2048 bytes:2772 [nbytes=2048]
             ia_cmds.status = 0;
             ia_cmds.len = 0x80;
             break;
          case MEMDUMP_FFL:
          {  

Error --->
             ia_regs_t       regs_local;
             ffredn_t        *ffL = &regs_local.ffredn;
             rfredn_t        *rfL = &regs_local.rfredn;
                     
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/net/wireless/airo.c:4316:readrids: ERROR:VAR:4316:4316: suspicious var 'iobuf' = 2048 bytes:4316 [nbytes=2048]
 * as needed.  This represents the READ side of control I/O to
 * the card
 */
static int readrids(struct net_device *dev, aironet_ioctl *comp) {
	unsigned short ridcode;

Error --->
	unsigned char iobuf[2048];

	switch(comp->command)
	{
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/net/wireless/airo.c:4364:writerids: ERROR:VAR:4364:4364: suspicious var 'iobuf' = 2048 bytes:4364 [nbytes=2048]

static int writerids(struct net_device *dev, aironet_ioctl *comp) {
	int  ridcode;
	Resp      rsp;
	static int (* writer)(struct airo_info *, u16 rid, const void *, int);

Error --->
	unsigned char iobuf[2048];

	/* Only super-user can write RIDs */
	if (!capable(CAP_NET_ADMIN))
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/scsi/cpqfcTScontrol.c:610:PeekIMQEntry: ERROR:VAR:610:610: suspicious var 'ulFibreFrame' = 2048 bytes:610 [nbytes=2048]
      // If we find it, check the incoming frame payload (1st word)
      // for LILP frame
        if( (fcChip->IMQ->QEntry[CI].type & 0x1FF) == 0x104 )
        { 
          TachFCHDR_GCMND* fchs;

Error --->
          ULONG ulFibreFrame[2048/4];  // max DWORDS in incoming FC Frame
	  USHORT SFQpi = (USHORT)(fcChip->IMQ->QEntry[CI].word[0] & 0x0fffL);

	  CpqTsGetSFQEntry( fcChip,
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/arch/i386/kernel/setup.c:519:sanitize_e820_map: ERROR:VAR:519:519: function stack consumes 1840 bytes:519 [nbytes=1840]
		   ______________________4_
	*/

	/* if there's only one memory region, don't bother */
	if (*pnr_map < 2)

Error --->
		return -1;

	old_nr = *pnr_map;

---------------------------------------------------------
[BUG] *think* so
/u2/engler/mc/oses/linux/2.4.17/fs/umsdos/ioctl.c:96:UMSDOS_ioctl_dir: ERROR:VAR:96:96: function stack consumes 1772 bytes:96 [nbytes=1772]
	    && cmd != UMSDOS_UNLINK_EMD
	    && cmd != UMSDOS_UNLINK_DOS
	    && cmd != UMSDOS_RMDIR_DOS
	    && cmd != UMSDOS_STAT_DOS
	    && cmd != UMSDOS_DOS_SETUP)

Error --->
		return fat_dir_ioctl (dir, filp, cmd, data_ptr);

	/* #Specification: ioctl / access
	 * Only root (effective id) is allowed to do IOCTL on directory
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/net/wireless/airo.c:3319:airo_ioctl: ERROR:VAR:3319:3319: function stack consumes 1676 bytes:3319 [nbytes=1676]
#endif /* CISCO_EXT */
	{
		/* If the command read some stuff, we better get it out of
		 * the card first... */
		if(IW_IS_GET(cmd))

Error --->
			readStatusRid(local, &status_rid);
		if(IW_IS_GET(cmd) || (cmd == SIOCSIWRATE) || (cmd == SIOCSIWENCODE))
			readCapabilityRid(local, &cap_rid);
		/* Get config in all cases, because SET will just modify it */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/block/cpqarray.c:1188:ida_ioctl: ERROR:VAR:1188:1188: suspicious var 'my_io' = 1296 bytes:1188 [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;

	switch(cmd) {
	case HDIO_GETGEO:
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/net/wanrouter/wanmain.c:754:device_new_if: ERROR:VAR:754:754: suspicious var 'conf' = 1272 bytes:754 [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;
	netdevice_t *dev=NULL;
#ifdef CONFIG_WANPIPE_MULTPPP
	struct ppp_device *pppdev=NULL;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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 */
#endif
  register ulg b;       /* bit buffer */
  register unsigned k;  /* number of bits in bit buffer */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/net/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 */
  register int w;               /* bits before this table == (l * h) */
  uInt x[BMAX+1];               /* bit offsets, then code stack */
  uIntf *xp;                    /* pointer into x */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/fs/jffs2/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 */
    z_stream z;         /* for falloc function */
    int f = FIXEDH;     /* number of hufts left in fixed_mem */

---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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 */
  register int w;               /* bits before this table == (l * h) */
  uInt x[BMAX+1];               /* bit offsets, then code stack */
  uIntf *xp;                    /* pointer into x */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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 */

DEBG("<fix");

---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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 */
  register int w;               /* bits before this table == (l * h) */
  unsigned x[BMAX+1];           /* bit offsets, then code stack */
  unsigned *xp;                 /* pointer into x */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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 */
    z_stream z;         /* for falloc function */
    int f = FIXEDH;     /* number of hufts left in fixed_mem */

---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/ide/ide-cs.c:247:ide_config: ERROR:VAR:367:367: function stack consumes 1148 bytes:367 [nbytes=1148]
	   link->conf.Vpp1/10, link->conf.Vpp1%10);

    link->state &= ~DEV_CONFIG_PENDING;
    return;
    

Error --->
cs_failed:
    cs_error(link->handle, last_fn, last_ret);
failed:
    ide_release((u_long)link);
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/parport/parport_cs.c:252:parport_config: ERROR:VAR:327:327: function stack consumes 1136 bytes:327 [nbytes=1136]
    printk("]\n");
    
    link->state &= ~DEV_CONFIG_PENDING;
    return;
    

Error --->
cs_failed:
    cs_error(link->handle, last_fn, last_ret);
failed:
    parport_cs_release((u_long)link);
---------------------------------------------------------
[BUG]  this function just keeps showing up...
/u2/engler/mc/oses/linux/2.4.17/drivers/telephony/ixj_pcmcia.c:221:ixj_config: ERROR:VAR:266:266: function stack consumes 1132 bytes:266 [nbytes=1132]
	info->node.major = PHONE_MAJOR;
	link->dev = &info->node;
	ixj_get_serial(link, j);
	link->state &= ~DEV_CONFIG_PENDING;
	return;

Error --->
      cs_failed:
	cs_error(link->handle, last_fn, last_ret);
	ixj_cs_release((u_long) link);
}
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/char/joystick/sidewinder.c:572:sw_connect: ERROR:VAR:572:572: function stack consumes 1093 bytes:572 [nbytes=1093]
	unsigned char m = 1;
	char comment[40];

	comment[0] = 0;


Error --->
	if (!(sw = kmalloc(sizeof(struct sw), GFP_KERNEL))) return;
	memset(sw, 0, sizeof(struct sw));

	gameport->private = sw;
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/cdrom/cdrom.c:738:cdrom_number_of_slots: ERROR:VAR:738:738: suspicious var 'info' = 1032 bytes:738 [nbytes=1032]
 */
int cdrom_number_of_slots(struct cdrom_device_info *cdi) 
{
	int status;
	int nslots = 1;

Error --->
	struct cdrom_changer_info info;

	cdinfo(CD_CHANGER, "entering cdrom_number_of_slots()\n"); 
	/* cdrom_read_mech_status requires a valid value for capacity: */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/cdrom/cdrom.c:780:cdrom_select_disc: ERROR:VAR:780:780: suspicious var 'info' = 1032 bytes:780 [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;
	int curslot;
	int ret;

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

	case CDROM_MEDIA_CHANGED: {

Error --->
		struct cdrom_changer_info info;

		cdinfo(CD_DO_IOCTL, "entering CDROM_MEDIA_CHANGED\n"); 
		if (!CDROM_CAN(CDC_MEDIA_CHANGED))
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/drivers/cdrom/cdrom.c:714:cdrom_slot_status: ERROR:VAR:714:714: suspicious var 'info' = 1032 bytes:714 [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;
	int ret;

	cdinfo(CD_CHANGER, "entering cdrom_slot_status()\n"); 
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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];
	int outpos = 0;
	int pos=0;
	
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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];
	char *cp;

	/* Set some default values */
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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],
	    	* phex = hex;

	if (len >= (sizeof(hex) / 2))
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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];
	int outpos = 0;
	int pos=0;

---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/arch/i386/kernel/nmi.c:48:check_nmi_watchdog: ERROR:VAR:48:48: suspicious var 'tmp' = 1024 bytes:48 [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];
	int j, cpu;

	printk(KERN_INFO "testing NMI watchdog ... ");
---------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.17/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];

		for (i=0;i<256;i++)
			indices[i] = 0;

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Pavel Machek]

Hi!

> # BUGs	|	File Name
> 4	|	/drivers/cdrom.c
> 4	|	/message/i2o_proc.c
> 3	|	/net/airo.c
> 3	|	/../inflate.c
> 2	|	/fs/zlib.c
> 2	|	/drivers/zlib.c
> 2	|	/drivers/cpqfcTScontrol.c
		~~~~~~~~~~~~~~~~~~~~~~~~~

Actually, 3 bugs, the name is so ugly that it is a bug, too :-).
									Pavel
-- 
(about SSSCA) "I don't say this lightly.  However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Nikita Danilov]

Pavel Machek writes:
 > Hi!
 > 
 > > # BUGs	|	File Name
 > > 4	|	/drivers/cdrom.c
 > > 4	|	/message/i2o_proc.c
 > > 3	|	/net/airo.c
 > > 3	|	/../inflate.c
 > > 2	|	/fs/zlib.c
 > > 2	|	/drivers/zlib.c
 > > 2	|	/drivers/cpqfcTScontrol.c
 > 		~~~~~~~~~~~~~~~~~~~~~~~~~

By the way, gcc has -Wlarger-than-NNNN option to do such checks.

 > 
 > Actually, 3 bugs, the name is so ugly that it is a bug, too :-).
 > 									Pavel

Nikita.

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Benjamin LaHaise]

On Sun, Jun 09, 2002 at 08:56:30PM -0700, Dawson Engler wrote:
> Here are 37 errors where variables >= 1024 bytes are allocated on a function's
> stack.

Is it possible to get checker to determine the stack depth of a worst 
case call chain (excluding interrupts)?  I've found that deep call chains 
are far more likely to cause stack overflows than short and bounded paths.

		-ben

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Alexander Viro]

On Wed, 12 Jun 2002, Benjamin LaHaise wrote:

> On Sun, Jun 09, 2002 at 08:56:30PM -0700, Dawson Engler wrote:
> > Here are 37 errors where variables >= 1024 bytes are allocated on a function's
> > stack.
> 
> Is it possible to get checker to determine the stack depth of a worst 
> case call chain (excluding interrupts)?  I've found that deep call chains 
> are far more likely to cause stack overflows than short and bounded paths.

Not realistic - we have a recursion through the ->follow_link(), and
a lot of stuff can be called from ->follow_link().  We _do_ have a
limit on depth of recursion here, but it won't be fun to deal with.

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Benjamin LaHaise]

On Wed, Jun 12, 2002 at 06:26:55PM -0400, Alexander Viro wrote:
> Not realistic - we have a recursion through the ->follow_link(), and
> a lot of stuff can be called from ->follow_link().  We _do_ have a
> limit on depth of recursion here, but it won't be fun to deal with.

Perfection isn't what I'm looking for, rather just an approximation.  
Any tool would have to give up on non-trivial recursion, or have 
additional rules imposed on the system.  Checker seems to be growing 
functionality in this area, so it seems like a useful feature request.

		-ben
-- 
"You will be reincarnated as a toad; and you will be much happier."

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Robert Love]

On Wed, 2002-06-12 at 15:38, Benjamin LaHaise wrote:

> Perfection isn't what I'm looking for, rather just an approximation.  
> Any tool would have to give up on non-trivial recursion, or have 
> additional rules imposed on the system.  Checker seems to be growing 
> functionality in this area, so it seems like a useful feature request.

I do not want to give up on this idea, either... if the implementation
needs to be "hackish" or even explicitly ignore certain functions, so be
it.  There is a _lot_ that can be done with detailed call chain analysis
-- the point you gave is one of many.

Checker already has some basic functionality here I suspect based on
what it is capable of reporting... imagine what more could be reported?

	Robert Love

procfs documentation

[Tom Bradley]

Where can I find documentation that explains all the entries in procfs?

man 5 procfs is out of date and doesn't match.
Documentation/filesystems/proc.txt is incomplete.

I am writing a user-land GUI based program that allows user to easily read
the procfs but I can't find info on all the entries.

Tom

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Alexander Viro]

On Wed, 12 Jun 2002, Benjamin LaHaise wrote:

> On Wed, Jun 12, 2002 at 06:26:55PM -0400, Alexander Viro wrote:
> > Not realistic - we have a recursion through the ->follow_link(), and
> > a lot of stuff can be called from ->follow_link().  We _do_ have a
> > limit on depth of recursion here, but it won't be fun to deal with.
> 
> Perfection isn't what I'm looking for, rather just an approximation.  
> Any tool would have to give up on non-trivial recursion, or have 

... in which case it will be useless - anything callable from path_walk()
will be out of its scope and that's a fairly large part of VFS, filesystems,
VM and upper halves of block devices.

> additional rules imposed on the system.  Checker seems to be growing 
> functionality in this area, so it seems like a useful feature request.

Just be careful with that loop - (mutual) recursion through ->follow_link()
must be special-cased if you want anything interesting to come out of that.

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Dawson Engler]

> 
> On Sun, Jun 09, 2002 at 08:56:30PM -0700, Dawson Engler wrote:
> > Here are 37 errors where variables >= 1024 bytes are allocated on a function's
> > stack.
> 
> Is it possible to get checker to determine the stack depth of a worst 
> case call chain (excluding interrupts)?  I've found that deep call chains 
> are far more likely to cause stack overflows than short and bounded paths.

Yeah, it's not that hard.  The main problem is determining if recursive
loops are feasible.  I'd released bugs from it before, but no one fixed any
so hadn't rerun it since.

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Dawson Engler]

> On Wed, 12 Jun 2002, Benjamin LaHaise wrote:
> 
> > On Sun, Jun 09, 2002 at 08:56:30PM -0700, Dawson Engler wrote:
> > > Here are 37 errors where variables >= 1024 bytes are allocated on a function's
> > > stack.
> > 
> > Is it possible to get checker to determine the stack depth of a worst 
> > case call chain (excluding interrupts)?  I've found that deep call chains 
> > are far more likely to cause stack overflows than short and bounded paths.
> 
> Not realistic - we have a recursion through the ->follow_link(), and
> a lot of stuff can be called from ->follow_link().  We _do_ have a
> limit on depth of recursion here, but it won't be fun to deal with.

You mean following function pointers is not realistic?  Actually the
function pointers in linxu are pretty easy to deal with since, by
and large, they are set by static structure initialization and not
really fussed with afterwards.

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Alexander Viro]

On Wed, 12 Jun 2002, Dawson Engler wrote:

> > Not realistic - we have a recursion through the ->follow_link(), and
> > a lot of stuff can be called from ->follow_link().  We _do_ have a
> > limit on depth of recursion here, but it won't be fun to deal with.
> 
> You mean following function pointers is not realistic?  Actually the
> function pointers in linxu are pretty easy to deal with since, by
> and large, they are set by static structure initialization and not
> really fussed with afterwards.

I mean that due to the loop (link_path_walk->do_follow_link->foofs_follow_link
->vfs_follow_link->link_path_walk) you will get infinite maximal depth
for everything that can be called by any of these functions.  And that's
a _lot_ of stuff.

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Helge Hafting]

Alexander Viro wrote:
> 
> On Wed, 12 Jun 2002, Benjamin LaHaise wrote:
> 
> > On Wed, Jun 12, 2002 at 06:26:55PM -0400, Alexander Viro wrote:
> > > Not realistic - we have a recursion through the ->follow_link(), and
> > > a lot of stuff can be called from ->follow_link().  We _do_ have a
> > > limit on depth of recursion here, but it won't be fun to deal with.
> >
> > Perfection isn't what I'm looking for, rather just an approximation.
> > Any tool would have to give up on non-trivial recursion, or have
> 
> ... in which case it will be useless - anything callable from path_walk()
> will be out of its scope and that's a fairly large part of VFS, filesystems,
> VM and upper halves of block devices.

The automated checker may use hard-coded limits for recursions with
limited depth.  If follow_link stops after n iterations, tell
the checker about it and it will use that in its computations.

Helge Hafting

Re: procfs documentation

[John Levon]

On Wed, Jun 12, 2002 at 04:55:49PM -0600, Tom Bradley wrote:

> Where can I find documentation that explains all the entries in procfs?
> 
> man 5 procfs is out of date and doesn't match.

Upgrade your man-pages.

> I am writing a user-land GUI based program that allows user to easily read
> the procfs but I can't find info on all the entries.

read the source

(and don't reply to irrelevant threads)

john

-- 
"All is change; all yields its place and goes"
	- Euripides

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Roger Larsson]

On Thursday 13 June 2002 10.30, Helge Hafting wrote:
> Alexander Viro wrote:
> > 
> > On Wed, 12 Jun 2002, Benjamin LaHaise wrote:
> > 
> > > On Wed, Jun 12, 2002 at 06:26:55PM -0400, Alexander Viro wrote:
> > > > Not realistic - we have a recursion through the ->follow_link(), and
> > > > a lot of stuff can be called from ->follow_link().  We _do_ have a
> > > > limit on depth of recursion here, but it won't be fun to deal with.
> > >
> > > Perfection isn't what I'm looking for, rather just an approximation.
> > > Any tool would have to give up on non-trivial recursion, or have
> > 
> > ... in which case it will be useless - anything callable from path_walk()
> > will be out of its scope and that's a fairly large part of VFS, 
filesystems,
> > VM and upper halves of block devices.
> 
> The automated checker may use hard-coded limits for recursions with
> limited depth.  If follow_link stops after n iterations, tell
> the checker about it and it will use that in its computations.

Alexander Viro <viro@math.psu.edu> wrote:
> (link_path_walk->do_follow_link->foofs_follow_link->
> vfs_follow_link->link_path_walk)

It would not need to follow the recursion at all.

A simple warning "vfs_follow_link makes a recursive call back
to link_path_walk, stack space needed for each recursion is N bytes"

/RogerL

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Daniel Phillips]

On Thursday 13 June 2002 08:59, Alexander Viro wrote:
> On Wed, 12 Jun 2002, Dawson Engler wrote:
> 
> > > Not realistic - we have a recursion through the ->follow_link(), and
> > > a lot of stuff can be called from ->follow_link().  We _do_ have a
> > > limit on depth of recursion here, but it won't be fun to deal with.
> > 
> > You mean following function pointers is not realistic?  Actually the
> > function pointers in linxu are pretty easy to deal with since, by
> > and large, they are set by static structure initialization and not
> > really fussed with afterwards.
> 
> I mean that due to the loop (link_path_walk->do_follow_link->foofs_follow_link
> ->vfs_follow_link->link_path_walk) you will get infinite maximal depth
> for everything that can be called by any of these functions.  And that's
> a _lot_ of stuff.

Then at the point of recursion a dynamic check for stack space is
needed, and [checker]'s role would be to determine the deepest static
depth, to plug into the stack check.  If we want to be sure about 
stack integrity there isn't any way around this.

-- 
Daniel

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Alexander Viro]

On Thu, 13 Jun 2002, Daniel Phillips wrote:

> > I mean that due to the loop (link_path_walk->do_follow_link->foofs_follow_link
> > ->vfs_follow_link->link_path_walk) you will get infinite maximal depth
> > for everything that can be called by any of these functions.  And that's
> > a _lot_ of stuff.
> 
> Then at the point of recursion a dynamic check for stack space is
> needed, and [checker]'s role would be to determine the deepest static
> depth, to plug into the stack check.  If we want to be sure about 
> stack integrity there isn't any way around this.

Wrong.  Check for stack _space_ will mean that maximal depth of nested
symlinks depends on syscall.  Definitely not what you want to see.
There is a static limit (no more than 5 nested), but it must be
explicitly known to checker - deducing it from code is easy for a
human, but hopeless for anything automatic.

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Andi Kleen]

Alexander Viro <viro@math.psu.edu> writes:
> 
> I mean that due to the loop (link_path_walk->do_follow_link->foofs_follow_link
> ->vfs_follow_link->link_path_walk) you will get infinite maximal depth
> for everything that can be called by any of these functions.  And that's
> a _lot_ of stuff.

Surely an analysis pass can detect recursive function chains and flag them
(e.g. the global IPA alias analysis pass in open64 does this)

-Andi

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Alexander Viro]

On 13 Jun 2002, Andi Kleen wrote:

> Alexander Viro <viro@math.psu.edu> writes:
> > 
> > I mean that due to the loop (link_path_walk->do_follow_link->foofs_follow_link
> > ->vfs_follow_link->link_path_walk) you will get infinite maximal depth
> > for everything that can be called by any of these functions.  And that's
> > a _lot_ of stuff.
> 
> Surely an analysis pass can detect recursive function chains and flag them
> (e.g. the global IPA alias analysis pass in open64 does this)

Ugh...  OK, let me try again:

One bit of apriory information needed to get anything interesting from
this analysis:  there is a set of mutually recursive functions (see above)
and there is a limit (5) on the depth of recursion in that loop.

It has to be known to checker.  Explicitly, since
	a) automatically deducing it is not realistic
	b) cutting off the stuff behind that loop will cut off a _lot_ of
things - both in filesystems and in VFS (and in block layer, while we are
at it).

I'm not saying that checker can't be used for that analysis - it can, but
naive approach (find recursive stuff and cut it off) will not be too
interesting.  One of the main stumbling blocks - see above.  With explicit
knowledge of that one the thing will be definitely very interesting - no
arguments here.

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Daniel Phillips]

On Thursday 13 June 2002 19:53, Alexander Viro wrote:
> On Thu, 13 Jun 2002, Daniel Phillips wrote:
> 
> > > I mean that due to the loop (link_path_walk->do_follow_link->foofs_follow_link
> > > ->vfs_follow_link->link_path_walk) you will get infinite maximal depth
> > > for everything that can be called by any of these functions.  And that's
> > > a _lot_ of stuff.
> > 
> > Then at the point of recursion a dynamic check for stack space is
> > needed, and [checker]'s role would be to determine the deepest static
> > depth, to plug into the stack check.  If we want to be sure about 
> > stack integrity there isn't any way around this.
> 
> Wrong.  Check for stack _space_ will mean that maximal depth of nested
> symlinks depends on syscall.  Definitely not what you want to see.
> There is a static limit (no more than 5 nested), but it must be
> explicitly known to checker - deducing it from code is easy for a
> human, but hopeless for anything automatic.

It's even hard to deduce the recursion in this case, and even if
checker is smart enough to spot it there's no way to know the
static requirement of out-of-tree filesystems.

Perhaps a BUG here is the right thing, in case the big chain of
assumptions here is inadvertently violated, in which case I'd much
rather have the system go down with a BUG than wig out in one of
the weird and wonderful ways typical of a stack overflow.

By the way:

327 /*
328  * This limits recursive symlink follows to 8, while
329  * limiting consecutive symlinks to 40.

Maybe:

327 /*
328  * This limits recursive symlink follows and consecutive symlinks.

-- 
Daniel

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Andi Kleen]

On Thu, Jun 13, 2002 at 08:26:54PM +0200, Alexander Viro wrote:
> Ugh...  OK, let me try again:
> 
> One bit of apriory information needed to get anything interesting from
> this analysis:  there is a set of mutually recursive functions (see above)
> and there is a limit (5) on the depth of recursion in that loop.
> 
> It has to be known to checker.  Explicitly, since
> 	a) automatically deducing it is not realistic
> 	b) cutting off the stuff behind that loop will cut off a _lot_ of
> things - both in filesystems and in VFS (and in block layer, while we are
> at it).
> 
> I'm not saying that checker can't be used for that analysis - it can, but
> naive approach (find recursive stuff and cut it off) will not be too

if you see all possible paths through the program as a tree which branches 
for every decision then you only need to cut off the branches that are 
actually pointing upward the tree again. This would still allow to follow
down into the callees of the recursive function because there should be 
at least one path that is non recursive (if not Checker should definitely
complain ;) 

e.g.
       ----<-----------------+
	   v                     |
 IF   TRUE                RECURSION
-------+------ some path ----+
       |
	  ELSE                    non recursive path 
       +-------------------------- other functions ---------->


Other functions can be still checked, you only need to prune the cycle.
I have no idea if checker's algorithms actually work like this, but I could
imagine that it would be one possible implementation.

-Andi

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Dawson Engler]

> On 13 Jun 2002, Andi Kleen wrote:
> 
> > Alexander Viro <viro@math.psu.edu> writes:
> > > 
> > > I mean that due to the loop (link_path_walk->do_follow_link->foofs_follow_link
> > > ->vfs_follow_link->link_path_walk) you will get infinite maximal depth
> > > for everything that can be called by any of these functions.  And that's
> > > a _lot_ of stuff.
> > 
> > Surely an analysis pass can detect recursive function chains and flag them
> > (e.g. the global IPA alias analysis pass in open64 does this)
> 
> Ugh...  OK, let me try again:
> 
> One bit of apriory information needed to get anything interesting from
> this analysis:  there is a set of mutually recursive functions (see above)
> and there is a limit (5) on the depth of recursion in that loop.
> 
> It has to be known to checker.  Explicitly, since
> 	a) automatically deducing it is not realistic
> 	b) cutting off the stuff behind that loop will cut off a _lot_ of
> things - both in filesystems and in VFS (and in block layer, while we are
> at it).

We're all about jamming system specific information into the checking
extensions.  It's usually just a few if statements.  So to be clear: we
can simply assume that the loop 
   link_path_walk
	->do_follow_link
		->foofs_follow_link
			->vfs_follow_link
				->link_path_walk
can happen exactly 5 times?

In general such restrictions are interesting, since they concretely
show how having a way to include system-specific knowlege into checking
is useful.  Are there any other such relationships?  The other example
I know of is the do_page_fault (sp?) routine that can potentially be
invoked at very copy_*_user site that page faults.  You need to know
this to detect certain classes of deadlock, but there's no way to
discern this path from the code without a priori knowlege.

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Oliver Xymoron]

On Thu, 13 Jun 2002, Dawson Engler wrote:

> > On 13 Jun 2002, Andi Kleen wrote:
> >
> > > Alexander Viro <viro@math.psu.edu> writes:
> > > >
> > > > I mean that due to the loop (link_path_walk->do_follow_link->foofs_follow_link
> > > > ->vfs_follow_link->link_path_walk) you will get infinite maximal depth
> > > > for everything that can be called by any of these functions.  And that's
> > > > a _lot_ of stuff.
> > >
> > > Surely an analysis pass can detect recursive function chains and flag them
> > > (e.g. the global IPA alias analysis pass in open64 does this)
> >
> > Ugh...  OK, let me try again:
> >
> > One bit of apriory information needed to get anything interesting from
> > this analysis:  there is a set of mutually recursive functions (see above)
> > and there is a limit (5) on the depth of recursion in that loop.
> >
> > It has to be known to checker.  Explicitly, since
> > 	a) automatically deducing it is not realistic
> > 	b) cutting off the stuff behind that loop will cut off a _lot_ of
> > things - both in filesystems and in VFS (and in block layer, while we are
> > at it).
>
> We're all about jamming system specific information into the checking
> extensions.  It's usually just a few if statements.  So to be clear: we
> can simply assume that the loop
>    link_path_walk
> 	->do_follow_link
> 		->foofs_follow_link
> 			->vfs_follow_link
> 				->link_path_walk
> can happen exactly 5 times?
>
> In general such restrictions are interesting, since they concretely
> show how having a way to include system-specific knowlege into checking
> is useful.  Are there any other such relationships?  The other example
> I know of is the do_page_fault (sp?) routine that can potentially be
> invoked at very copy_*_user site that page faults.  You need to know
> this to detect certain classes of deadlock, but there's no way to
> discern this path from the code without a priori knowlege.

There are various flags for memory allocation that prevent it (usually)
from being recursive (or livelocking).

Though I'm not really convinced by Al's argument. I think if you do a
breadth-first traversal of the possible call tree, as you get past a
certain predicted stack depth, you'll be able to manually prune things
that aren't of interest. Very few things should be potentially recursive
or mutally recursive and any that are probably bear closer manual
scrutiny.

How's Checker do on passing through function pointers?

-- 
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.."

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[William Lee Irwin III]

On Thu, Jun 13, 2002 at 09:01:30PM +0200, Andi Kleen wrote:
> if you see all possible paths through the program as a tree which branches 
> for every decision then you only need to cut off the branches that are 
> actually pointing upward the tree again. This would still allow to follow
> down into the callees of the recursive function because there should be 
> at least one path that is non recursive (if not Checker should definitely
> complain ;) 
> e.g.
>        ----<-----------------+
> 	   v                     |
>  IF   TRUE                RECURSION
> -------+------ some path ----+
>        |
> 	  ELSE                    non recursive path 
>        +-------------------------- other functions ---------->
> Other functions can be still checked, you only need to prune the cycle.
> I have no idea if checker's algorithms actually work like this, but I could
> imagine that it would be one possible implementation.

Why would you do it this way? AFAICT this is more naturally phrased as
cycle detection in a digraph.

Cheers,
Bill

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Alexander Viro]

On Thu, 13 Jun 2002, Dawson Engler wrote:

> > It has to be known to checker.  Explicitly, since
> > 	a) automatically deducing it is not realistic
> > 	b) cutting off the stuff behind that loop will cut off a _lot_ of
> > things - both in filesystems and in VFS (and in block layer, while we are
> > at it).
> 
> We're all about jamming system specific information into the checking
> extensions.  It's usually just a few if statements.  So to be clear: we
> can simply assume that the loop 
>    link_path_walk
> 	->do_follow_link
> 		->foofs_follow_link
> 			->vfs_follow_link
> 				->link_path_walk
> can happen exactly 5 times?

static inline int do_follow_link(struct dentry *dentry, struct nameidata *nd)
{   
        int err;
        if (current->link_count >= 5)
                goto loop;
...
        current->link_count++;
...
        err = dentry->d_inode->i_op->follow_link(dentry, nd);
        current->link_count--;
        return err;
loop:
        path_release(nd);
        return -ELOOP;
}

->link_count is modified only by the code above.

INIT_TASK has zero link_count.

new task inherits ->link_count of parent at the moment of do_fork()

It means that:
	* at any moment ->link_count is between 0 and 5
	* at any moment the call chain contains at most 6 instances of
do_follow_link()
	* if there are 6 such instances then the last of them is either the
last element of chain or it is followed by path_release().

	BTW, it shows a potential subtle problem - if we ever call do_fork()
while resolving a symlink (the only way that can happen is kernel_thread()
being called in process of symlink resolution) we will get a kernel thread
with limit for nested symlinks lower than 5.  Proposed fix (both 2.4 and 2.5):

ed kernel/fork.c <<EOF
/swappable/a
        p->link_count = 0;
.
w
EOF

Linus?

Re: [CHECKER] 37 stack variables >= 1K in 2.4.17

[Helge Hafting]

Roger Larsson wrote:

> > The automated checker may use hard-coded limits for recursions with
> > limited depth.  If follow_link stops after n iterations, tell
> > the checker about it and it will use that in its computations.
> 
> Alexander Viro <viro@math.psu.edu> wrote:
> > (link_path_walk->do_follow_link->foofs_follow_link->
> > vfs_follow_link->link_path_walk)
> 
> It would not need to follow the recursion at all.
> 
> A simple warning "vfs_follow_link makes a recursive call back
> to link_path_walk, stack space needed for each recursion is N bytes"
> 
That's all you can do with recursion of unknown depth, sure.

But the recursion here is known limited, and we want to know
"what is the deepest stack we can get into, following the
deepest call chain _after_ VFS recursed 5 levels of symlinks"
We know it won't recurse after that - but it might go
on calling x levels of non-recursive functions.

Hard-coding the limit for that particular call chain makes
sense as a lot of stuff may be called from it.  Or similiar,
various stuff may pile up on the stack and _then_ call into VFS
and recurse to the limit.

Printing the hardcoded assumption is probably a good idea when 
it is used to find some max depth - the kernel code might change
after all.


Helge Hafting