bin/83424: [ PATCH ] improper handling of malloc failures within
libstand
Dan Lukes
dan at obluda.cz
Thu Jul 14 00:20:16 GMT 2005
>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 <bsd.lib.mk>
.if ${MACHINE_ARCH} == "amd64"
--- patch ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
More information about the freebsd-bugs
mailing list