[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3] qga: Support enum names in guest-file-seek

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v3] qga: Support enum names in guest-file-seek
Date: Fri, 20 Mar 2020 09:57:10 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

On 3/20/20 8:49 AM, Peter Maydell wrote:
On Tue, 9 Feb 2016 at 21:27, Eric Blake <address@hidden> wrote:

Magic constants are a pain to use, especially when we run the
risk that our choice of '1' for QGA_SEEK_CUR might differ from
the host or guest's choice of SEEK_CUR.  Better is to use an
enum value, via a qapi alternate type for back-compatibility.

Hi; dragging up this patch from 2016 to say that Coverity
has just noticed that there's some C undefined behaviour
in it (CID 1421990):

Wow, took us a long time to find that!

+/* Convert GuestFileWhence (either a raw integer or an enum value) into
+ * the guest's SEEK_ constants.  */
+int ga_parse_whence(GuestFileWhence *whence, Error **errp)
+    /* Exploit the fact that we picked values to match QGA_SEEK_*. */
+    if (whence->type == QTYPE_QSTRING) {
+        whence->type = QTYPE_QINT;
+        whence->u.value = whence->u.name;

Here whence->u.value and whence->u.name are two different
fields in a union generated by QAPI:

typedef enum QGASeek {
} QGASeek;

struct GuestFileWhence {
     QType type;
     union { /* union tag is @type */
         int64_t value;
         QGASeek name;
     } u;

So u.value and u.name overlap in storage. The C standard
says that this assignment is only valid if the overlap is
exact and the two objects have qualified or unqualified
versions of a compatible type. In this case the enum
type is likely not the same size as an int64_t, and so
we have undefined behaviour.

You are (well, Coverity is) absolutely right!  Patch coming up.

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]