Commit 6db87aae authored by Peter Maydell's avatar Peter Maydell
Browse files

Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 



Block layer patches:

- file-posix: Don't waste a file descriptor for locking, don't lock the
  same bit multiple times
- nvme: Fix double free and memory leak
- Misc error handling fixes
- Added NULL checks found by static analysis
- Allow more block drivers to not be included in the qemu build

# gpg: Signature made Mon 12 Nov 2018 17:05:00 GMT
# gpg:                using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* remotes/kevin/tags/for-upstream:
  qcow2: Read outside array bounds in qcow2_pre_write_overlap_check()
  block: Fix potential Null pointer dereferences in vvfat.c
  qemu-img: assert block_job_get() does not return NULL in img_commit()
  block: Null pointer dereference in blk_root_get_parent_desc()
  job: Fix off-by-one assert checks for JobSTT and JobVerbTable
  block: Make more block drivers compile-time configurable
  tests: Add unit tests for image locking
  file-posix: Drop s->lock_fd
  file-posix: Skip effectiveless OFD lock operations
  nvme: free cmbuf in nvme_exit
  nvme: don't unref ctrl_mem when device unrealized
  blockdev: Consistently use snapshot_node_name in external_snapshot_prepare()
  blockdev: handle error on block latency histogram set error
  file-posix: Use error API properly

Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
parents 5704c36d 1a42e5d8

block/Makefile.objs

+16 −6
Original line number Diff line number Diff line 
block-obj-y += raw-format.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.o

block-obj-y += raw-format.o vmdk.o vpc.o

block-obj-$(CONFIG_QCOW1) += qcow.o

block-obj-$(CONFIG_VDI) += vdi.o

block-obj-$(CONFIG_CLOOP) += cloop.o

block-obj-$(CONFIG_BOCHS) += bochs.o

block-obj-$(CONFIG_VVFAT) += vvfat.o

block-obj-$(CONFIG_DMG) += dmg.o



block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-bitmap.o

block-obj-y += qed.o qed-l2-cache.o qed-table.o qed-cluster.o

block-obj-y += qed-check.o

block-obj-$(CONFIG_QED) += qed.o qed-l2-cache.o qed-table.o qed-cluster.o

block-obj-$(CONFIG_QED) += qed-check.o

block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o

block-obj-y += quorum.o

block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o

block-obj-y += blkdebug.o blkverify.o blkreplay.o

block-obj-$(CONFIG_PARALLELS) += parallels.o

block-obj-y += blklogwrites.o

block-obj-y += block-backend.o snapshot.o qapi.o

block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
@@ -14,7 +22,8 @@ block-obj-y += null.o mirror.o commit.o io.o create.o 
block-obj-y += throttle-groups.o

block-obj-$(CONFIG_LINUX) += nvme.o



block-obj-y += nbd.o nbd-client.o sheepdog.o

block-obj-y += nbd.o nbd-client.o

block-obj-$(CONFIG_SHEEPDOG) += sheepdog.o

block-obj-$(CONFIG_LIBISCSI) += iscsi.o

block-obj-$(if $(CONFIG_LIBISCSI),y,n) += iscsi-opts.o

block-obj-$(CONFIG_LIBNFS) += nfs.o
@@ -45,7 +54,8 @@ gluster.o-libs := $(GLUSTERFS_LIBS) 
vxhs.o-libs        := $(VXHS_LIBS)

ssh.o-cflags       := $(LIBSSH2_CFLAGS)

ssh.o-libs         := $(LIBSSH2_LIBS)

block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o

block-obj-dmg-bz2-$(CONFIG_BZIP2) += dmg-bz2.o

block-obj-$(if $(CONFIG_DMG),m,n) += $(block-obj-dmg-bz2-y)

dmg-bz2.o-libs     := $(BZIP2_LIBS)

qcow.o-libs        := -lz

linux-aio.o-libs   := -laio

block/block-backend.c

+2 −1
Original line number Diff line number Diff line
@@ -918,7 +918,8 @@ char *blk_get_attached_dev_id(BlockBackend *blk) 
    } else if (dev->id) {

        return g_strdup(dev->id);

    }

    return object_get_canonical_path(OBJECT(dev));



    return object_get_canonical_path(OBJECT(dev)) ?: g_strdup("");

}



/*

block/file-posix.c

+69 −53
Original line number Diff line number Diff line
@@ -142,7 +142,6 @@ do { \ 


typedef struct BDRVRawState {

    int fd;

    int lock_fd;

    bool use_lock;

    int type;

    int open_flags;
@@ -152,6 +151,11 @@ typedef struct BDRVRawState { 
    uint64_t perm;

    uint64_t shared_perm;



    /* The perms bits whose corresponding bytes are already locked in

     * s->fd. */

    uint64_t locked_perm;

    uint64_t locked_shared_perm;



#ifdef CONFIG_XFS

    bool is_xfs:1;

#endif
@@ -205,7 +209,7 @@ static int cdrom_reopen(BlockDriverState *bs); 
#endif



#if defined(__NetBSD__)

static int raw_normalize_devicepath(const char **filename)

static int raw_normalize_devicepath(const char **filename, Error **errp)

{

    static char namebuf[PATH_MAX];

    const char *dp, *fname;
@@ -214,8 +218,7 @@ static int raw_normalize_devicepath(const char **filename) 
    fname = *filename;

    dp = strrchr(fname, '/');

    if (lstat(fname, &sb) < 0) {

        fprintf(stderr, "%s: stat failed: %s\n",

            fname, strerror(errno));

        error_setg_errno(errp, errno, "%s: stat failed", fname);

        return -errno;

    }
@@ -229,14 +232,13 @@ static int raw_normalize_devicepath(const char **filename) 
        snprintf(namebuf, PATH_MAX, "%.*s/r%s",

            (int)(dp - fname), fname, dp + 1);

    }

    fprintf(stderr, "%s is a block device", fname);

    *filename = namebuf;

    fprintf(stderr, ", using %s\n", *filename);

    warn_report("%s is a block device, using %s", fname, *filename);



    return 0;

}

#else

static int raw_normalize_devicepath(const char **filename)

static int raw_normalize_devicepath(const char **filename, Error **errp)

{

    return 0;

}
@@ -461,9 +463,8 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, 


    filename = qemu_opt_get(opts, "filename");



    ret = raw_normalize_devicepath(&filename);

    ret = raw_normalize_devicepath(&filename, errp);

    if (ret != 0) {

        error_setg_errno(errp, -ret, "Could not normalize device path");

        goto fail;

    }
@@ -492,10 +493,9 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, 
    case ON_OFF_AUTO_ON:

        s->use_lock = true;

        if (!qemu_has_ofd_lock()) {

            fprintf(stderr,

                    "File lock requested but OFD locking syscall is "

                    "unavailable, falling back to POSIX file locks.\n"

                    "Due to the implementation, locks can be lost "

            warn_report("File lock requested but OFD locking syscall is "

                        "unavailable, falling back to POSIX file locks");

            error_printf("Due to the implementation, locks can be lost "

                         "unexpectedly.\n");

        }

        break;
@@ -550,18 +550,6 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, 
    }

    s->fd = fd;



    s->lock_fd = -1;

    if (s->use_lock) {

        fd = qemu_open(filename, s->open_flags);

        if (fd < 0) {

            ret = -errno;

            error_setg_errno(errp, errno, "Could not open '%s' for locking",

                             filename);

            qemu_close(s->fd);

            goto fail;

        }

        s->lock_fd = fd;

    }

    s->perm = 0;

    s->shared_perm = BLK_PERM_ALL;
