diff mbox series

[v3,1/4] qom: allow to mark objects as deprecated or not secure.

Message ID 20240606143010.1318226-2-kraxel@redhat.com
State New
Headers show
Series allow to deprecate objects and devices | expand

Commit Message

Gerd Hoffmann June 6, 2024, 2:30 p.m. UTC
Add flags to ObjectClass for objects which are deprecated or not secure.
Add 'deprecated' and 'not-secure' bools to ObjectTypeInfo, report in
'qom-list-types'.  Print the flags when listing devices via '-device
help'.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/qom/object.h  | 3 +++
 qom/qom-qmp-cmds.c    | 8 ++++++++
 system/qdev-monitor.c | 8 ++++++++
 qapi/qom.json         | 8 +++++++-
 4 files changed, 26 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé June 6, 2024, 2:38 p.m. UTC | #1
On Thu, Jun 06, 2024 at 04:30:07PM +0200, Gerd Hoffmann wrote:
> Add flags to ObjectClass for objects which are deprecated or not secure.
> Add 'deprecated' and 'not-secure' bools to ObjectTypeInfo, report in
> 'qom-list-types'.  Print the flags when listing devices via '-device
> help'.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/qom/object.h  | 3 +++
>  qom/qom-qmp-cmds.c    | 8 ++++++++
>  system/qdev-monitor.c | 8 ++++++++
>  qapi/qom.json         | 8 +++++++-
>  4 files changed, 26 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
Philippe Mathieu-Daudé June 7, 2024, 6:24 a.m. UTC | #2
On 6/6/24 16:30, Gerd Hoffmann wrote:
> Add flags to ObjectClass for objects which are deprecated or not secure.
> Add 'deprecated' and 'not-secure' bools to ObjectTypeInfo, report in
> 'qom-list-types'.  Print the flags when listing devices via '-device
> help'.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   include/qom/object.h  | 3 +++
>   qom/qom-qmp-cmds.c    | 8 ++++++++
>   system/qdev-monitor.c | 8 ++++++++
>   qapi/qom.json         | 8 +++++++-
>   4 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 13d3a655ddf9..419bd9a4b219 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -136,6 +136,9 @@ struct ObjectClass
>       ObjectUnparent *unparent;
>   
>       GHashTable *properties;
> +
> +    bool deprecated;
> +    bool not_secure;

LGTM but I'd rather use a reason string instead of a boolean,
so we are forced to justify.

That would be in line with MachineClass::deprecation_reason:

  * MachineClass:
  * @deprecation_reason: If set, the machine is marked as deprecated.
  *    The string should provide some clear information about what to
  *    use instead.

