diff mbox

qdev: fix crash by validating the object type

Message ID 1397613434-26497-1-git-send-email-akong@redhat.com
State New
Headers show

Commit Message

Amos Kong April 16, 2014, 1:57 a.m. UTC
QEMU crashed when I try to list device parameters, the driver name is
actually the available bus name.

 # qemu -device virtio-pci-bus,?
 # qemu -device virtio-bus,?
 # qemu -device virtio-serial-bus,?
 qdev-monitor.c:212:qdev_device_help: Object 0x7fd932f50620 is not an
 instance of type device
 Aborted (core dumped)

We can also reproduce this bug by adding device from monitor, so it's
worth to fix the crash.

 (qemu) device_add virtio-serial-bus
 qdev-monitor.c:491:qdev_device_add: Object 0x7f5e89530920 is not an
 instance of type device
 Aborted (core dumped)

Cc: qemu-stable@nongnu.org
Signed-off-by: Amos Kong <akong@redhat.com>
---
 qdev-monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Markus Armbruster April 16, 2014, 7:02 a.m. UTC | #1
Amos Kong <akong@redhat.com> writes:

> QEMU crashed when I try to list device parameters, the driver name is
> actually the available bus name.
>
>  # qemu -device virtio-pci-bus,?
>  # qemu -device virtio-bus,?
>  # qemu -device virtio-serial-bus,?
>  qdev-monitor.c:212:qdev_device_help: Object 0x7fd932f50620 is not an
>  instance of type device
>  Aborted (core dumped)
>
> We can also reproduce this bug by adding device from monitor, so it's
> worth to fix the crash.
>
>  (qemu) device_add virtio-serial-bus
>  qdev-monitor.c:491:qdev_device_add: Object 0x7f5e89530920 is not an
>  instance of type device
>  Aborted (core dumped)
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  qdev-monitor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 9268c87..40c117d 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -206,7 +206,7 @@ int qdev_device_help(QemuOpts *opts)
       if (!driver || !qemu_opt_has_help_opt(opts)) {
           return 0;
       }

       klass = object_class_by_name(driver);
       if (!klass) {
           const char *typename = find_typename_by_alias(driver);

           if (typename) {
               driver = typename;
               klass = object_class_by_name(driver);
>          }
>      }
>  
> -    if (!klass) {
> +    if (!object_class_dynamic_cast(klass, TYPE_DEVICE)) {
>          return 0;
>      }
>      do {

Works because when qdev_device_help() returns zero, its caller
do_device_add() proceeds to call qdev_device_add(), which checks "klass
subtype of TYPE_DEVICE" again, and reports properly when it's not:
"-device virtio-bus,help: 'virtio-bus' is not a valid device model
name".

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Andreas Färber April 30, 2014, 3:55 p.m. UTC | #2
Am 16.04.2014 09:02, schrieb Markus Armbruster:
> Amos Kong <akong@redhat.com> writes:
> 
>> QEMU crashed when I try to list device parameters, the driver name is
>> actually the available bus name.
>>
>>  # qemu -device virtio-pci-bus,?
>>  # qemu -device virtio-bus,?
>>  # qemu -device virtio-serial-bus,?
>>  qdev-monitor.c:212:qdev_device_help: Object 0x7fd932f50620 is not an
>>  instance of type device
>>  Aborted (core dumped)
>>
>> We can also reproduce this bug by adding device from monitor, so it's
>> worth to fix the crash.
>>
>>  (qemu) device_add virtio-serial-bus
>>  qdev-monitor.c:491:qdev_device_add: Object 0x7f5e89530920 is not an
>>  instance of type device
>>  Aborted (core dumped)
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Amos Kong <akong@redhat.com>
>> ---
>>  qdev-monitor.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index 9268c87..40c117d 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -206,7 +206,7 @@ int qdev_device_help(QemuOpts *opts)
>        if (!driver || !qemu_opt_has_help_opt(opts)) {
>            return 0;
>        }
> 
>        klass = object_class_by_name(driver);
>        if (!klass) {
>            const char *typename = find_typename_by_alias(driver);
> 
>            if (typename) {
>                driver = typename;
>                klass = object_class_by_name(driver);
>>          }
>>      }
>>  
>> -    if (!klass) {
>> +    if (!object_class_dynamic_cast(klass, TYPE_DEVICE)) {
>>          return 0;
>>      }
>>      do {
> 
> Works because when qdev_device_help() returns zero, its caller
> do_device_add() proceeds to call qdev_device_add(), which checks "klass
> subtype of TYPE_DEVICE" again, and reports properly when it's not:
> "-device virtio-bus,help: 'virtio-bus' is not a valid device model
> name".
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks, applied to qom-next (with message slightly tweaked):
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas
diff mbox

Patch

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 9268c87..40c117d 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -206,7 +206,7 @@  int qdev_device_help(QemuOpts *opts)
         }
     }
 
-    if (!klass) {
+    if (!object_class_dynamic_cast(klass, TYPE_DEVICE)) {
         return 0;
     }
     do {