* [Qemu-devel] [PATCH v2 1/3] vmstate_register_with_alias_id: Take an Error **
2017-02-02 12:59 [Qemu-devel] [PATCH v2 0/3] Protect against long IDs Dr. David Alan Gilbert (git)
@ 2017-02-02 12:59 ` Dr. David Alan Gilbert (git)
2017-02-02 12:59 ` [Qemu-devel] [PATCH v2 2/3] migration: Check for ID length Dr. David Alan Gilbert (git)
2017-02-02 12:59 ` [Qemu-devel] [PATCH v2 3/3] vmstate registration: check return values Dr. David Alan Gilbert (git)
2 siblings, 0 replies; 4+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-02-02 12:59 UTC (permalink / raw)
To: qemu-devel, quintela, amit.shah
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
I'll be adding an error to it in a subsequent patch.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
hw/core/qdev.c | 3 ++-
hw/intc/apic_common.c | 2 +-
include/migration/vmstate.h | 5 +++--
migration/savevm.c | 3 ++-
stubs/vmstate.c | 3 ++-
5 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 5783442..ea97b15 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -935,7 +935,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
if (qdev_get_vmsd(dev)) {
vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
dev->instance_id_alias,
- dev->alias_required_for_version);
+ dev->alias_required_for_version,
+ NULL);
}
QLIST_FOREACH(bus, &dev->child_bus, sibling) {
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 17df24c..6ce8ef7 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -329,7 +329,7 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
instance_id = -1;
}
vmstate_register_with_alias_id(NULL, instance_id, &vmstate_apic_common,
- s, -1, 0);
+ s, -1, 0, NULL);
}
static void apic_common_unrealize(DeviceState *dev, Error **errp)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 3bbe3ed..c38b00a 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -988,14 +988,15 @@ bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *base, int alias_id,
- int required_for_version);
+ int required_for_version,
+ Error **errp);
static inline int vmstate_register(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *opaque)
{
return vmstate_register_with_alias_id(dev, instance_id, vmsd,
- opaque, -1, 0);
+ opaque, -1, 0, NULL);
}
void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
diff --git a/migration/savevm.c b/migration/savevm.c
index 204012e..59928d6 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -656,7 +656,8 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *opaque, int alias_id,
- int required_for_version)
+ int required_for_version,
+ Error **errp)
{
SaveStateEntry *se;
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index 6590627..bbe158f 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -8,7 +8,8 @@ int vmstate_register_with_alias_id(DeviceState *dev,
int instance_id,
const VMStateDescription *vmsd,
void *base, int alias_id,
- int required_for_version)
+ int required_for_version,
+ Error **errp)
{
return 0;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Qemu-devel] [PATCH v2 2/3] migration: Check for ID length
2017-02-02 12:59 [Qemu-devel] [PATCH v2 0/3] Protect against long IDs Dr. David Alan Gilbert (git)
2017-02-02 12:59 ` [Qemu-devel] [PATCH v2 1/3] vmstate_register_with_alias_id: Take an Error ** Dr. David Alan Gilbert (git)
@ 2017-02-02 12:59 ` Dr. David Alan Gilbert (git)
2017-02-02 12:59 ` [Qemu-devel] [PATCH v2 3/3] vmstate registration: check return values Dr. David Alan Gilbert (git)
2 siblings, 0 replies; 4+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-02-02 12:59 UTC (permalink / raw)
To: qemu-devel, quintela, amit.shah
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
The qdev id of a device can be huge if it's on the end of a chain
of bridges; in reality such chains shouldn't occur but they can
be made to by chaining PCIe bridges together.
The migration format has a number of 256 character long format
limits; check we don't hit them (we already use pstrcat/cpy but
that just protects us from buffer overruns, we fairly quickly
hit an assert).
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
include/migration/vmstate.h | 2 ++
migration/savevm.c | 21 ++++++++++++++++-----
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index c38b00a..6233fe2 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -985,12 +985,14 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
+/* Returns: 0 on success, -1 on failure */
int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *base, int alias_id,
int required_for_version,
Error **errp);
+/* Returns: 0 on success, -1 on failure */
static inline int vmstate_register(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *opaque)
diff --git a/migration/savevm.c b/migration/savevm.c
index 59928d6..e8e5ff5 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -590,8 +590,14 @@ int register_savevm_live(DeviceState *dev,
if (dev) {
char *id = qdev_get_dev_path(dev);
if (id) {
- pstrcpy(se->idstr, sizeof(se->idstr), id);
- pstrcat(se->idstr, sizeof(se->idstr), "/");
+ if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >=
+ sizeof(se->idstr)) {
+ error_report("Path too long for VMState (%s)", id);
+ g_free(id);
+ g_free(se);
+
+ return -1;
+ }
g_free(id);
se->compat = g_new0(CompatEntry, 1);
@@ -674,9 +680,14 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
if (dev) {
char *id = qdev_get_dev_path(dev);
if (id) {
- pstrcpy(se->idstr, sizeof(se->idstr), id);
- pstrcat(se->idstr, sizeof(se->idstr), "/");
- g_free(id);
+ if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >=
+ sizeof(se->idstr)) {
+ error_setg(errp, "Path too long for VMState (%s)", id);
+ g_free(id);
+ g_free(se);
+
+ return -1;
+ }
se->compat = g_new0(CompatEntry, 1);
pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), vmsd->name);
--
2.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Qemu-devel] [PATCH v2 3/3] vmstate registration: check return values
2017-02-02 12:59 [Qemu-devel] [PATCH v2 0/3] Protect against long IDs Dr. David Alan Gilbert (git)
2017-02-02 12:59 ` [Qemu-devel] [PATCH v2 1/3] vmstate_register_with_alias_id: Take an Error ** Dr. David Alan Gilbert (git)
2017-02-02 12:59 ` [Qemu-devel] [PATCH v2 2/3] migration: Check for ID length Dr. David Alan Gilbert (git)
@ 2017-02-02 12:59 ` Dr. David Alan Gilbert (git)
2 siblings, 0 replies; 4+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-02-02 12:59 UTC (permalink / raw)
To: qemu-devel, quintela, amit.shah
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Check qdev's call to vmstate_register_with_alias_id; that gets
most of the common uses; there's hundreds of calls via vmstate_register
which could get fixed over time.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
hw/core/qdev.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index ea97b15..06ba02e 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -933,10 +933,12 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
}
if (qdev_get_vmsd(dev)) {
- vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
- dev->instance_id_alias,
- dev->alias_required_for_version,
- NULL);
+ if (vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
+ dev->instance_id_alias,
+ dev->alias_required_for_version,
+ &local_err) < 0) {
+ goto post_realize_fail;
+ }
}
QLIST_FOREACH(bus, &dev->child_bus, sibling) {
--
2.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread