block: Prevent coroutine stack overflow when recursing in bdrv_open_backing_file.

In 1.7.1 qcow2_create2 reopen the file for flushing without the BDRV_O_NO_BACKING
flags.

As a consequence the code would recursively open the whole backing chain.

These three stack arrays would pile up through the recursion and lead to a coroutine
stack overflow.

Convert these array to malloced buffers in order to streamline the coroutine
footprint.

Symptoms where freezes or segfaults on production machines while taking QMP externals
snapshots. The overflow disturbed coroutine switching.

Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
*note: backport of upstream's 1ba4b6a

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

diff --git a/block.c b/block.c
index 382ea71f4b8c28550d4a798377fd1e6c4bae04ae..8f84dbc5cb3ad4196f7312c77caec8cd0b13d82c 100644 (file)
--- a/block.c
+++ b/block.c
@@ -966,14 +966,14 @@ fail:
  */
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
 {
-    char backing_filename[PATH_MAX];
-    int back_flags, ret;
+    char *backing_filename = g_malloc0(PATH_MAX);
+    int back_flags, ret = 0;
     BlockDriver *back_drv = NULL;
     Error *local_err = NULL;
 
     if (bs->backing_hd != NULL) {
         QDECREF(options);
-        return 0;
+        goto free_exit;
     }
 
     /* NULL means an empty set of options */
@@ -986,10 +986,9 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
         backing_filename[0] = '\0';
     } else if (bs->backing_file[0] == '\0' && qdict_size(options) == 0) {
         QDECREF(options);
-        return 0;
+        goto free_exit;
     } else {
-        bdrv_get_full_backing_filename(bs, backing_filename,
-                                       sizeof(backing_filename));
+        bdrv_get_full_backing_filename(bs, backing_filename, PATH_MAX);
     }
 
     bs->backing_hd = bdrv_new("");
@@ -1012,11 +1011,14 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
         error_setg(errp, "Could not open backing file: %s",
                    error_get_pretty(local_err));
         error_free(local_err);
-        return ret;
+        goto free_exit;
     }
     pstrcpy(bs->backing_file, sizeof(bs->backing_file),
             bs->backing_hd->file->filename);
-    return 0;
+    ret = 0;
+free_exit:
+    g_free(backing_filename);
+    return ret;
 }
 
 /*
@@ -1032,7 +1034,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
 {
     int ret;
     /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
-    char tmp_filename[PATH_MAX + 1];
+    char *backing_filename = NULL;
+    char *tmp_filename = g_malloc0(PATH_MAX + 1);
     BlockDriverState *file = NULL;
     QDict *file_options = NULL;
     const char *drvname;
@@ -1052,7 +1055,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         int64_t total_size;
         BlockDriver *bdrv_qcow2;
         QEMUOptionParameter *create_options;
-        char backing_filename[PATH_MAX];
+        backing_filename = g_malloc0(PATH_MAX);
 
         if (qdict_size(options) != 0) {
             error_setg(errp, "Can't use snapshot=on with driver-specific options");
@@ -1075,7 +1078,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
 
         bdrv_unref(bs1);
 
-        ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
+        ret = get_tmp_filename(tmp_filename, PATH_MAX + 1);
         if (ret < 0) {
             error_setg_errno(errp, -ret, "Could not get temporary filename");
             goto fail;
@@ -1083,8 +1086,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
 
         /* Real path is meaningless for protocols */
         if (path_has_protocol(filename)) {
-            snprintf(backing_filename, sizeof(backing_filename),
-                     "%s", filename);
+            snprintf(backing_filename, PATH_MAX, "%s", filename);
         } else if (!realpath(filename, backing_filename)) {
             ret = -errno;
             error_setg_errno(errp, errno, "Could not resolve path '%s'", filename);
@@ -1206,6 +1208,8 @@ fail:
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
     }
+    g_free(tmp_filename);
+    g_free(backing_filename);
     return ret;
 
 close_and_fail:
@@ -1214,6 +1218,8 @@ close_and_fail:
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
     }
+    g_free(tmp_filename);
+    g_free(backing_filename);
     return ret;
 }