From owner-freebsd-bugs@FreeBSD.ORG Thu Jul 14 00:20:16 2005 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0707F16A41C for ; Thu, 14 Jul 2005 00:20:16 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 273AB43D48 for ; Thu, 14 Jul 2005 00:20:15 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.3/8.13.3) with ESMTP id j6E0KF0C045436 for ; Thu, 14 Jul 2005 00:20:15 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.3/8.13.1/Submit) id j6E0KFDH045432; Thu, 14 Jul 2005 00:20:15 GMT (envelope-from gnats) Resent-Date: Thu, 14 Jul 2005 00:20:15 GMT Resent-Message-Id: <200507140020.j6E0KFDH045432@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Dan Lukes Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C5EBB16A41C for ; Thu, 14 Jul 2005 00:19:45 +0000 (GMT) (envelope-from dan@kulesh.obluda.cz) Received: from kulesh.obluda.cz (kulesh.obluda.cz [193.179.22.243]) by mx1.FreeBSD.org (Postfix) with ESMTP id 29ED543D48 for ; Thu, 14 Jul 2005 00:19:42 +0000 (GMT) (envelope-from dan@kulesh.obluda.cz) Received: from kulesh.obluda.cz (localhost.eunet.cz [127.0.0.1]) by kulesh.obluda.cz (8.13.3/8.13.3) with ESMTP id j6E0JfK4027673 for ; Thu, 14 Jul 2005 02:19:41 +0200 (CEST) (envelope-from dan@kulesh.obluda.cz) Received: (from root@localhost) by kulesh.obluda.cz (8.13.3/8.13.1/Submit) id j6E0JeNv027672; Thu, 14 Jul 2005 02:19:40 +0200 (CEST) (envelope-from dan) Message-Id: <200507140019.j6E0JeNv027672@kulesh.obluda.cz> Date: Thu, 14 Jul 2005 02:19:40 +0200 (CEST) From: Dan Lukes To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: bin/83424: [ PATCH ] improper handling of malloc failures within libstand X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Dan Lukes List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Jul 2005 00:20:16 -0000 >Number: 83424 >Category: bin >Synopsis: [ PATCH ] improper handling of malloc failures within libstand >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Jul 14 00:20:14 GMT 2005 >Closed-Date: >Last-Modified: >Originator: Dan Lukes >Release: FreeBSD 5.4-STABLE i386 >Organization: Obludarium >Environment: System: FreeBSD 5.4-STABLE #8: Sat Jul 9 16:31:08 CEST 2005 i386 lib/libstand/bzipfs.c,v 1.6 2004/01/21 20:12:23 jhb lib/libstand/cd9660.c,v 1.11 2001/11/06 22:31:10 jhb lib/libstand/dosfs.c,v 1.7 2004/01/21 20:12:23 jhb lib/libstand/environment.c,v 1.6 2003/10/26 04:04:12 peter lib/libstand/ext2fs.c,v 1.5 2004/01/21 20:12:23 jhb lib/libstand/nfs.c,v 1.12 2004/01/21 20:12:23 jhb lib/libstand/open.c,v 1.6 2001/09/30 22:28:01 dillon lib/libstand/splitfs.c,v 1.6 2004/01/21 20:12:23 jhb lib/libstand/ufs.c,v 1.14 2004/01/21 20:12:23 jhb lib/libstand/bswap.c,v 1.2 2001/09/30 22:28:00 dillon lib/libstand/Makefile,v 1.43.2.1 2005/02/13 07:23:14 obrien >Description: The major problem is that there are too many places within libstand where the malloc's return value isn't properly checked. Minor problem is that bswap64() code break strict-aliasing rules WARNS level can be raised to 2 >How-To-Repeat: >Fix: --- patch begins here --- --- lib/libstand/bzipfs.c.ORIG Mon Jan 26 18:09:54 2004 +++ lib/libstand/bzipfs.c Thu Jul 14 00:25:48 2005 @@ -155,6 +155,8 @@ /* Construct new name */ bzfname = malloc(strlen(fname) + 5); + if (bzfname == NULL) + return(ENOMEM); sprintf(bzfname, "%s.bz2", fname); /* Try to open the compressed datafile */ @@ -176,6 +178,11 @@ /* Allocate a bz_file structure, populate it */ bzf = malloc(sizeof(struct bz_file)); + if (bzf == NULL) { + printf("bzf_open: not enought memory\n"); + close(rawfd); + return(ENOMEM); + } bzero(bzf, sizeof(struct bz_file)); bzf->bzf_rawfd = rawfd; --- lib/libstand/cd9660.c.ORIG Thu Jul 14 00:07:07 2005 +++ lib/libstand/cd9660.c Thu Jul 14 00:12:05 2005 @@ -283,17 +283,19 @@ static int cd9660_open(const char *path, struct open_file *f) { - struct file *fp = 0; + struct file *fp = NULL; void *buf; struct iso_primary_descriptor *vd; size_t buf_size, read, dsize, off; daddr_t bno, boff; struct iso_directory_record rec; - struct iso_directory_record *dp = 0; + struct iso_directory_record *dp = NULL; int rc, first, use_rrip, lenskip; /* First find the volume descriptor */ buf = malloc(buf_size = ISO_DEFAULT_BLOCK_SIZE); + if (buf == NULL) + return(ENOMEM); vd = buf; for (bno = 16;; bno++) { twiddle(); @@ -378,6 +380,10 @@ /* allocate file system specific data structure */ fp = malloc(sizeof(struct file)); + if (fp == NULL) { + rc = ENOMEM; + goto out; + } bzero(fp, sizeof(struct file)); f->f_fsdata = (void *)fp; @@ -413,8 +419,7 @@ return 0; out: - if (fp) - free(fp); + free(fp); free(buf); return rc; @@ -443,8 +448,11 @@ blkoff = fp->f_off % ISO_DEFAULT_BLOCK_SIZE; if (blkno != fp->f_buf_blkno) { - if (fp->f_buf == (char *)0) + if (fp->f_buf == (char *)NULL) { fp->f_buf = malloc(ISO_DEFAULT_BLOCK_SIZE); + if (fp->f_buf == NULL) + return(ENOMEM); + } twiddle(); rc = f->f_dev->dv_strategy(f->f_devdata, F_READ, --- lib/libstand/dosfs.c.ORIG Mon Jan 26 18:09:54 2004 +++ lib/libstand/dosfs.c Thu Jul 14 00:38:00 2005 @@ -202,16 +202,20 @@ DOS_FS *fs; u_int size, clus; int err = 0; + int mounted = 0; /* Allocate mount structure, associate with open */ fs = malloc(sizeof(DOS_FS)); + if (fs == NULL) + return(ENOMEM); if ((err = dos_mount(fs, fd))) goto out; + mounted = 1; if ((err = namede(fs, path, &de))) goto out; - + clus = stclus(fs->fatsz, de); size = cv4(de->size); @@ -222,6 +226,11 @@ goto out; } f = malloc(sizeof(DOS_FILE)); + if (f == NULL) { + err = ENOMEM; + goto out; + } + bzero(f, sizeof(DOS_FILE)); f->fs = fs; fs->links++; @@ -229,6 +238,8 @@ fd->f_fsdata = (void *)f; out: + if (mounted) + dos_unmount(fs); return(err); } --- lib/libstand/environment.c.ORIG Fri Nov 14 03:26:04 2003 +++ lib/libstand/environment.c Thu Jul 14 00:06:06 2005 @@ -82,7 +82,13 @@ * New variable; create and sort into list */ ev = malloc(sizeof(struct env_var)); + if (ev == NULL) + return(-1); ev->ev_name = strdup(name); + if (ev->ev_name == NULL) { + free(ev); + return(-1); + } ev->ev_value = NULL; /* hooks can only be set when the variable is instantiated */ ev->ev_sethook = sethook; @@ -126,7 +132,7 @@ if (flags & EV_VOLATILE) { ev->ev_value = strdup(value); } else { - ev->ev_value = value; + ev->ev_value = (void *)value; } /* Keep the flag components that are relevant */ --- lib/libstand/ext2fs.c.ORIG Mon Jan 26 18:09:54 2004 +++ lib/libstand/ext2fs.c Thu Jul 14 00:59:56 2005 @@ -352,6 +352,10 @@ /* allocate space and read super block */ fs = (struct ext2fs *)malloc(sizeof(*fs)); + if (fs == NULL) { + error = ENOMEM; + goto out; + } fp->f_fs = fs; twiddle(); error = (f->f_dev->dv_strategy)(f->f_devdata, F_READ, @@ -395,6 +399,10 @@ len = blkgrps * fs->fs_bsize; fp->f_bg = malloc(len); + if (fp->f_bg == NULL) { + error = ENOMEM; + goto out; + } twiddle(); error = (f->f_dev->dv_strategy)(f->f_devdata, F_READ, EXT2_SBLOCK + EXT2_SBSIZE / DEV_BSIZE, len, @@ -501,8 +509,10 @@ daddr_t disk_block; size_t buf_size; - if (! buf) - buf = malloc(fs->fs_bsize); + if (buf == NULL && (buf = malloc(fs->fs_bsize)) == NULL) { + error = ENOMEM; + goto out; + } error = block_map(f, (daddr_t)0, &disk_block); if (error) goto out; @@ -537,13 +547,10 @@ */ error = 0; out: - if (buf) - free(buf); - if (path) - free(path); - if (error) { - if (fp->f_buf) - free(fp->f_buf); + free(buf); + free(path); + if (error && fp) { + free(fp->f_buf); free(fp->f_fs); free(fp); } @@ -567,6 +574,10 @@ * Read inode and save it. */ buf = malloc(fs->fs_bsize); + if (buf == NULL) { + error = ENOMEM; + goto out; + } twiddle(); error = (f->f_dev->dv_strategy)(f->f_devdata, F_READ, ino_to_db(fs, fp->f_bg, inumber), fs->fs_bsize, buf, &rsize); @@ -660,9 +671,10 @@ } if (fp->f_blkno[level] != ind_block_num) { - if (fp->f_blk[level] == (char *)0) - fp->f_blk[level] = - malloc(fs->fs_bsize); + if (fp->f_blk[level] == (char *)NULL && + (fp->f_blk[level] = malloc(fs->fs_bsize)) + == NULL) + return(ENOMEM); twiddle(); error = (f->f_dev->dv_strategy)(f->f_devdata, F_READ, fsb_to_db(fp->f_fs, ind_block_num), fs->fs_bsize, @@ -714,8 +726,11 @@ if (error) goto done; - if (fp->f_buf == (char *)0) - fp->f_buf = malloc(fs->fs_bsize); + if (fp->f_buf == (char *)NULL && + (fp->f_buf = malloc(fs->fs_bsize)) == NULL) { + error = ENOMEM; + goto done; + }; if (disk_block == 0) { bzero(fp->f_buf, block_size); --- lib/libstand/nfs.c.ORIG Mon Jan 26 18:09:54 2004 +++ lib/libstand/nfs.c Thu Jul 14 01:08:54 2005 @@ -471,6 +471,10 @@ /* allocate file system specific data structure */ newfd = malloc(sizeof(*newfd)); + if (newfd == NULL) { + error = ENOMEM; + goto out; + } newfd->iodesc = currfd->iodesc; newfd->off = 0; @@ -552,6 +556,8 @@ #else /* allocate file system specific data structure */ currfd = malloc(sizeof(*currfd)); + if (currfd == NULL) + return(ENOMEM); currfd->iodesc = desc; currfd->off = 0; --- lib/libstand/open.c.ORIG Mon Oct 1 00:28:01 2001 +++ lib/libstand/open.c Thu Jul 14 00:21:50 2005 @@ -82,12 +82,15 @@ return(-1); } -static void +static int o_rainit(struct open_file *f) { f->f_rabuf = malloc(SOPEN_RASIZE); + if (f->f_rabuf == NULL) + return(-1); f->f_ralen = 0; f->f_raoffset = 0; + return(0); } int @@ -128,7 +131,8 @@ if (error == 0) { f->f_ops = file_system[i]; - o_rainit(f); + if (o_rainit(f) == -1) + goto err; return (fd); } if (error != EINVAL) --- lib/libstand/splitfs.c.ORIG Mon Jan 26 18:09:54 2004 +++ lib/libstand/splitfs.c Thu Jul 14 01:13:16 2005 @@ -118,6 +118,8 @@ /* Construct new name */ confname = malloc(strlen(fname) + 7); + if (confname == NULL) + return(ENOMEM); sprintf(confname, "%s.split", fname); /* Try to open the configuration file */ @@ -139,8 +141,14 @@ /* Allocate a split_file structure, populate it from the config file */ sf = malloc(sizeof(struct split_file)); - bzero(sf, sizeof(struct split_file)); buf = malloc(CONF_BUF); + if (sf == NULL || buf == NULL) { + free(sf); + free(buf); + close(conffd); + return(ENOMEM); + } + bzero(sf, sizeof(struct split_file)); while (fgetstr(buf, CONF_BUF, conffd) > 0) { cp = buf; while ((*cp != '\0') && (isspace(*cp) == 0)) @@ -192,7 +200,7 @@ static int splitfs_read(struct open_file *f, void *buf, size_t size, size_t *resid) { - int i, nread, totread; + int nread, totread; struct split_file *sf; sf = (struct split_file *)f->f_fsdata; --- lib/libstand/ufs.c.ORIG Mon Jan 26 18:09:54 2004 +++ lib/libstand/ufs.c Thu Jul 14 01:21:38 2005 @@ -163,6 +163,10 @@ * Read inode and save it. */ buf = malloc(fs->fs_bsize); + if (buf == NULL) { + rc = ENOMEM; + goto out; + } twiddle(); rc = (f->f_dev->dv_strategy)(f->f_devdata, F_READ, fsbtodb(fs, ino_to_fsba(fs, inumber)), fs->fs_bsize, @@ -269,9 +273,10 @@ } if (fp->f_blkno[level] != ind_block_num) { - if (fp->f_blk[level] == (char *)0) - fp->f_blk[level] = - malloc(fs->fs_bsize); + if (fp->f_blk[level] == (char *)NULL && + (fp->f_blk[level] = malloc(fs->fs_bsize)) + == NULL) + return(ENOMEM); twiddle(); rc = (f->f_dev->dv_strategy)(f->f_devdata, F_READ, fsbtodb(fp->f_fs, ind_block_num), @@ -350,8 +355,9 @@ if (((off > 0) || (*size_p + off < block_size)) && (file_block != fp->f_buf_blkno)) { - if (fp->f_buf == (char *)0) - fp->f_buf = malloc(fs->fs_bsize); + if (fp->f_buf == (char *)NULL && + (fp->f_buf = malloc(fs->fs_bsize)) == NULL) + return(ENOMEM); twiddle(); rc = (f->f_dev->dv_strategy)(f->f_devdata, F_READ, @@ -402,8 +408,9 @@ block_size = sblksize(fs, DIP(fp, di_size), file_block); if (file_block != fp->f_buf_blkno) { - if (fp->f_buf == (char *)0) - fp->f_buf = malloc(fs->fs_bsize); + if (fp->f_buf == (char *)NULL && + (fp->f_buf = malloc(fs->fs_bsize)) == NULL) + return(ENOMEM); rc = block_map(f, file_block, &disk_block); if (rc) @@ -516,11 +523,19 @@ /* allocate file system specific data structure */ fp = malloc(sizeof(struct file)); + if (fp == NULL) { + rc = ENOMEM; + goto out; + } bzero(fp, sizeof(struct file)); f->f_fsdata = (void *)fp; /* allocate space and read super block */ fs = malloc(SBLOCKSIZE); + if (fs == NULL) { + rc = ENOMEM; + goto out; + } fp->f_fs = fs; twiddle(); /* @@ -650,8 +665,11 @@ ufs2_daddr_t disk_block; struct fs *fs = fp->f_fs; - if (!buf) - buf = malloc(fs->fs_bsize); + if (buf == NULL && + (buf = malloc(fs->fs_bsize)) == NULL) { + rc = ENOMEM; + goto out; + } rc = block_map(f, (ufs2_daddr_t)0, &disk_block); if (rc) goto out; @@ -690,9 +708,8 @@ free(buf); if (path) free(path); - if (rc) { - if (fp->f_buf) - free(fp->f_buf); + if (rc && fp) { + free(fp->f_buf); free(fp->f_fs); free(fp); } --- lib/libstand/bswap.c.ORIG Mon Jul 1 22:53:56 2002 +++ lib/libstand/bswap.c Thu Jul 14 01:54:49 2005 @@ -30,11 +30,7 @@ bswap64(x) u_int64_t x; { - u_int32_t *p = (u_int32_t*)&x; - u_int32_t t; - t = bswap32(p[0]); - p[0] = bswap32(p[1]); - p[1] = t; - return x; + return ((u_int64_t)bswap32(x & 0xffffffff) << 32) | + (u_int64_t)bswap32((x >> 32) & 0xffffffff); } --- lib/libstand/Makefile.ORIG Mon Feb 14 12:33:34 2005 +++ lib/libstand/Makefile Thu Jul 14 00:03:48 2005 @@ -184,6 +184,8 @@ SRCS+= dosfs.c ext2fs.c SRCS+= splitfs.c +WARNS+= 2 + .include .if ${MACHINE_ARCH} == "amd64" --- patch ends here --- >Release-Note: >Audit-Trail: >Unformatted: