From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Hanna Reitz" <hreitz@redhat.com>,
"Igor Mammedov" <imammedo@redhat.com>,
"Laurent Vivier" <laurent@vivier.eu>,
"Alexandre Iooss" <erdnaxe@crans.org>,
"Alex Bennée" <alex.bennee@linaro.org>,
"Michael Roth" <michael.roth@amd.com>,
"Zhang Chen" <chen.zhang@intel.com>,
"Shannon Zhao" <shannon.zhaosl@gmail.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Alex Williamson" <alex.williamson@redhat.com>,
"Eduardo Habkost" <ehabkost@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>,
"Eric Blake" <eblake@redhat.com>, "Stefan Weil" <sw@weilnetz.de>,
"John Snow" <jsnow@redhat.com>,
"Mahmoud Mandour" <ma.mandourr@gmail.com>,
"Li Zhijian" <lizhijian@cn.fujitsu.com>,
"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
qemu-block@nongnu.org, "Helge Deller" <deller@gmx.de>,
"Michael S. Tsirkin" <mst@redhat.com>,
"David Gibson" <david@gibson.dropbear.id.au>,
"Peter Xu" <peterx@redhat.com>,
"Gonglei (Arei)" <arei.gonglei@huawei.com>,
"Gerd Hoffmann" <kraxel@redhat.com>, "Fam Zheng" <fam@euphon.net>,
"Jason Wang" <jasowang@redhat.com>,
"Vladimir Sementsov-Ogievskiy" <vsementsov@virtuozzo.com>,
"Christian Schoenebeck" <qemu_oss@crudebyte.com>,
"Kevin Wolf" <kwolf@redhat.com>,
"Yuval Shaia" <yuval.shaia.ml@gmail.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Peter Maydell" <peter.maydell@linaro.org>,
qemu-arm@nongnu.org, "Thomas Huth" <thuth@redhat.com>,
"Laurent Vivier" <lvivier@redhat.com>,
"Greg Kurz" <groug@kaod.org>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
qemu-ppc@nongnu.org, "David Hildenbrand" <david@redhat.com>
Subject: [PATCH 03/28] qapi: Replace g_memdup() by g_memdup2_qemu()
Date: Fri, 3 Sep 2021 13:06:37 +0200 [thread overview]
Message-ID: <20210903110702.588291-4-philmd@redhat.com> (raw)
In-Reply-To: <20210903110702.588291-1-philmd@redhat.com>
Per https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538
The old API took the size of the memory to duplicate as a guint,
whereas most memory functions take memory sizes as a gsize. This
made it easy to accidentally pass a gsize to g_memdup(). For large
values, that would lead to a silent truncation of the size from 64
to 32 bits, and result in a heap area being returned which is
significantly smaller than what the caller expects. This can likely
be exploited in various modules to cause a heap buffer overflow.
Replace g_memdup() by the safer g_memdup2_qemu() wrapper.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
qapi/qapi-clone-visitor.c | 16 ++++++++--------
qapi/qapi-visit-core.c | 6 ++++--
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
index c45c5caa3b8..fb38505d982 100644
--- a/qapi/qapi-clone-visitor.c
+++ b/qapi/qapi-clone-visitor.c
@@ -37,7 +37,7 @@ static bool qapi_clone_start_struct(Visitor *v, const char *name, void **obj,
return true;
}
- *obj = g_memdup(*obj, size);
+ *obj = g_memdup2_qemu(*obj, size);
qcv->depth++;
return true;
}
@@ -65,8 +65,8 @@ static GenericList *qapi_clone_next_list(Visitor *v, GenericList *tail,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Unshare the tail of the list cloned by g_memdup() */
- tail->next = g_memdup(tail->next, size);
+ /* Unshare the tail of the list cloned by g_memdup2() */
+ tail->next = g_memdup2_qemu(tail->next, size);
return tail->next;
}
@@ -83,7 +83,7 @@ static bool qapi_clone_type_int64(Visitor *v, const char *name, int64_t *obj,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Value was already cloned by g_memdup() */
+ /* Value was already cloned by g_memdup2() */
return true;
}
@@ -93,7 +93,7 @@ static bool qapi_clone_type_uint64(Visitor *v, const char *name,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Value was already cloned by g_memdup() */
+ /* Value was already cloned by g_memdup2() */
return true;
}
@@ -103,7 +103,7 @@ static bool qapi_clone_type_bool(Visitor *v, const char *name, bool *obj,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Value was already cloned by g_memdup() */
+ /* Value was already cloned by g_memdup2() */
return true;
}
@@ -114,7 +114,7 @@ static bool qapi_clone_type_str(Visitor *v, const char *name, char **obj,
assert(qcv->depth);
/*
- * Pointer was already cloned by g_memdup; create fresh copy.
+ * Pointer was already cloned by g_memdup2; create fresh copy.
* Note that as long as qobject-output-visitor accepts NULL instead of
* "", then we must do likewise. However, we want to obey the
* input visitor semantics of never producing NULL when the empty
@@ -130,7 +130,7 @@ static bool qapi_clone_type_number(Visitor *v, const char *name, double *obj,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Value was already cloned by g_memdup() */
+ /* Value was already cloned by g_memdup2() */
return true;
}
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index a641adec51e..ebabe63b6ea 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -413,8 +413,10 @@ bool visit_type_enum(Visitor *v, const char *name, int *obj,
case VISITOR_OUTPUT:
return output_type_enum(v, name, obj, lookup, errp);
case VISITOR_CLONE:
- /* nothing further to do, scalar value was already copied by
- * g_memdup() during visit_start_*() */
+ /*
+ * nothing further to do, scalar value was already copied by
+ * g_memdup2() during visit_start_*()
+ */
return true;
case VISITOR_DEALLOC:
/* nothing to deallocate for a scalar */
--
2.31.1
WARNING: multiple messages have this Message-ID (diff)
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Fam Zheng" <fam@euphon.net>,
"Peter Maydell" <peter.maydell@linaro.org>,
"Li Zhijian" <lizhijian@cn.fujitsu.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
"Jason Wang" <jasowang@redhat.com>,
"Christian Schoenebeck" <qemu_oss@crudebyte.com>,
"Yuval Shaia" <yuval.shaia.ml@gmail.com>,
"Peter Xu" <peterx@redhat.com>,
"Gerd Hoffmann" <kraxel@redhat.com>,
"Alexandre Iooss" <erdnaxe@crans.org>,
"Eric Blake" <eblake@redhat.com>,
qemu-block@nongnu.org, "Zhang Chen" <chen.zhang@intel.com>,
"Alex Bennée" <alex.bennee@linaro.org>,
"Helge Deller" <deller@gmx.de>,
"David Hildenbrand" <david@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>,
"Gonglei (Arei)" <arei.gonglei@huawei.com>,
"Stefan Weil" <sw@weilnetz.de>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"Laurent Vivier" <lvivier@redhat.com>,
"Thomas Huth" <thuth@redhat.com>,
"Eduardo Habkost" <ehabkost@redhat.com>,
"Michael Roth" <michael.roth@amd.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Greg Kurz" <groug@kaod.org>,
"Alex Williamson" <alex.williamson@redhat.com>,
qemu-arm@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
"John Snow" <jsnow@redhat.com>,
"David Gibson" <david@gibson.dropbear.id.au>,
"Kevin Wolf" <kwolf@redhat.com>,
"Vladimir Sementsov-Ogievskiy" <vsementsov@virtuozzo.com>,
"Laurent Vivier" <laurent@vivier.eu>,
"Shannon Zhao" <shannon.zhaosl@gmail.com>,
"Hanna Reitz" <hreitz@redhat.com>,
qemu-ppc@nongnu.org, "Igor Mammedov" <imammedo@redhat.com>,
"Mahmoud Mandour" <ma.mandourr@gmail.com>
Subject: [PATCH 03/28] qapi: Replace g_memdup() by g_memdup2_qemu()
Date: Fri, 3 Sep 2021 13:06:37 +0200 [thread overview]
Message-ID: <20210903110702.588291-4-philmd@redhat.com> (raw)
In-Reply-To: <20210903110702.588291-1-philmd@redhat.com>
Per https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538
The old API took the size of the memory to duplicate as a guint,
whereas most memory functions take memory sizes as a gsize. This
made it easy to accidentally pass a gsize to g_memdup(). For large
values, that would lead to a silent truncation of the size from 64
to 32 bits, and result in a heap area being returned which is
significantly smaller than what the caller expects. This can likely
be exploited in various modules to cause a heap buffer overflow.
Replace g_memdup() by the safer g_memdup2_qemu() wrapper.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
qapi/qapi-clone-visitor.c | 16 ++++++++--------
qapi/qapi-visit-core.c | 6 ++++--
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
index c45c5caa3b8..fb38505d982 100644
--- a/qapi/qapi-clone-visitor.c
+++ b/qapi/qapi-clone-visitor.c
@@ -37,7 +37,7 @@ static bool qapi_clone_start_struct(Visitor *v, const char *name, void **obj,
return true;
}
- *obj = g_memdup(*obj, size);
+ *obj = g_memdup2_qemu(*obj, size);
qcv->depth++;
return true;
}
@@ -65,8 +65,8 @@ static GenericList *qapi_clone_next_list(Visitor *v, GenericList *tail,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Unshare the tail of the list cloned by g_memdup() */
- tail->next = g_memdup(tail->next, size);
+ /* Unshare the tail of the list cloned by g_memdup2() */
+ tail->next = g_memdup2_qemu(tail->next, size);
return tail->next;
}
@@ -83,7 +83,7 @@ static bool qapi_clone_type_int64(Visitor *v, const char *name, int64_t *obj,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Value was already cloned by g_memdup() */
+ /* Value was already cloned by g_memdup2() */
return true;
}
@@ -93,7 +93,7 @@ static bool qapi_clone_type_uint64(Visitor *v, const char *name,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Value was already cloned by g_memdup() */
+ /* Value was already cloned by g_memdup2() */
return true;
}
@@ -103,7 +103,7 @@ static bool qapi_clone_type_bool(Visitor *v, const char *name, bool *obj,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Value was already cloned by g_memdup() */
+ /* Value was already cloned by g_memdup2() */
return true;
}
@@ -114,7 +114,7 @@ static bool qapi_clone_type_str(Visitor *v, const char *name, char **obj,
assert(qcv->depth);
/*
- * Pointer was already cloned by g_memdup; create fresh copy.
+ * Pointer was already cloned by g_memdup2; create fresh copy.
* Note that as long as qobject-output-visitor accepts NULL instead of
* "", then we must do likewise. However, we want to obey the
* input visitor semantics of never producing NULL when the empty
@@ -130,7 +130,7 @@ static bool qapi_clone_type_number(Visitor *v, const char *name, double *obj,
QapiCloneVisitor *qcv = to_qcv(v);
assert(qcv->depth);
- /* Value was already cloned by g_memdup() */
+ /* Value was already cloned by g_memdup2() */
return true;
}
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index a641adec51e..ebabe63b6ea 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -413,8 +413,10 @@ bool visit_type_enum(Visitor *v, const char *name, int *obj,
case VISITOR_OUTPUT:
return output_type_enum(v, name, obj, lookup, errp);
case VISITOR_CLONE:
- /* nothing further to do, scalar value was already copied by
- * g_memdup() during visit_start_*() */
+ /*
+ * nothing further to do, scalar value was already copied by
+ * g_memdup2() during visit_start_*()
+ */
return true;
case VISITOR_DEALLOC:
/* nothing to deallocate for a scalar */
--
2.31.1
next prev parent reply other threads:[~2021-09-03 11:07 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-03 11:06 [PATCH 00/28] glib: Replace g_memdup() by g_memdup2_qemu() Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 01/28] hw/hyperv/vmbus: Remove unused vmbus_load/save_req() Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 02/28] glib-compat: Introduce g_memdup2() wrapper Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:16 ` Daniel P. Berrangé
2021-09-03 11:16 ` Daniel P. Berrangé
2021-09-03 11:51 ` Vladimir Sementsov-Ogievskiy
2021-09-03 11:56 ` Daniel P. Berrangé
2021-09-03 11:56 ` Daniel P. Berrangé
2021-09-03 12:03 ` Vladimir Sementsov-Ogievskiy
2021-09-03 17:09 ` Philippe Mathieu-Daudé
2021-09-03 17:09 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé [this message]
2021-09-03 11:06 ` [PATCH 03/28] qapi: Replace g_memdup() by g_memdup2_qemu() Philippe Mathieu-Daudé
2021-09-03 11:18 ` Daniel P. Berrangé
2021-09-03 11:18 ` Daniel P. Berrangé
2021-09-03 17:10 ` Philippe Mathieu-Daudé
2021-09-03 17:10 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 04/28] accel/tcg: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 05/28] block/qcow2-bitmap: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 06/28] softmmu: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 07/28] hw/9pfs: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 08/28] hw/acpi: Avoid truncating acpi_data_len() to 32-bit Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-08 7:15 ` Igor Mammedov
2021-09-08 7:15 ` Igor Mammedov
2021-09-03 11:06 ` [PATCH 09/28] hw/acpi: Replace g_memdup() by g_memdup2_qemu() Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-08 7:16 ` Igor Mammedov
2021-09-08 7:16 ` Igor Mammedov
2021-09-03 11:06 ` [PATCH 10/28] hw/core/machine: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 11/28] hw/hppa/machine: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 12/28] hw/i386/multiboot: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 13/28] hw/net/eepro100: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 14/28] hw/nvram/fw_cfg: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 15/28] hw/scsi/mptsas: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 16/28] hw/ppc/spapr_pci: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:45 ` David Gibson
2021-09-03 11:45 ` David Gibson
2021-09-03 11:06 ` [PATCH 17/28] hw/rdma: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 18/28] hw/vfio/pci: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [RFC PATCH 19/28] hw/virtio: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 20/28] net/colo: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [RFC PATCH 21/28] ui/clipboard: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [RFC PATCH 22/28] linux-user: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 23/28] tests/unit: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 24/28] tests/qtest: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:06 ` [PATCH 25/28] target/arm: " Philippe Mathieu-Daudé
2021-09-03 11:06 ` Philippe Mathieu-Daudé
2021-09-03 11:07 ` [PATCH 26/28] target/ppc: " Philippe Mathieu-Daudé
2021-09-03 11:07 ` Philippe Mathieu-Daudé
2021-09-03 11:45 ` David Gibson
2021-09-03 11:45 ` David Gibson
2021-09-03 11:07 ` [PATCH 27/28] contrib: " Philippe Mathieu-Daudé
2021-09-03 11:07 ` Philippe Mathieu-Daudé
2021-09-03 11:07 ` [PATCH 28/28] checkpatch: Do not allow deprecated g_memdup() Philippe Mathieu-Daudé
2021-09-03 11:07 ` Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210903110702.588291-4-philmd@redhat.com \
--to=philmd@redhat.com \
--cc=alex.bennee@linaro.org \
--cc=alex.williamson@redhat.com \
--cc=arei.gonglei@huawei.com \
--cc=armbru@redhat.com \
--cc=chen.zhang@intel.com \
--cc=david@gibson.dropbear.id.au \
--cc=david@redhat.com \
--cc=deller@gmx.de \
--cc=eblake@redhat.com \
--cc=ehabkost@redhat.com \
--cc=erdnaxe@crans.org \
--cc=fam@euphon.net \
--cc=groug@kaod.org \
--cc=hreitz@redhat.com \
--cc=imammedo@redhat.com \
--cc=jasowang@redhat.com \
--cc=jsnow@redhat.com \
--cc=kraxel@redhat.com \
--cc=kwolf@redhat.com \
--cc=laurent@vivier.eu \
--cc=lizhijian@cn.fujitsu.com \
--cc=lvivier@redhat.com \
--cc=ma.mandourr@gmail.com \
--cc=marcel.apfelbaum@gmail.com \
--cc=michael.roth@amd.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=peterx@redhat.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=qemu_oss@crudebyte.com \
--cc=richard.henderson@linaro.org \
--cc=shannon.zhaosl@gmail.com \
--cc=sw@weilnetz.de \
--cc=thuth@redhat.com \
--cc=vsementsov@virtuozzo.com \
--cc=yuval.shaia.ml@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.