@@ -693,43 +681,72 @@ typedef enum { 
 * file; if @unlock == true, also unlock the unneeded bytes.

 * @shared_perm_lock_bits is the mask of all permissions that are NOT shared.

 */

static int raw_apply_lock_bytes(int fd,

static int raw_apply_lock_bytes(BDRVRawState *s, int fd,

                                uint64_t perm_lock_bits,

                                uint64_t shared_perm_lock_bits,

                                bool unlock, Error **errp)

{

    int ret;

    int i;

    uint64_t locked_perm, locked_shared_perm;



    if (s) {

        locked_perm = s->locked_perm;

        locked_shared_perm = s->locked_shared_perm;

    } else {

        /*

         * We don't have the previous bits, just lock/unlock for each of the

         * requested bits.

         */

        if (unlock) {

            locked_perm = BLK_PERM_ALL;

            locked_shared_perm = BLK_PERM_ALL;

        } else {

            locked_perm = 0;

            locked_shared_perm = 0;

        }

    }



    PERM_FOREACH(i) {

        int off = RAW_LOCK_PERM_BASE + i;

        if (perm_lock_bits & (1ULL << i)) {

        uint64_t bit = (1ULL << i);

        if ((perm_lock_bits & bit) && !(locked_perm & bit)) {

            ret = qemu_lock_fd(fd, off, 1, false);

            if (ret) {

                error_setg(errp, "Failed to lock byte %d", off);

                return ret;

            } else if (s) {

                s->locked_perm |= bit;

            }

        } else if (unlock) {

        } else if (unlock && (locked_perm & bit) && !(perm_lock_bits & bit)) {

            ret = qemu_unlock_fd(fd, off, 1);

            if (ret) {

                error_setg(errp, "Failed to unlock byte %d", off);

                return ret;

            } else if (s) {

                s->locked_perm &= ~bit;

            }

        }

    }

    PERM_FOREACH(i) {

        int off = RAW_LOCK_SHARED_BASE + i;

        if (shared_perm_lock_bits & (1ULL << i)) {

        uint64_t bit = (1ULL << i);

        if ((shared_perm_lock_bits & bit) && !(locked_shared_perm & bit)) {

            ret = qemu_lock_fd(fd, off, 1, false);

            if (ret) {

                error_setg(errp, "Failed to lock byte %d", off);

                return ret;

            } else if (s) {

                s->locked_shared_perm |= bit;

            }

        } else if (unlock) {

        } else if (unlock && (locked_shared_perm & bit) &&

                   !(shared_perm_lock_bits & bit)) {

            ret = qemu_unlock_fd(fd, off, 1);

            if (ret) {

                error_setg(errp, "Failed to unlock byte %d", off);

                return ret;

            } else if (s) {

                s->locked_shared_perm &= ~bit;

            }

        }

    }
@@ -793,15 +810,13 @@ static int raw_handle_perm_lock(BlockDriverState *bs, 
        return 0;

    }



    assert(s->lock_fd > 0);



    switch (op) {

    case RAW_PL_PREPARE:

        ret = raw_apply_lock_bytes(s->lock_fd, s->perm | new_perm,

        ret = raw_apply_lock_bytes(s, s->fd, s->perm | new_perm,

                                   ~s->shared_perm | ~new_shared,

                                   false, errp);

        if (!ret) {

            ret = raw_check_lock_bytes(s->lock_fd, new_perm, new_shared, errp);

            ret = raw_check_lock_bytes(s->fd, new_perm, new_shared, errp);

            if (!ret) {

                return 0;

            }
@@ -812,23 +827,23 @@ static int raw_handle_perm_lock(BlockDriverState *bs, 
        op = RAW_PL_ABORT;

        /* fall through to unlock bytes. */

    case RAW_PL_ABORT:

        raw_apply_lock_bytes(s->lock_fd, s->perm, ~s->shared_perm,

        raw_apply_lock_bytes(s, s->fd, s->perm, ~s->shared_perm,

                             true, &local_err);

        if (local_err) {

            /* Theoretically the above call only unlocks bytes and it cannot

             * fail. Something weird happened, report it.

             */

            error_report_err(local_err);

            warn_report_err(local_err);

        }

        break;

    case RAW_PL_COMMIT:

        raw_apply_lock_bytes(s->lock_fd, new_perm, ~new_shared,

        raw_apply_lock_bytes(s, s->fd, new_perm, ~new_shared,

                             true, &local_err);

        if (local_err) {

            /* Theoretically the above call only unlocks bytes and it cannot

             * fail. Something weird happened, report it.

             */

            error_report_err(local_err);

            warn_report_err(local_err);

        }

        break;

    }
@@ -905,10 +920,8 @@ static int raw_reopen_prepare(BDRVReopenState *state, 
    /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */

    if (rs->fd == -1) {

        const char *normalized_filename = state->bs->filename;

        ret = raw_normalize_devicepath(&normalized_filename);

        if (ret < 0) {

            error_setg_errno(errp, -ret, "Could not normalize device path");

        } else {

        ret = raw_normalize_devicepath(&normalized_filename, errp);

        if (ret >= 0) {

            assert(!(rs->open_flags & O_CREAT));

            rs->fd = qemu_open(normalized_filename, rs->open_flags);

            if (rs->fd == -1) {
@@ -939,10 +952,18 @@ static void raw_reopen_commit(BDRVReopenState *state) 
{

    BDRVRawReopenState *rs = state->opaque;

    BDRVRawState *s = state->bs->opaque;

    Error *local_err = NULL;



    s->check_cache_dropped = rs->check_cache_dropped;

    s->open_flags = rs->open_flags;



    /* Copy locks to the new fd before closing the old one. */

    raw_apply_lock_bytes(NULL, rs->fd, s->locked_perm,

                         ~s->locked_shared_perm, false, &local_err);

    if (local_err) {

        /* shouldn't fail in a sane host, but report it just in case. */

        error_report_err(local_err);

    }

    qemu_close(s->fd);

    s->fd = rs->fd;
@@ -1788,7 +1809,7 @@ static int aio_worker(void *arg) 
        ret = handle_aiocb_truncate(aiocb);

        break;

    default:

        fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type);

        error_report("invalid aio request (0x%x)", aiocb->aio_type);

        ret = -EINVAL;

        break;

    }
@@ -1935,10 +1956,6 @@ static void raw_close(BlockDriverState *bs) 
        qemu_close(s->fd);

        s->fd = -1;

    }

    if (s->lock_fd >= 0) {

        qemu_close(s->lock_fd);

        s->lock_fd = -1;

    }

}



/**
@@ -2226,7 +2243,7 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp) 
    shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;



    /* Step one: Take locks */

    result = raw_apply_lock_bytes(fd, perm, ~shared, false, errp);

    result = raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp);

    if (result < 0) {

        goto out_close;

    }
@@ -2270,13 +2287,13 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp) 
    }



out_unlock:

    raw_apply_lock_bytes(fd, 0, 0, true, &local_err);

    raw_apply_lock_bytes(NULL, fd, 0, 0, true, &local_err);

    if (local_err) {

        /* The above call should not fail, and if it does, that does

         * not mean the whole creation operation has failed.  So

         * report it the user for their convenience, but do not report

         * it to the caller. */

        error_report_err(local_err);

        warn_report_err(local_err);

    }



out_close:
@@ -3141,9 +3158,8 @@ static int coroutine_fn hdev_co_create_opts(const char *filename, QemuOpts *opts 


    (void)has_prefix;



    ret = raw_normalize_devicepath(&filename);

    ret = raw_normalize_devicepath(&filename, errp);

    if (ret < 0) {

        error_setg_errno(errp, -ret, "Could not normalize device path");

        return ret;

    }

block/qcow2-refcount.c

+10 −8
Original line number Diff line number Diff line
@@ -2727,7 +2727,9 @@ static const char *metadata_ol_names[] = { 
    [QCOW2_OL_SNAPSHOT_TABLE_BITNR]     = "snapshot table",

    [QCOW2_OL_INACTIVE_L1_BITNR]        = "inactive L1 table",

    [QCOW2_OL_INACTIVE_L2_BITNR]        = "inactive L2 table",

    [QCOW2_OL_BITMAP_DIRECTORY_BITNR]   = "bitmap directory",

};

QEMU_BUILD_BUG_ON(QCOW2_OL_MAX_BITNR != ARRAY_SIZE(metadata_ol_names));



/*

 * First performs a check for metadata overlaps (through

block/vvfat.c

+30 −16
Original line number Diff line number Diff line
@@ -100,30 +100,26 @@ static inline void array_free(array_t* array) 
/* does not automatically grow */

static inline void* array_get(array_t* array,unsigned int index) {

    assert(index < array->next);

    assert(array->pointer);

    return array->pointer + index * array->item_size;

}



static inline int array_ensure_allocated(array_t* array, int index)

static inline void array_ensure_allocated(array_t *array, int index)

{

    if((index + 1) * array->item_size > array->size) {

        int new_size = (index + 32) * array->item_size;

        array->pointer = g_realloc(array->pointer, new_size);

        if (!array->pointer)

            return -1;

        assert(array->pointer);

        memset(array->pointer + array->size, 0, new_size - array->size);

        array->size = new_size;

        array->next = index + 1;

    }



    return 0;

}



static inline void* array_get_next(array_t* array) {

    unsigned int next = array->next;



    if (array_ensure_allocated(array, next) < 0)

        return NULL;



    array_ensure_allocated(array, next);

    array->next = next + 1;

    return array_get(array, next);

}
@@ -2422,16 +2418,13 @@ static int commit_direntries(BDRVVVFATState* s, 
    direntry_t* direntry = array_get(&(s->directory), dir_index);

    uint32_t first_cluster = dir_index == 0 ? 0 : begin_of_direntry(direntry);

    mapping_t* mapping = find_mapping_for_cluster(s, first_cluster);



    int factor = 0x10 * s->sectors_per_cluster;

    int old_cluster_count, new_cluster_count;

    int current_dir_index = mapping->info.dir.first_dir_index;

    int first_dir_index = current_dir_index;

    int current_dir_index;

    int first_dir_index;

    int ret, i;

    uint32_t c;



DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapping->path, parent_mapping_index));



    assert(direntry);

    assert(mapping);

    assert(mapping->begin == first_cluster);
@@ -2439,6 +2432,11 @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp 
    assert(mapping->mode & MODE_DIRECTORY);

    assert(dir_index == 0 || is_directory(direntry));



    DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n",

                 mapping->path, parent_mapping_index));



    current_dir_index = mapping->info.dir.first_dir_index;

    first_dir_index = current_dir_index;

    mapping->info.dir.parent_mapping_index = parent_mapping_index;



    if (first_cluster == 0) {
@@ -2488,6 +2486,9 @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp 
        direntry = array_get(&(s->directory), first_dir_index + i);

        if (is_directory(direntry) && !is_dot(direntry)) {

            mapping = find_mapping_for_cluster(s, first_cluster);

            if (mapping == NULL) {

                return -1;

            }

            assert(mapping->mode & MODE_DIRECTORY);

            ret = commit_direntries(s, first_dir_index + i,

                array_index(&(s->mapping), mapping));
@@ -2516,6 +2517,10 @@ static int commit_one_file(BDRVVVFATState* s, 
    assert(offset < size);

    assert((offset % s->cluster_size) == 0);



    if (mapping == NULL) {

        return -1;

    }



    for (i = s->cluster_size; i < offset; i += s->cluster_size)

        c = modified_fat_get(s, c);
@@ -2662,8 +2667,12 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s) 
        if (commit->action == ACTION_RENAME) {

            mapping_t* mapping = find_mapping_for_cluster(s,

                    commit->param.rename.cluster);

            char* old_path = mapping->path;

            char *old_path;



            if (mapping == NULL) {

                return -1;

            }

            old_path = mapping->path;

            assert(commit->path);

            mapping->path = commit->path;

            if (rename(old_path, mapping->path))
@@ -2684,10 +2693,15 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s) 
                        direntry_t* d = direntry + i;



                        if (is_file(d) || (is_directory(d) && !is_dot(d))) {

                            int l;

                            char *new_path;

                            mapping_t* m = find_mapping_for_cluster(s,

                                    begin_of_direntry(d));

                            int l = strlen(m->path);

                            char* new_path = g_malloc(l + diff + 1);

                            if (m == NULL) {

                                return -1;

                            }

                            l = strlen(m->path);

                            new_path = g_malloc(l + diff + 1);



                            assert(!strncmp(m->path, mapping->path, l2));