Skip to content

Commit

Permalink
check return value from read() and write() properly
Browse files Browse the repository at this point in the history
The system calls read and write may return less than the whole amount
requested for a number of reasons.  So the idioms
   if (read(fd, &object, sizeof(object)) != sizeof(object)) goto fail;
and even worse
   if (read(fd, &object, sizeof(object)) < 0) goto fail;
are wrong.  Additionally, read and write may sometimes return EINTR on
some systems so interruption is not desired or expected a loop is
needed.
  • Loading branch information
Ian Jackson authored and ijackson-citrix committed May 12, 2008
1 parent ff347fd commit a5975c8
Show file tree
Hide file tree
Showing 26 changed files with 161 additions and 145 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ qemu-img-%.o: %.c
$(CC) $(CFLAGS) $(CPPFLAGS) -c -o $@ $<

# dyngen host tool
dyngen$(EXESUF): dyngen.c
dyngen$(EXESUF): dyngen.c osdep.o
$(HOST_CC) $(CFLAGS) $(CPPFLAGS) -o $@ $^

clean:
Expand Down
13 changes: 8 additions & 5 deletions block-bochs.c
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ static int bochs_open(BlockDriverState *bs, const char *filename, int flags)

s->fd = fd;

if (read(fd, &bochs, sizeof(bochs)) != sizeof(bochs)) {
if (qemu_read(fd, &bochs, sizeof(bochs)) != sizeof(bochs)) {
goto fail;
}

Expand All @@ -151,7 +151,7 @@ static int bochs_open(BlockDriverState *bs, const char *filename, int flags)
s->catalog_bitmap = qemu_malloc(s->catalog_size * 4);
if (!s->catalog_bitmap)
goto fail;
if (read(s->fd, s->catalog_bitmap, s->catalog_size * 4) !=
if (qemu_read(s->fd, s->catalog_bitmap, s->catalog_size * 4) !=
s->catalog_size * 4)
goto fail;
for (i = 0; i < s->catalog_size; i++)
Expand All @@ -176,6 +176,7 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
int64_t offset = sector_num * 512;
int64_t extent_index, extent_offset, bitmap_offset, block_offset;
char bitmap_entry;
int r;

// seek to sector
extent_index = offset / s->extent_size;
Expand All @@ -200,7 +201,9 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
// read in bitmap for current extent
lseek(s->fd, bitmap_offset + (extent_offset / 8), SEEK_SET);

read(s->fd, &bitmap_entry, 1);
r = read(s->fd, &bitmap_entry, 1);
if (r < 0) return -1;
if (r == 0) { errno= EIO; return -1; }

if (!((bitmap_entry >> (extent_offset % 8)) & 1))
{
Expand All @@ -223,8 +226,8 @@ static int bochs_read(BlockDriverState *bs, int64_t sector_num,
while (nb_sectors > 0) {
if (!seek_to_sector(bs, sector_num))
{
ret = read(s->fd, buf, 512);
if (ret != 512)
ret = qemu_read_ok(s->fd, buf, 512);
if (ret < 0)
return -1;
}
else
Expand Down
10 changes: 5 additions & 5 deletions block-cloop.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,18 @@ static int cloop_open(BlockDriverState *bs, const char *filename, int flags)
close(s->fd);
return -1;
}
if(read(s->fd,&s->block_size,4)<4)
if(qemu_read_ok(s->fd,&s->block_size,4)<0)
goto cloop_close;
s->block_size=be32_to_cpu(s->block_size);
if(read(s->fd,&s->n_blocks,4)<4)
if(qemu_read_ok(s->fd,&s->n_blocks,4)<0)
goto cloop_close;
s->n_blocks=be32_to_cpu(s->n_blocks);

/* read offsets */
offsets_size=s->n_blocks*sizeof(uint64_t);
if(!(s->offsets=(uint64_t*)malloc(offsets_size)))
goto cloop_close;
if(read(s->fd,s->offsets,offsets_size)<offsets_size)
if(qemu_read_ok(s->fd,s->offsets,offsets_size)<0)
goto cloop_close;
for(i=0;i<s->n_blocks;i++) {
s->offsets[i]=be64_to_cpu(s->offsets[i]);
Expand Down Expand Up @@ -109,8 +109,8 @@ static inline int cloop_read_block(BDRVCloopState *s,int block_num)
uint32_t bytes = s->offsets[block_num+1]-s->offsets[block_num];

lseek(s->fd, s->offsets[block_num], SEEK_SET);
ret = read(s->fd, s->compressed_block, bytes);
if (ret != bytes)
ret = qemu_read_ok(s->fd, s->compressed_block, bytes);
if (ret < 0)
return -1;

s->zstream.next_in = s->compressed_block;
Expand Down
6 changes: 3 additions & 3 deletions block-cow.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ static int cow_open(BlockDriverState *bs, const char *filename, int flags)
}
s->fd = fd;
/* see if it is a cow image */
if (read(fd, &cow_header, sizeof(cow_header)) != sizeof(cow_header)) {
if (qemu_read_ok(fd, &cow_header, sizeof(cow_header)) < 0) {
goto fail;
}

Expand Down Expand Up @@ -159,8 +159,8 @@ static int cow_read(BlockDriverState *bs, int64_t sector_num,
while (nb_sectors > 0) {
if (is_changed(s->cow_bitmap, sector_num, nb_sectors, &n)) {
lseek(s->fd, s->cow_sectors_offset + sector_num * 512, SEEK_SET);
ret = read(s->fd, buf, n * 512);
if (ret != n * 512)
ret = qemu_read_ok(s->fd, buf, n * 512);
if (ret < 0)
return -1;
} else {
if (bs->backing_hd) {
Expand Down
23 changes: 7 additions & 16 deletions block-dmg.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
static off_t read_off(int fd)
{
uint64_t buffer;
if(read(fd,&buffer,8)<8)
if(qemu_read_ok(fd,&buffer,8)<0)
return 0;
return be64_to_cpu(buffer);
}

static off_t read_uint32(int fd)
{
uint32_t buffer;
if(read(fd,&buffer,4)<4)
if(qemu_read_ok(fd,&buffer,4)<0)
return 0;
return be32_to_cpu(buffer);
}
Expand Down Expand Up @@ -209,24 +209,15 @@ static inline int dmg_read_chunk(BDRVDMGState *s,int sector_num)
s->current_chunk = s->n_chunks;
switch(s->types[chunk]) {
case 0x80000005: { /* zlib compressed */
int i;

ret = lseek(s->fd, s->offsets[chunk], SEEK_SET);
if(ret<0)
return -1;

/* we need to buffer, because only the chunk as whole can be
* inflated. */
i=0;
do {
ret = read(s->fd, s->compressed_chunk+i, s->lengths[chunk]-i);
if(ret<0 && errno==EINTR)
ret=0;
i+=ret;
} while(ret>=0 && ret+i<s->lengths[chunk]);

if (ret != s->lengths[chunk])
return -1;
ret = qemu_read_ok(s->fd, s->compressed_chunk, s->lengths[chunk]);
if (ret < 0)
return -1;

s->zstream.next_in = s->compressed_chunk;
s->zstream.avail_in = s->lengths[chunk];
Expand All @@ -240,8 +231,8 @@ static inline int dmg_read_chunk(BDRVDMGState *s,int sector_num)
return -1;
break; }
case 1: /* copy */
ret = read(s->fd, s->uncompressed_chunk, s->lengths[chunk]);
if (ret != s->lengths[chunk])
ret = qemu_read_ok(s->fd, s->uncompressed_chunk, s->lengths[chunk]);
if (ret < 0)
return -1;
break;
case 2: /* zero */
Expand Down
7 changes: 3 additions & 4 deletions block-parallels.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ static int parallels_open(BlockDriverState *bs, const char *filename, int flags)

s->fd = fd;

if (read(fd, &ph, sizeof(ph)) != sizeof(ph))
if (qemu_read_ok(fd, &ph, sizeof(ph)) < 0)
goto fail;

if (memcmp(ph.magic, HEADER_MAGIC, 16) ||
Expand All @@ -103,8 +103,7 @@ static int parallels_open(BlockDriverState *bs, const char *filename, int flags)
s->catalog_bitmap = qemu_malloc(s->catalog_size * 4);
if (!s->catalog_bitmap)
goto fail;
if (read(s->fd, s->catalog_bitmap, s->catalog_size * 4) !=
s->catalog_size * 4)
if (qemu_read_ok(s->fd, s->catalog_bitmap, s->catalog_size * 4) < 0)
goto fail;
for (i = 0; i < s->catalog_size; i++)
le32_to_cpus(&s->catalog_bitmap[i]);
Expand Down Expand Up @@ -147,7 +146,7 @@ static int parallels_read(BlockDriverState *bs, int64_t sector_num,

while (nb_sectors > 0) {
if (!seek_to_sector(bs, sector_num)) {
if (read(s->fd, buf, 512) != 512)
if (qemu_read_ok(s->fd, buf, 512) < 0)
return -1;
} else
memset(buf, 0, 512);
Expand Down
18 changes: 14 additions & 4 deletions block-qcow.c
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ static void qcow_close(BlockDriverState *bs)
static int qcow_create(const char *filename, int64_t total_size,
const char *backing_file, int flags)
{
int fd, header_size, backing_filename_len, l1_size, i, shift;
int fd, header_size, backing_filename_len, l1_size, i, shift, errno_save;
QCowHeader header;
uint64_t tmp;

Expand Down Expand Up @@ -779,18 +779,28 @@ static int qcow_create(const char *filename, int64_t total_size,
header.crypt_method = cpu_to_be32(QCOW_CRYPT_NONE);
}


/* write all the data */
write(fd, &header, sizeof(header));
if (qemu_write_ok(fd, &header, sizeof(header)) < 0)
goto fail;
if (backing_file) {
write(fd, backing_file, backing_filename_len);
if (qemu_write_ok(fd, backing_file, backing_filename_len) < 0)
goto fail;
}
lseek(fd, header_size, SEEK_SET);
tmp = 0;
for(i = 0;i < l1_size; i++) {
write(fd, &tmp, sizeof(tmp));
if (qemu_write_ok(fd, &tmp, sizeof(tmp)) < 0)
goto fail;
}
close(fd);
return 0;

fail:
errno_save= errno;
close(fd);
errno= errno_save;
return -1;
}

static int qcow_make_empty(BlockDriverState *bs)
Expand Down
8 changes: 4 additions & 4 deletions block-raw-posix.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ static int raw_pread(BlockDriverState *bs, int64_t offset,
}
s->lseek_err_cnt=0;

ret = read(s->fd, buf, count);
ret = qemu_read(s->fd, buf, count);
if (ret == count)
goto label__raw_read__success;

Expand All @@ -175,11 +175,11 @@ static int raw_pread(BlockDriverState *bs, int64_t offset,
/* Try harder for CDrom. */
if (bs->type == BDRV_TYPE_CDROM) {
lseek(s->fd, offset, SEEK_SET);
ret = read(s->fd, buf, count);
ret = qemu_read(s->fd, buf, count);
if (ret == count)
goto label__raw_read__success;
lseek(s->fd, offset, SEEK_SET);
ret = read(s->fd, buf, count);
ret = qemu_read(s->fd, buf, count);
if (ret == count)
goto label__raw_read__success;

Expand Down Expand Up @@ -216,7 +216,7 @@ static int raw_pwrite(BlockDriverState *bs, int64_t offset,
}
s->lseek_err_cnt = 0;

ret = write(s->fd, buf, count);
ret = qemu_write(s->fd, buf, count);
if (ret == count)
goto label__raw_write__success;

Expand Down
18 changes: 10 additions & 8 deletions block-vmdk.c
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,13 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
/* read the header */
if (lseek(p_fd, 0x0, SEEK_SET) == -1)
goto fail;
if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE)
if (qemu_read_ok(p_fd, hdr, HEADER_SIZE) < 0)
goto fail;

/* write the header */
if (lseek(snp_fd, 0x0, SEEK_SET) == -1)
goto fail;
if (write(snp_fd, hdr, HEADER_SIZE) == -1)
if (qemu_write_ok(snp_fd, hdr, HEADER_SIZE) == -1)
goto fail;

memset(&header, 0, sizeof(header));
Expand All @@ -236,7 +236,7 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
/* the descriptor offset = 0x200 */
if (lseek(p_fd, 0x200, SEEK_SET) == -1)
goto fail;
if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE)
if (qemu_read_ok(p_fd, p_desc, DESC_SIZE) < 0)
goto fail;

if ((p_name = strstr(p_desc,"CID")) != 0) {
Expand All @@ -258,7 +258,7 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
/* write the descriptor */
if (lseek(snp_fd, 0x200, SEEK_SET) == -1)
goto fail;
if (write(snp_fd, s_desc, strlen(s_desc)) == -1)
if (qemu_write_ok(snp_fd, s_desc, strlen(s_desc)) == -1)
goto fail;

gd_offset = header.gd_offset * SECTOR_SIZE; // offset of GD table
Expand All @@ -280,11 +280,11 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
goto fail;
if (lseek(p_fd, rgd_offset, SEEK_SET) == -1)
goto fail_rgd;
if (read(p_fd, rgd_buf, gd_size) != gd_size)
if (qemu_read_ok(p_fd, rgd_buf, gd_size) < 0)
goto fail_rgd;
if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1)
goto fail_rgd;
if (write(snp_fd, rgd_buf, gd_size) == -1)
if (qemu_write_ok(snp_fd, rgd_buf, gd_size) == -1)
goto fail_rgd;
qemu_free(rgd_buf);

Expand All @@ -294,11 +294,11 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
goto fail_rgd;
if (lseek(p_fd, gd_offset, SEEK_SET) == -1)
goto fail_gd;
if (read(p_fd, gd_buf, gd_size) != gd_size)
if (qemu_read_ok(p_fd, gd_buf, gd_size) < 0)
goto fail_gd;
if (lseek(snp_fd, gd_offset, SEEK_SET) == -1)
goto fail_gd;
if (write(snp_fd, gd_buf, gd_size) == -1)
if (qemu_write_ok(snp_fd, gd_buf, gd_size) == -1)
goto fail_gd;
qemu_free(gd_buf);

Expand Down Expand Up @@ -765,6 +765,8 @@ static int vmdk_create(const char *filename, int64_t total_size,
header.check_bytes[2] = 0xd;
header.check_bytes[3] = 0xa;

/* BUG XXX No error checking anywhere here! XXX BUG */

/* write all the data */
write(fd, &magic, sizeof(magic));
write(fd, &header, sizeof(header));
Expand Down
15 changes: 7 additions & 8 deletions block-vpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)

s->fd = fd;

if (read(fd, &header, HEADER_SIZE) != HEADER_SIZE)
if (qemu_read_ok(fd, &header, HEADER_SIZE) < 0)
goto fail;

if (strncmp(header.magic, "conectix", 8))
goto fail;
lseek(s->fd, be32_to_cpu(header.type.main.subheader_offset), SEEK_SET);

if (read(fd, &header, HEADER_SIZE) != HEADER_SIZE)
if (qemu_read_ok(fd, &header, HEADER_SIZE) < 0)
goto fail;

if (strncmp(header.magic, "cxsparse", 8))
Expand All @@ -122,8 +122,7 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
s->pagetable = qemu_malloc(s->pagetable_entries * 4);
if (!s->pagetable)
goto fail;
if (read(s->fd, s->pagetable, s->pagetable_entries * 4) !=
s->pagetable_entries * 4)
if (qemu_read_ok(s->fd, s->pagetable, s->pagetable_entries * 4) < 0)
goto fail;
for (i = 0; i < s->pagetable_entries; i++)
be32_to_cpus(&s->pagetable[i]);
Expand Down Expand Up @@ -175,7 +174,7 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)

// Scary! Bitmap is stored as big endian 32bit entries,
// while we used to look it up byte by byte
read(s->fd, s->pageentry_u8, 512);
read(s->fd, s->pageentry_u8, 512); // no error handling ?!
for (i = 0; i < 128; i++)
be32_to_cpus(&s->pageentry_u32[i]);
}
Expand All @@ -185,7 +184,7 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
#else
lseek(s->fd, bitmap_offset + (pageentry_index / 8), SEEK_SET);

read(s->fd, &bitmap_entry, 1);
read(s->fd, &bitmap_entry, 1); // no error handling ?!

if ((bitmap_entry >> (pageentry_index % 8)) & 1)
return -1; // not allocated
Expand All @@ -205,8 +204,8 @@ static int vpc_read(BlockDriverState *bs, int64_t sector_num,
while (nb_sectors > 0) {
if (!seek_to_sector(bs, sector_num))
{
ret = read(s->fd, buf, 512);
if (ret != 512)
ret = qemu_read_ok(s->fd, buf, 512);
if (ret < 0)
return -1;
}
else
Expand Down
Loading

0 comments on commit a5975c8

Please sign in to comment.