>   };
>   
>   /**
> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
> index e91a2353472a..325ff0ba2a25 100644
> --- a/qom/qom-qmp-cmds.c
> +++ b/qom/qom-qmp-cmds.c
> @@ -101,6 +101,14 @@ static void qom_list_types_tramp(ObjectClass *klass, void *data)
>       if (parent) {
>           info->parent = g_strdup(object_class_get_name(parent));
>       }
> +    if (klass->deprecated) {
> +        info->has_deprecated = true;
> +        info->deprecated = true;
> +    }
> +    if (klass->not_secure) {
> +        info->has_not_secure = true;
> +        info->not_secure = true;
> +    }
>   
>       QAPI_LIST_PREPEND(*pret, info);
>   }
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index 6af6ef7d667f..effdc95d21d3 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -144,6 +144,8 @@ static bool qdev_class_has_alias(DeviceClass *dc)
>   
>   static void qdev_print_devinfo(DeviceClass *dc)
>   {
> +    ObjectClass *klass = OBJECT_CLASS(dc);
> +
>       qemu_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
>       if (dc->bus_type) {
>           qemu_printf(", bus %s", dc->bus_type);
> @@ -157,6 +159,12 @@ static void qdev_print_devinfo(DeviceClass *dc)
>       if (!dc->user_creatable) {
>           qemu_printf(", no-user");
>       }
> +    if (klass->deprecated) {
> +        qemu_printf(", deprecated");
> +    }
> +    if (klass->not_secure) {
> +        qemu_printf(", not-secure");
> +    }
>       qemu_printf("\n");
>   }
>   
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 8bd299265e39..3f20d4c6413b 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -163,10 +163,16 @@
>   #
>   # @parent: Name of parent type, if any (since 2.10)
>   #
> +# @deprecated: the type is deprecated (since 9.1)
> +#
> +# @not-secure: the type (typically a device) is not considered
> +#     a security boundary (since 9.1)
> +#
>   # Since: 1.1
>   ##
>   { 'struct': 'ObjectTypeInfo',
> -  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str' } }
> +  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str',
> +            '*deprecated': 'bool', '*not-secure': 'bool' } }
>   
>   ##
>   # @qom-list-types:
Markus Armbruster June 12, 2024, 11:07 a.m. UTC | #3
Gerd Hoffmann <kraxel@redhat.com> writes:

> Add flags to ObjectClass for objects which are deprecated or not secure.
> Add 'deprecated' and 'not-secure' bools to ObjectTypeInfo, report in
> 'qom-list-types'.  Print the flags when listing devices via '-device
> help'.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/qom/object.h  | 3 +++
>  qom/qom-qmp-cmds.c    | 8 ++++++++
>  system/qdev-monitor.c | 8 ++++++++
>  qapi/qom.json         | 8 +++++++-
>  4 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 13d3a655ddf9..419bd9a4b219 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -136,6 +136,9 @@ struct ObjectClass
>      ObjectUnparent *unparent;
>  
>      GHashTable *properties;
> +
> +    bool deprecated;
> +    bool not_secure;
>  };

Ignorant question: should this be in struct TypeImpl instead?

>  
>  /**
> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
> index e91a2353472a..325ff0ba2a25 100644
> --- a/qom/qom-qmp-cmds.c
> +++ b/qom/qom-qmp-cmds.c
> @@ -101,6 +101,14 @@ static void qom_list_types_tramp(ObjectClass *klass, void *data)
>      if (parent) {
>          info->parent = g_strdup(object_class_get_name(parent));
>      }
> +    if (klass->deprecated) {
> +        info->has_deprecated = true;
> +        info->deprecated = true;
> +    }
> +    if (klass->not_secure) {
> +        info->has_not_secure = true;
> +        info->not_secure = true;
> +    }
>  
>      QAPI_LIST_PREPEND(*pret, info);
>  }
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index 6af6ef7d667f..effdc95d21d3 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -144,6 +144,8 @@ static bool qdev_class_has_alias(DeviceClass *dc)
>  
>  static void qdev_print_devinfo(DeviceClass *dc)
>  {
> +    ObjectClass *klass = OBJECT_CLASS(dc);
> +
>      qemu_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
>      if (dc->bus_type) {
>          qemu_printf(", bus %s", dc->bus_type);
> @@ -157,6 +159,12 @@ static void qdev_print_devinfo(DeviceClass *dc)
>      if (!dc->user_creatable) {
>          qemu_printf(", no-user");
>      }
> +    if (klass->deprecated) {
> +        qemu_printf(", deprecated");
> +    }
> +    if (klass->not_secure) {
> +        qemu_printf(", not-secure");
> +    }
>      qemu_printf("\n");
>  }
>  
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 8bd299265e39..3f20d4c6413b 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -163,10 +163,16 @@
>  #
>  # @parent: Name of parent type, if any (since 2.10)
>  #
> +# @deprecated: the type is deprecated (since 9.1)
> +#
> +# @not-secure: the type (typically a device) is not considered
> +#     a security boundary (since 9.1)

What does this mean?  Does it mean "do not add an instance of this
device the guest unless you trust the guest"?

> +#
>  # Since: 1.1
>  ##
>  { 'struct': 'ObjectTypeInfo',
> -  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str' } }
> +  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str',
> +            '*deprecated': 'bool', '*not-secure': 'bool' } }
>  
>  ##
>  # @qom-list-types:

I dislike booleans named "no-FOO" or "not-FOO", because they lead to
double-negation.
Daniel P. Berrangé June 12, 2024, 11:24 a.m. UTC | #4
On Wed, Jun 12, 2024 at 01:07:44PM +0200, Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> > Add flags to ObjectClass for objects which are deprecated or not secure.
> > Add 'deprecated' and 'not-secure' bools to ObjectTypeInfo, report in
> > 'qom-list-types'.  Print the flags when listing devices via '-device
> > help'.
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  include/qom/object.h  | 3 +++
> >  qom/qom-qmp-cmds.c    | 8 ++++++++
> >  system/qdev-monitor.c | 8 ++++++++
> >  qapi/qom.json         | 8 +++++++-
> >  4 files changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index 13d3a655ddf9..419bd9a4b219 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -136,6 +136,9 @@ struct ObjectClass
> >      ObjectUnparent *unparent;
> >  
> >      GHashTable *properties;
> > +
> > +    bool deprecated;
> > +    bool not_secure;
> >  };
> 
> Ignorant question: should this be in struct TypeImpl instead?
> 
> >  
> >  /**
> > diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
> > index e91a2353472a..325ff0ba2a25 100644
> > --- a/qom/qom-qmp-cmds.c
> > +++ b/qom/qom-qmp-cmds.c
> > @@ -101,6 +101,14 @@ static void qom_list_types_tramp(ObjectClass *klass, void *data)
> >      if (parent) {
> >          info->parent = g_strdup(object_class_get_name(parent));
> >      }
> > +    if (klass->deprecated) {
> > +        info->has_deprecated = true;
> > +        info->deprecated = true;
> > +    }
> > +    if (klass->not_secure) {
> > +        info->has_not_secure = true;
> > +        info->not_secure = true;
> > +    }
> >  
> >      QAPI_LIST_PREPEND(*pret, info);
> >  }
> > diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> > index 6af6ef7d667f..effdc95d21d3 100644
> > --- a/system/qdev-monitor.c
> > +++ b/system/qdev-monitor.c
> > @@ -144,6 +144,8 @@ static bool qdev_class_has_alias(DeviceClass *dc)
> >  
> >  static void qdev_print_devinfo(DeviceClass *dc)
> >  {
> > +    ObjectClass *klass = OBJECT_CLASS(dc);
> > +
> >      qemu_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
> >      if (dc->bus_type) {
> >          qemu_printf(", bus %s", dc->bus_type);
> > @@ -157,6 +159,12 @@ static void qdev_print_devinfo(DeviceClass *dc)
> >      if (!dc->user_creatable) {
> >          qemu_printf(", no-user");
> >      }
> > +    if (klass->deprecated) {
> > +        qemu_printf(", deprecated");
> > +    }
> > +    if (klass->not_secure) {
> > +        qemu_printf(", not-secure");
> > +    }
> >      qemu_printf("\n");
> >  }
> >  
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index 8bd299265e39..3f20d4c6413b 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -163,10 +163,16 @@
> >  #
> >  # @parent: Name of parent type, if any (since 2.10)
> >  #
> > +# @deprecated: the type is deprecated (since 9.1)
> > +#
> > +# @not-secure: the type (typically a device) is not considered
> > +#     a security boundary (since 9.1)
> 
> What does this mean?  Does it mean "do not add an instance of this
> device the guest unless you trust the guest"?

Essentially yes. This ties to our security doc where we declare
we won't consider non-virtualization use cases as being security
bugs (CVEs) as large parts of QEMU haven't been designed to
provide a guest security boundary

  https://www.qemu.org/docs/master/system/security.html


With regards,
Daniel
Markus Armbruster June 12, 2024, 11:44 a.m. UTC | #5
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, Jun 12, 2024 at 01:07:44PM +0200, Markus Armbruster wrote:
>> Gerd Hoffmann <kraxel@redhat.com> writes:
>> 
>> > Add flags to ObjectClass for objects which are deprecated or not secure.
>> > Add 'deprecated' and 'not-secure' bools to ObjectTypeInfo, report in
>> > 'qom-list-types'.  Print the flags when listing devices via '-device
>> > help'.
>> >
>> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

[...]

>> > diff --git a/qapi/qom.json b/qapi/qom.json
>> > index 8bd299265e39..3f20d4c6413b 100644
>> > --- a/qapi/qom.json
>> > +++ b/qapi/qom.json
>> > @@ -163,10 +163,16 @@
>> >  #
>> >  # @parent: Name of parent type, if any (since 2.10)
>> >  #
>> > +# @deprecated: the type is deprecated (since 9.1)
>> > +#
>> > +# @not-secure: the type (typically a device) is not considered
>> > +#     a security boundary (since 9.1)
>> 
>> What does this mean?  Does it mean "do not add an instance of this
>> device the guest unless you trust the guest"?
>
> Essentially yes. This ties to our security doc where we declare
> we won't consider non-virtualization use cases as being security
> bugs (CVEs) as large parts of QEMU haven't been designed to
> provide a guest security boundary
>
>   https://www.qemu.org/docs/master/system/security.html

Would it make sense to add a suitable pointer to the doc comment?
diff mbox series

Patch

diff --git a/include/qom/object.h b/include/qom/object.h
index 13d3a655ddf9..419bd9a4b219 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -136,6 +136,9 @@  struct ObjectClass
     ObjectUnparent *unparent;
 
     GHashTable *properties;
+
+    bool deprecated;
+    bool not_secure;
 };
 
 /**
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index e91a2353472a..325ff0ba2a25 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -101,6 +101,14 @@  static void qom_list_types_tramp(ObjectClass *klass, void *data)
     if (parent) {
         info->parent = g_strdup(object_class_get_name(parent));
     }
+    if (klass->deprecated) {
+        info->has_deprecated = true;
+        info->deprecated = true;
+    }
+    if (klass->not_secure) {
+        info->has_not_secure = true;
+        info->not_secure = true;
+    }
 
     QAPI_LIST_PREPEND(*pret, info);
 }
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 6af6ef7d667f..effdc95d21d3 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -144,6 +144,8 @@  static bool qdev_class_has_alias(DeviceClass *dc)
 
 static void qdev_print_devinfo(DeviceClass *dc)
 {
+    ObjectClass *klass = OBJECT_CLASS(dc);
+
     qemu_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
     if (dc->bus_type) {
         qemu_printf(", bus %s", dc->bus_type);
@@ -157,6 +159,12 @@  static void qdev_print_devinfo(DeviceClass *dc)
     if (!dc->user_creatable) {
         qemu_printf(", no-user");
     }
+    if (klass->deprecated) {
+        qemu_printf(", deprecated");
+    }
+    if (klass->not_secure) {
+        qemu_printf(", not-secure");
+    }
     qemu_printf("\n");
 }
 
diff --git a/qapi/qom.json b/qapi/qom.json
index 8bd299265e39..3f20d4c6413b 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -163,10 +163,16 @@ 
 #
 # @parent: Name of parent type, if any (since 2.10)
 #
+# @deprecated: the type is deprecated (since 9.1)
+#
+# @not-secure: the type (typically a device) is not considered
+#     a security boundary (since 9.1)
+#
 # Since: 1.1
 ##
 { 'struct': 'ObjectTypeInfo',
-  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str' } }
+  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str',
+            '*deprecated': 'bool', '*not-secure': 'bool' } }
 
 ##
 # @qom-list-types: