Fix compilation with _FORTIFY_SOURCE

PATCH In review Juan Quintela 4 participants 23 messages Jan 20, 2010 – Jan 25, 2010

Subsystems (2)

Networking Block layer

lore

Share & tools

lore

Juan Quintela

author

2010-01-20 20:14 UTC #0 lore raw

v2 cover Fix compilation with _FORTIFY_SOURCE

changes in v2:

- we retry the write in the signal handler when error is -EINTR (just use the qemu_write_full() function) (spoted by Jamie Lokier)
- we don't do arith on void * anymore (spoted by malc)

and there were no more pending commets.

you can get it as a branch at:
git://repo.or.cz/qemu/quintela.git fortify

Later, Juan.

v1:

This series is a work on top of Kirill previous patches. Changes are:
- I addressed all reviewers comments
- Improved some error messages
- Check that it is valid to return -errno (switched lots of places to just return -1).
- check correctly system() result for errors.
- -fstack-protector-all is only used if compiler accept it

And new additions:
- add WARN_UNUSED_RESULT
- check pipe() use in xen code.
- fix mmap_frag() returned -EINVAL when users only deal with -1 for errors.

Kirill, could you coment on the series?

Still not completely happy:
- qemu_write_full() just loops if write() returns 0. it is only used for pipes and files, but pipes and files shouldn't give you short writes in the 1st place. Not sure what to do here.

- check write() errors against -EINVAL/-EINTR/-EAGAIN series where not consistent on this regard. I didn't changed any.

You can pull this series from:

git://repo.or.cz/qemu/quintela.git fortify

With this changes, I am able to compile qemu for all architectures in a linux host without a single warning.

This is important for us (Fedora) because F12 compile all packages with -Wp,-D_FORTIFY_SOURCE=2, and we had -Werror disabled due to this.

Juan Quintela (4):
force to test result for qemu_write_full()
check pipe() return value
Check availavility of -fstack-protector-all
mmap_frag() users only check for -1 error

Kirill A. Shutemov (13):
Introduce qemu_write_full() posix-aio-compat.c: fix warning with _FORTIFY_SOURCE block/cow.c: fix warnings with _FORTIFY_SOURCE block/qcow.c: fix warnings with _FORTIFY_SOURCE block/vmdk.o: fix warnings with _FORTIFY_SOURCE block/vvfat.c: fix warnings with _FORTIFY_SOURCE block/qcow2.c: fix warnings with _FORTIFY_SOURCE net/slirp.c: fix warning with _FORTIFY_SOURCE usb-linux.c: fix warning with _FORTIFY_SOURCE vl.c: fix warning with _FORTIFY_SOURCE monitor.c: fix warnings with _FORTIFY_SOURCE linux-user/mmap.c: fix warnings with _FORTIFY_SOURCE Enable _FORTIFY_SOURCE=2

 block/cow.c          |   19 ++++++++++++++--
 block/qcow.c         |   26 ++++++++++++++++++++---
 block/qcow2.c        |   55 ++++++++++++++++++++++++++++++++++++++++---------
 block/vmdk.c         |   50 ++++++++++++++++++++++++++++++++++++---------
 block/vvfat.c        |    9 ++++++-
 configure            |    4 +-
 hw/xen_domainbuild.c |   13 ++++++++---
 linux-user/mmap.c    |    8 ++++--
 monitor.c            |   12 +++++++++-
 net/slirp.c          |    9 +++++++-
 osdep.c              |   27 ++++++++++++++++++++++++
 posix-aio-compat.c   |    5 +++-
 qemu-common.h        |    2 +
 usb-linux.c          |    3 +-
 vl.c                 |   22 ++++++++++++++++---
 15 files changed, 216 insertions(+), 48 deletions(-)

Juan Quintela

author

2010-01-20 20:14 UTC #1 lore raw

PATCH v2 1/17 Introduce qemu_write_full()

From: Kirill A. Shutemov <[email protected]>

A variant of write(2) which handles partial write.

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 osdep.c       |   27 +++++++++++++++++++++++++++
 qemu-common.h |    1 +
 2 files changed, 28 insertions(+), 0 deletions(-)

osdep.c +27 -0

index 1310684..c5d608e 100644
@@ -243,6 +243,33 @@ int qemu_open(const char *name, int flags, ...)
     return ret;
 }
 
+/*
+ * A variant of write(2) which handles partial write.
+ *
+ * Return the number of bytes transferred.
+ * Set errno if fewer than `count' bytes are written.
+ */
+ssize_t qemu_write_full(int fd, const void *buf, size_t count)
+{
+    ssize_t written = 0;
+    const char *mem = buf;
+
+    while (count) {
+        ssize_t ret = write(fd, mem, count);
+        if (ret < 0) {
+            if (errno == EINTR || errno == EAGAIN)
+                continue;
+            break;
+        }
+
+        count -= ret;
+        mem += ret;
+        written += ret;
+    }
+
+    return written;
+}
+
 #ifndef _WIN32
 /*
  * Creates a pipe with FD_CLOEXEC set on both file descriptors

qemu-common.h +1 -0

index d96060a..a2731e1 100644
@@ -161,6 +161,7 @@ void qemu_mutex_lock_iothread(void);
 void qemu_mutex_unlock_iothread(void);
 
 int qemu_open(const char *name, int flags, ...);
+ssize_t qemu_write_full(int fd, const void *buf, size_t count);
 void qemu_set_cloexec(int fd);
 
 #ifndef _WIN32

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #2 lore raw

PATCH v2 2/17 force to test result for qemu_write_full()

In reply to Juan Quintela

Signed-off-by Juan Quintela <[email protected]>

---
 qemu-common.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

qemu-common.h +2 -1

index a2731e1..ae4f23b 100644
@@ -161,7 +161,8 @@ void qemu_mutex_lock_iothread(void);
 void qemu_mutex_unlock_iothread(void);
 
 int qemu_open(const char *name, int flags, ...);
-ssize_t qemu_write_full(int fd, const void *buf, size_t count);
+ssize_t qemu_write_full(int fd, const void *buf, size_t count)
+    QEMU_WARN_UNUSED_RESULT;
 void qemu_set_cloexec(int fd);
 
 #ifndef _WIN32

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #3 lore raw

PATCH v2 3/17 posix-aio-compat.c: fix warning with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC posix-aio-compat.o
cc1: warnings being treated as errors
posix-aio-compat.c: In function 'aio_signal_handler':
posix-aio-compat.c:505: error: ignoring return value of 'write', declared with attribute warn_unused_result
make: *** [posix-aio-compat.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 posix-aio-compat.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

posix-aio-compat.c +4 -1

index dc14f53..f5f3db6 100644
@@ -501,8 +501,11 @@ static void aio_signal_handler(int signum)
 {
     if (posix_aio_state) {
         char byte = 0;
+        ssize_t ret;
 
-        write(posix_aio_state->wfd, &byte, sizeof(byte));
+        ret = qemu_write_full(posix_aio_state->wfd, &byte, sizeof(byte));
+        if (ret < 0)
+            die("write()");
     }
 
     qemu_service_io();

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #4 lore raw

PATCH v2 4/17 block/cow.c: fix warnings with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC block/cow.o
cc1: warnings being treated as errors
block/cow.c: In function 'cow_create':
block/cow.c:251: error: ignoring return value of 'write', declared with attribute warn_unused_result block/cow.c:253: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result
make: *** [block/cow.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 block/cow.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

block/cow.c +16 -3

index a70854e..3733385 100644
@@ -209,6 +209,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
     struct stat st;
     int64_t image_sectors = 0;
     const char *image_filename = NULL;
+    int ret;
 
     /* Read out options */
     while (options && options->name) {

@@ -248,11 +249,23 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
     }
     cow_header.sectorsize = cpu_to_be32(512);
     cow_header.size = cpu_to_be64(image_sectors * 512);
-    write(cow_fd, &cow_header, sizeof(cow_header));
+    ret = qemu_write_full(cow_fd, &cow_header, sizeof(cow_header));
+    if (ret != sizeof(cow_header)) {
+        ret = -1;
+        goto exit;
+    }
+
     /* resize to include at least all the bitmap */
-    ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) >> 3));
+    ret = ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) >> 3));
+    if (ret) {
+        ret = -errno;
+        goto exit;
+    }
+
+    ret = 0;
+exit:
     close(cow_fd);
-    return 0;
+    return ret;
 }
 
 static void cow_flush(BlockDriverState *bs)

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #5 lore raw

PATCH v2 5/17 block/qcow.c: fix warnings with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC block/qcow.o
cc1: warnings being treated as errors
block/qcow.c: In function 'qcow_create':
block/qcow.c:804: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow.c:806: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow.c:811: error: ignoring return value of 'write', declared with attribute warn_unused_result
make: *** [block/qcow.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 block/qcow.c |   26 ++++++++++++++++++++++----
 1 files changed, 22 insertions(+), 4 deletions(-)

block/qcow.c +22 -4

index 1e3e59b..003db1e 100644
@@ -750,6 +750,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)
     int64_t total_size = 0;
     const char *backing_file = NULL;
     int flags = 0;
+    int ret;
 
     /* Read out options */
     while (options && options->name) {

@@ -801,17 +802,34 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)
     }
 
     /* write all the data */
-    write(fd, &header, sizeof(header));
+    ret = qemu_write_full(fd, &header, sizeof(header));
+    if (ret != sizeof(header)) {
+        ret = -1;
+        goto exit;
+    }
+
     if (backing_file) {
-        write(fd, backing_file, backing_filename_len);
+        ret = qemu_write_full(fd, backing_file, backing_filename_len);
+        if (ret != backing_filename_len) {
+            ret = -1;
+            goto exit;
+        }
+
     }
     lseek(fd, header_size, SEEK_SET);
     tmp = 0;
     for(i = 0;i < l1_size; i++) {
-        write(fd, &tmp, sizeof(tmp));
+        ret = qemu_write_full(fd, &tmp, sizeof(tmp));
+        if (ret != sizeof(tmp)) {
+            ret = -1;
+            goto exit;
+        }
     }
+
+    ret = 0;
+exit:
     close(fd);
-    return 0;
+    return ret;
 }
 
 static int qcow_make_empty(BlockDriverState *bs)

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #6 lore raw

PATCH v2 6/17 block/vmdk.o: fix warnings with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC block/vmdk.o
cc1: warnings being treated as errors
block/vmdk.c: In function 'vmdk_snapshot_create':
block/vmdk.c:236: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result block/vmdk.c: In function 'vmdk_create':
block/vmdk.c:775: error: ignoring return value of 'write', declared with attribute warn_unused_result block/vmdk.c:776: error: ignoring return value of 'write', declared with attribute warn_unused_result block/vmdk.c:778: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result block/vmdk.c:784: error: ignoring return value of 'write', declared with attribute warn_unused_result block/vmdk.c:790: error: ignoring return value of 'write', declared with attribute warn_unused_result block/vmdk.c:807: error: ignoring return value of 'write', declared with attribute warn_unused_result
make: *** [block/vmdk.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 block/vmdk.c |   50 ++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 40 insertions(+), 10 deletions(-)

block/vmdk.c +40 ~~-10~~

index ddc2fcb..56c28a0 100644
@@ -233,7 +233,8 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
     memset(&header, 0, sizeof(header));
     memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC
 
-    ftruncate(snp_fd, header.grain_offset << 9);
+    if (ftruncate(snp_fd, header.grain_offset << 9))
+        goto fail;
     /* the descriptor offset = 0x200 */
     if (lseek(p_fd, 0x200, SEEK_SET) == -1)
         goto fail;

@@ -717,6 +718,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     int64_t total_size = 0;
     const char *backing_file = NULL;
     int flags = 0;
+    int ret;
 
     // Read out options
     while (options && options->name) {

@@ -773,22 +775,44 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     header.check_bytes[3] = 0xa;
 
     /* write all the data */
-    write(fd, &magic, sizeof(magic));
-    write(fd, &header, sizeof(header));
+    ret = qemu_write_full(fd, &magic, sizeof(magic));
+    if (ret != sizeof(magic)) {
+        ret = -1;
+        goto exit;
+    }
+    ret = qemu_write_full(fd, &header, sizeof(header));
+    if (ret != sizeof(header)) {
+        ret = -1;
+        goto exit;
+    }
 
-    ftruncate(fd, header.grain_offset << 9);
+    ret = ftruncate(fd, header.grain_offset << 9);
+    if (ret < 0) {
+        ret = -1;
+        goto exit;
+    }
 
     /* write grain directory */
     lseek(fd, le64_to_cpu(header.rgd_offset) << 9, SEEK_SET);
     for (i = 0, tmp = header.rgd_offset + gd_size;
-         i < gt_count; i++, tmp += gt_size)
-        write(fd, &tmp, sizeof(tmp));
+         i < gt_count; i++, tmp += gt_size) {
+        ret = qemu_write_full(fd, &tmp, sizeof(tmp));
+        if (ret != sizeof(tmp)) {
+            ret = -1;
+            goto exit;
+        }
+    }
 
     /* write backup grain directory */
     lseek(fd, le64_to_cpu(header.gd_offset) << 9, SEEK_SET);
     for (i = 0, tmp = header.gd_offset + gd_size;
-         i < gt_count; i++, tmp += gt_size)
-        write(fd, &tmp, sizeof(tmp));
+         i < gt_count; i++, tmp += gt_size) {
+        ret = qemu_write_full(fd, &tmp, sizeof(tmp));
+        if (ret != sizeof(tmp)) {
+            ret = -1;
+            goto exit;
+        }
+    }
 
     /* compose the descriptor */
     real_filename = filename;

@@ -805,10 +829,16 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
 
     /* write the descriptor */
     lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET);
-    write(fd, desc, strlen(desc));
+    ret = qemu_write_full(fd, desc, strlen(desc));
+    if (ret != strlen(desc)) {
+        ret = -1;
+        goto exit;
+    }
 
+    ret = 0;
+exit:
     close(fd);
-    return 0;
+    return ret;
 }
 
 static void vmdk_close(BlockDriverState *bs)

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #7 lore raw

PATCH v2 7/17 block/vvfat.c: fix warnings with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC block/vvfat.o
cc1: warnings being treated as errors
block/vvfat.c: In function 'commit_one_file':
block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result
make: *** [block/vvfat.o] Error 1
CC block/vvfat.o
In file included from /usr/include/stdio.h:912,

                 from ./qemu-common.h:19,
                 from block/vvfat.c:27:

In function 'snprintf',
inlined from 'init_directories' at block/vvfat.c:871,
inlined from 'vvfat_open' at block/vvfat.c:1068:
/usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk will always overflow destination buffer
make: *** [block/vvfat.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 block/vvfat.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

block/vvfat.c +7 -2

index 063f731..df957e5 100644
@@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s,
     {
 	direntry_t* entry=array_get_next(&(s->directory));
 	entry->attributes=0x28; /* archive | volume label */
-	snprintf((char*)entry->name,11,"QEMU VVFAT");
+	memcpy(entry->name,"QEMU VVF",8);
+	memcpy(entry->extension,"AT ",3);
     }
 
     /* Now build FAT, and write back information into directory */

@@ -2256,7 +2257,11 @@ static int commit_one_file(BDRVVVFATState* s,
 	c = c1;
     }
 
-    ftruncate(fd, size);
+    if (ftruncate(fd, size)) {
+        perror("ftruncate()");
+        close(fd);
+        return -4;
+    }
     close(fd);
 
     return commit_mappings(s, first_cluster, dir_index);

Signature · 1 line

1.6.6

Gleb Natapov

2010-01-21 07:37 UTC #8 lore raw

Re: [Qemu-devel] [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE

Quoted 28 lines · Juan Quintela

From: Kirill A. Shutemov <[email protected]>
CC block/vvfat.o
cc1: warnings being treated as errors
block/vvfat.c: In function 'commit_one_file':
block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result
make: *** [block/vvfat.o] Error 1
CC block/vvfat.o
In file included from /usr/include/stdio.h:912,
                 from ./qemu-common.h:19,
                 from block/vvfat.c:27:
In function 'snprintf',
inlined from 'init_directories' at block/vvfat.c:871,
inlined from 'vvfat_open' at block/vvfat.c:1068:
/usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk will always overflow destination buffer
make: *** [block/vvfat.o] Error 1
Signed-off-by Kirill A. Shutemov <[email protected]>
Signed-off-by Juan Quintela <[email protected]>
---
 block/vvfat.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
block/vvfat.c +0 -0
index 063f731..df957e5 100644

block/vvfat.c static int init_directories(BDRVVVFATState* s, · line 874

Quoted 8 lines

block/vvfat.c +2 -1

@@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s,
     {
 	direntry_t* entry=array_get_next(&(s->directory));
 	entry->attributes=0x28; /* archive | volume label */
-	snprintf((char*)entry->name,11,"QEMU VVFAT");
+	memcpy(entry->name,"QEMU VVF",8);
+	memcpy(entry->extension,"AT ",3);
     }

Before the change extension was initialized to "AT\0" after it is "AT "

Quoted 18 lines

block/vvfat.c +5 -1

     /* Now build FAT, and write back information into directory */

@@ -2256,7 +2257,11 @@ static int commit_one_file(BDRVVVFATState* s,
 	c = c1;
     }
 
-    ftruncate(fd, size);
+    if (ftruncate(fd, size)) {
+        perror("ftruncate()");
+        close(fd);
+        return -4;
+    }
     close(fd);
 
     return commit_mappings(s, first_cluster, dir_index);

Signature · 1 line

1.6.6

--
Gleb.

Juan Quintela

author

2010-01-21 08:17 UTC #9 lore raw

[Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE

Quoted 24 lines · Gleb Natapov

Quoted 23 lines · Juan Quintela
From: Kirill A. Shutemov <[email protected]>
CC block/vvfat.o
cc1: warnings being treated as errors
block/vvfat.c: In function 'commit_one_file':
block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result
make: *** [block/vvfat.o] Error 1
CC block/vvfat.o
In file included from /usr/include/stdio.h:912,
                 from ./qemu-common.h:19,
                 from block/vvfat.c:27:
In function 'snprintf',
inlined from 'init_directories' at block/vvfat.c:871,
inlined from 'vvfat_open' at block/vvfat.c:1068:
/usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk will always overflow destination buffer
make: *** [block/vvfat.o] Error 1
Signed-off-by Kirill A. Shutemov <[email protected]>
Signed-off-by Juan Quintela <[email protected]>
---
 block/vvfat.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

Quoted 14 lines

block/vvfat.c static int init_directories(BDRVVVFATState* s, · line 874

Quoted 13 lines

block/vvfat.c +2 -1

index 063f731..df957e5 100644
@@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s,
     {
 	direntry_t* entry=array_get_next(&(s->directory));
 	entry->attributes=0x28; /* archive | volume label */
-	snprintf((char*)entry->name,11,"QEMU VVFAT");
+	memcpy(entry->name,"QEMU VVF",8);
+	memcpy(entry->extension,"AT ",3);
     }

Before the change extension was initialized to "AT\0" after it is "AT "

it was paolo who told to do that change. entries are not 0 ended.

that was his explanation.

Later, Juan.

Gleb Natapov

2010-01-21 08:19 UTC #10 lore raw

[Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE

Quoted 40 lines · Juan Quintela

Quoted 24 lines · Gleb Natapov
Quoted 23 lines · Juan Quintela
From: Kirill A. Shutemov <[email protected]>
CC block/vvfat.o
cc1: warnings being treated as errors
block/vvfat.c: In function 'commit_one_file':
block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result
make: *** [block/vvfat.o] Error 1
CC block/vvfat.o
In file included from /usr/include/stdio.h:912,
                 from ./qemu-common.h:19,
                 from block/vvfat.c:27:
In function 'snprintf',
inlined from 'init_directories' at block/vvfat.c:871,
inlined from 'vvfat_open' at block/vvfat.c:1068:
/usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk will always overflow destination buffer
make: *** [block/vvfat.o] Error 1
Signed-off-by Kirill A. Shutemov <[email protected]>
Signed-off-by Juan Quintela <[email protected]>
---
 block/vvfat.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
Quoted 14 lines
block/vvfat.c static int init_directories(BDRVVVFATState* s, · line 874
Quoted 13 lines
block/vvfat.c +2 -1
index 063f731..df957e5 100644
@@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s,
     {
 	direntry_t* entry=array_get_next(&(s->directory));
 	entry->attributes=0x28; /* archive | volume label */
-	snprintf((char*)entry->name,11,"QEMU VVFAT");
+	memcpy(entry->name,"QEMU VVF",8);
+	memcpy(entry->extension,"AT ",3);
     }
 
Before the change extension was initialized to "AT\0" after it is "AT "

Quoted 4 lines

it was paolo who told to do that change. entries are not 0 ended.
that was his explanation.

OK. Just wanted to make sure this is intentional.

--
Gleb.

Markus Armbruster

2010-01-21 09:28 UTC #11 lore raw

Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE

In reply to Juan Quintela

Quoted 2 lines · Juan Quintela

Quoted 1 line · Gleb Natapov
On Wed, Jan 20, 2010 at 09:14:03PM +0100, Juan Quintela wrote:

[...]

Quoted 15 lines

Quoted 6 lines
Quoted 6 lines
block/vvfat.c +0 -0
index 063f731..df957e5 100644
@@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s,
     {
Quoted 8 lines
block/vvfat.c static int init_directories(BDRVVVFATState* s, · line 874
Quoted 7 lines
block/vvfat.c +2 -1
 	direntry_t* entry=array_get_next(&(s->directory));
 	entry->attributes=0x28; /* archive | volume label */
-	snprintf((char*)entry->name,11,"QEMU VVFAT");
+	memcpy(entry->name,"QEMU VVF",8);
+	memcpy(entry->extension,"AT ",3);
     }
 
Before the change extension was initialized to "AT\0" after it is "AT "

Quoted 3 lines

it was paolo who told to do that change. entries are not 0 ended.
that was his explanation.

Please mention this in the commit message.

Juan Quintela

author

2010-01-20 20:14 UTC #12 lore raw

PATCH v2 8/17 block/qcow2.c: fix warnings with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC block/qcow2.o
cc1: warnings being treated as errors
block/qcow2.c: In function 'qcow_create2':
block/qcow2.c:829: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:838: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:839: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:841: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:844: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:849: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:852: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:855: error: ignoring return value of 'write', declared with attribute warn_unused_result
make: *** [block/qcow2.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 block/qcow2.c |   55 +++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 45 insertions(+), 10 deletions(-)

block/qcow2.c +45 ~~-10~~

index 6622eba..1bf94c5 100644
@@ -842,7 +842,7 @@ static int qcow_create2(const char *filename, int64_t total_size,
     uint64_t tmp, offset;
     QCowCreateState s1, *s = &s1;
     QCowExtension ext_bf = {0, 0};
-
+    int ret;
 
     memset(s, 0, sizeof(*s));

@@ -925,7 +925,11 @@ static int qcow_create2(const char *filename, int64_t total_size,
         ref_clusters * s->cluster_size);
 
     /* write all the data */
-    write(fd, &header, sizeof(header));
+    ret = qemu_write_full(fd, &header, sizeof(header));
+    if (ret != sizeof(header)) {
+        ret = -1;
+        goto exit;
+    }
     if (backing_file) {
         if (backing_format_len) {
             char zero[16];

@@ -934,25 +938,56 @@ static int qcow_create2(const char *filename, int64_t total_size,
             memset(zero, 0, sizeof(zero));
             cpu_to_be32s(&ext_bf.magic);
             cpu_to_be32s(&ext_bf.len);
-            write(fd, &ext_bf, sizeof(ext_bf));
-            write(fd, backing_format, backing_format_len);
+            ret = qemu_write_full(fd, &ext_bf, sizeof(ext_bf));
+            if (ret != sizeof(ext_bf)) {
+                ret = -1;
+                goto exit;
+            }
+            ret = qemu_write_full(fd, backing_format, backing_format_len);
+            if (ret != backing_format_len) {
+                ret = -1;
+                goto exit;
+            }
             if (padding > 0) {
-                write(fd, zero, padding);
+                ret = qemu_write_full(fd, zero, padding);
+                if (ret != padding) {
+                    ret = -1;
+                    goto exit;
+                }
             }
         }
-        write(fd, backing_file, backing_filename_len);
+        ret = qemu_write_full(fd, backing_file, backing_filename_len);
+        if (ret != backing_filename_len) {
+            ret = -1;
+            goto exit;
+        }
     }
     lseek(fd, s->l1_table_offset, SEEK_SET);
     tmp = 0;
     for(i = 0;i < l1_size; i++) {
-        write(fd, &tmp, sizeof(tmp));
+        ret = qemu_write_full(fd, &tmp, sizeof(tmp));
+        if (ret != sizeof(tmp)) {
+            ret = -1;
+            goto exit;
+        }
     }
     lseek(fd, s->refcount_table_offset, SEEK_SET);
-    write(fd, s->refcount_table, s->cluster_size);
+    ret = qemu_write_full(fd, s->refcount_table, s->cluster_size);
+    if (ret != s->cluster_size) {
+        ret = -1;
+        goto exit;
+    }
 
     lseek(fd, s->refcount_block_offset, SEEK_SET);
-    write(fd, s->refcount_block, ref_clusters * s->cluster_size);
+    ret = qemu_write_full(fd, s->refcount_block,
+		    ref_clusters * s->cluster_size);
+    if (ret != s->cluster_size) {
+        ret = -1;
+        goto exit;
+    }
 
+    ret = 0;
+exit:
     qemu_free(s->refcount_table);
     qemu_free(s->refcount_block);
     close(fd);

@@ -966,7 +1001,7 @@ static int qcow_create2(const char *filename, int64_t total_size,
         bdrv_close(bs);
     }
 
-    return 0;
+    return ret;
 }
 
 static int qcow_create(const char *filename, QEMUOptionParameter *options)

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #13 lore raw

PATCH v2 9/17 net/slirp.c: fix warning with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC net/slirp.o
cc1: warnings being treated as errors
net/slirp.c: In function 'slirp_smb_cleanup':
net/slirp.c:470: error: ignoring return value of 'system', declared with attribute warn_unused_result
make: *** [net/slirp.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 net/slirp.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

net/slirp.c +8 -1

index 3f91c4b..b75ad16 100644
@@ -464,10 +464,17 @@ int net_slirp_redir(const char *redir_str)
 static void slirp_smb_cleanup(SlirpState *s)
 {
     char cmd[128];
+    int ret;
 
     if (s->smb_dir[0] != '\0') {
         snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
-        system(cmd);
+        ret = system(cmd);
+        if (!WIFEXITED(ret)) {
+            qemu_error("'%s' failed.\n", cmd);
+        } else if (WEXITSTATUS(ret)) {
+            qemu_error("'%s' failed. Error code: %d\n",
+                    cmd, WEXITSTATUS(ret));
+        }
         s->smb_dir[0] = '\0';
     }
 }

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #14 lore raw

PATCH v2 10/17 usb-linux.c: fix warning with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC usb-linux.o
cc1: warnings being treated as errors
usb-linux.c: In function 'usb_host_read_file':
usb-linux.c:1204: error: ignoring return value of 'fgets', declared with attribute warn_unused_result
make: *** [usb-linux.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 usb-linux.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

usb-linux.c +1 -2

index 5619b30..1aaa595 100644
@@ -1201,9 +1201,8 @@ static int usb_host_read_file(char *line, size_t line_size, const char *device_f
              device_file);
     f = fopen(filename, "r");
     if (f) {
-        fgets(line, line_size, f);
+        ret = fgets(line, line_size, f) != NULL;
         fclose(f);
-        ret = 1;
 #if 0
     } else {
         if (mon)

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #15 lore raw

PATCH v2 11/17 vl.c: fix warning with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC i386-softmmu/vl.o
cc1: warnings being treated as errors
/usr/src/RPM/BUILD/qemu-0.11.92/vl.c: In function 'qemu_event_increment':
/usr/src/RPM/BUILD/qemu-0.11.92/vl.c:3404: error: ignoring return value of 'write', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/vl.c: In function 'main':
/usr/src/RPM/BUILD/qemu-0.11.92/vl.c:5774: error: ignoring return value of 'write', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:6064: error: ignoring return value of 'chdir', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:6083: error: ignoring return value of 'chdir', declared with attribute warn_unused_result make[1]: *** [vl.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 vl.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

vl.c +18 -4

index e070ec9..9c74038 100644
@@ -3185,11 +3185,17 @@ static int io_thread_fd = -1;
 static void qemu_event_increment(void)
 {
     static const char byte = 0;
+    ssize_t ret;
 
     if (io_thread_fd == -1)
         return;
 
-    write(io_thread_fd, &byte, sizeof(byte));
+    ret = write(io_thread_fd, &byte, sizeof(byte));
+    if (ret < 0 && (errno != EINTR && errno != EAGAIN)) {
+        fprintf(stderr, "qemu_event_increment: write() filed: %s\n",
+                strerror(errno));
+        exit (1);
+    }
 }
 
 static void qemu_event_read(void *opaque)

@@ -5594,7 +5600,9 @@ int main(int argc, char **argv, char **envp)
 #ifndef _WIN32
         if (daemonize) {
             uint8_t status = 1;
-            write(fds[1], &status, 1);
+            if (write(fds[1], &status, 1) != 1) {
+                perror("daemonize. Writing to pipe\n");
+            }
         } else
 #endif
             fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));

@@ -5893,7 +5901,10 @@ int main(int argc, char **argv, char **envp)
 	if (len != 1)
 	    exit(1);
 
-	chdir("/");
+        if (chdir("/")) {
+            perror("not able to chdir to /");
+            exit(1);
+        }
 	TFR(fd = qemu_open("/dev/null", O_RDWR));
 	if (fd == -1)
 	    exit(1);

@@ -5912,7 +5923,10 @@ int main(int argc, char **argv, char **envp)
             fprintf(stderr, "chroot failed\n");
             exit(1);
         }
-        chdir("/");
+        if (chdir("/")) {
+            perror("not able to chdir to /");
+            exit(1);
+        }
     }
 
     if (run_as) {

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #16 lore raw

PATCH v2 12/17 monitor.c: fix warnings with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC i386-softmmu/monitor.o
cc1: warnings being treated as errors
/usr/src/RPM/BUILD/qemu-0.11.92/monitor.c: In function 'do_memory_save':
/usr/src/RPM/BUILD/qemu-0.11.92/monitor.c:1318: error: ignoring return value of 'fwrite', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/monitor.c: In function 'do_physical_memory_save':
/usr/src/RPM/BUILD/qemu-0.11.92/monitor.c:1345: error: ignoring return value of 'fwrite', declared with attribute warn_unused_result make[1]: *** [monitor.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 monitor.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

monitor.c +10 -2

index cadf422..7fc9973 100644
@@ -1330,10 +1330,14 @@ static void do_memory_save(Monitor *mon, const QDict *qdict, QObject **ret_data)
         if (l > size)
             l = size;
         cpu_memory_rw_debug(env, addr, buf, l, 0);
-        fwrite(buf, 1, l, f);
+        if (fwrite(buf, 1, l, f) != l) {
+            monitor_printf(mon, "fwrite() error in do_memory_save\n");
+            goto exit;
+        }
         addr += l;
         size -= l;
     }
+exit:
     fclose(f);
 }

@@ -1357,11 +1361,15 @@ static void do_physical_memory_save(Monitor *mon, const QDict *qdict,
         if (l > size)
             l = size;
         cpu_physical_memory_rw(addr, buf, l, 0);
-        fwrite(buf, 1, l, f);
+        if (fwrite(buf, 1, l, f) != l) {
+            monitor_printf(mon, "fwrite() error in do_physical_memory_save\n");
+            goto exit;
+        }
         fflush(f);
         addr += l;
         size -= l;
     }
+exit:
     fclose(f);
 }

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #17 lore raw

PATCH v2 13/17 linux-user/mmap.c: fix warnings with _FORTIFY_SOURCE

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

CC i386-linux-user/mmap.o
cc1: warnings being treated as errors
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c: In function 'mmap_frag':
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c:253: error: ignoring return value of 'pread', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c: In function 'target_mmap':
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c:477: error: ignoring return value of 'pread', declared with attribute warn_unused_result make[1]: *** [mmap.o] Error 1

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 linux-user/mmap.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

linux-user/mmap.c +4 -2

index 144fb7c..c1c7e48 100644
@@ -250,7 +250,8 @@ static int mmap_frag(abi_ulong real_start,
             mprotect(host_start, qemu_host_page_size, prot1 | PROT_WRITE);
 
         /* read the corresponding file data */
-        pread(fd, g2h(start), end - start, offset);
+        if (pread(fd, g2h(start), end - start, offset) == -1)
+            return -1;
 
         /* put final protection */
         if (prot_new != (prot1 | PROT_WRITE))

@@ -474,7 +475,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                                   -1, 0);
             if (retaddr == -1)
                 goto fail;
-            pread(fd, g2h(start), len, offset);
+            if (pread(fd, g2h(start), len, offset) == -1)
+                goto fail;
             if (!(prot & PROT_WRITE)) {
                 ret = target_mprotect(start, len, prot);
                 if (ret != 0) {

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #18 lore raw

PATCH v2 14/17 check pipe() return value

In reply to Juan Quintela

Signed-off-by Juan Quintela <[email protected]>

---
 hw/xen_domainbuild.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

hw/xen_domainbuild.c +9 -4

index 20d731d..2f59856 100644
@@ -156,15 +156,18 @@ quit:
     return;
 }
 
-static void xen_domain_watcher(void)
+static int xen_domain_watcher(void)
 {
     int qemu_running = 1;
     int fd[2], i, n, rc;
     char byte;
 
-    pipe(fd);
+    if (pipe(fd) != 0) {
+        qemu_log("%s: Huh? pipe error: %s\n", __FUNCTION__, strerror(errno));
+        return -1;
+    }
     if (fork() != 0)
-        return; /* not child */
+        return 0; /* not child */
 
     /* close all file handles, except stdio/out/err,
      * our watch pipe and the xen interface handle */

@@ -238,7 +241,9 @@ int xen_domain_build_pv(const char *kernel, const char *ramdisk,
     }
     qemu_log("xen: created domain %d\n", xen_domid);
     atexit(xen_domain_cleanup);
-    xen_domain_watcher();
+    if (xen_domain_watcher() == -1) {
+        goto err;
+    }
 
     xenstore_domain_init1(kernel, ramdisk, cmdline);

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #19 lore raw

PATCH v2 15/17 Enable _FORTIFY_SOURCE=2

In reply to Juan Quintela

From: Kirill A. Shutemov <[email protected]>

_FORTIFY_SOURCE is a Glibc feature which adds memory and string function protection.

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Juan Quintela <[email protected]>

---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

configure +1 -1

index 5631bbb..5556b9d 100755
@@ -97,7 +97,7 @@ CFLAGS="-g $CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
-QEMU_CFLAGS="-U_FORTIFY_SOURCE $QEMU_CFLAGS"
+QEMU_CFLAGS="-D_FORTIFY_SOURCE=2 $QEMU_CFLAGS"
 QEMU_CFLAGS="-I. -I\$(SRC_PATH) $QEMU_CFLAGS"
 LDFLAGS="-g $LDFLAGS"

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #20 lore raw

PATCH v2 16/17 Check availavility of -fstack-protector-all

In reply to Juan Quintela

Signed-off-by Juan Quintela <[email protected]>

---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

configure +1 -1

index 5556b9d..d8af978 100755
@@ -101,7 +101,7 @@ QEMU_CFLAGS="-D_FORTIFY_SOURCE=2 $QEMU_CFLAGS"
 QEMU_CFLAGS="-I. -I\$(SRC_PATH) $QEMU_CFLAGS"
 LDFLAGS="-g $LDFLAGS"
 
-gcc_flags="-Wold-style-declaration -Wold-style-definition"
+gcc_flags="-Wold-style-declaration -Wold-style-definition -fstack-protector-all"
 cat > $TMPC << EOF
 int main(void) { }
 EOF

Signature · 1 line

1.6.6

Juan Quintela

author

2010-01-20 20:14 UTC #21 lore raw

PATCH v2 17/17 mmap_frag() users only check for -1 error

In reply to Juan Quintela

Signed-off-by Juan Quintela <[email protected]>

---
 linux-user/mmap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

linux-user/mmap.c +1 -1

index c1c7e48..25fc0b2 100644
@@ -243,7 +243,7 @@ static int mmap_frag(abi_ulong real_start,
            possible while it is a shared mapping */
         if ((flags & MAP_TYPE) == MAP_SHARED &&
             (prot & PROT_WRITE))
-            return -EINVAL;
+            return -1;
 
         /* adjust protection to be able to read */
         if (!(prot1 & PROT_WRITE))

Signature · 1 line

1.6.6

Laurent Desnogues

2010-01-25 08:53 UTC #22 lore raw

Re: [Qemu-devel] [PATCH 17/17] mmap_frag() users only check for -1 error

Quoted 16 lines · Juan Quintela

Signed-off-by Juan Quintela <[email protected]>
---
linux-user/mmap.c | 2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
linux-user/mmap.c +0 -0
index c1c7e48..25fc0b2 100644
@@ -243,7 +243,7 @@ static int mmap_frag(abi_ulong real_start,
possible while it is a shared mapping */ if ((flags & MAP_TYPE) == MAP_SHARED && (prot & PROT_WRITE))
- return -EINVAL;
+ return -1;

Quoted 2 lines

/* adjust protection to be able to read */ if (!(prot1 & PROT_WRITE))

Looks good to me, but a similar patch is needed for bsd-user/mmap.c and darwin-user/mmap.c.

Laurent