Re: [libvirt] [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[P J P]

+-- On Fri, 26 Oct 2018, Daniel P. Berrangé wrote --+
| ... 
| One thing we should do, however, is to make it clear which of the
| device models we consider secure, and which we consider only usable
| in a friendly guest environment, as we have very different code
| maintainership & quality standards for different parts of QEMU.
| 
| Essentially virtio devices, and then only a handful of the emulated
| devices are things we consider suitable for usage in secure envs.
| Likewise for machine types probably.

True, +1.

It did come up in another thread. It'll surely be helpful to list these 
professional and friendly components. 'Professional' being production ready 
and thus security relevant. And 'Friendly' being experimental or not suitable 
for production usage. Maybe like staging drivers in the kernel tree. They are 
available for use but not considered production ready and thus are not 
security relevant.

To be clear, irrespective of professional or friendly, we strive to fix every 
single issue that is found and/or reported. Only difference is, professional 
ones are tracked by a CVE ID and friendly ones are fixed as bug fixes, not 
tracked by CVE ID.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Fri, 26 Oct 2018, Daniel P. Berrangé wrote --+
| > No, since the adlib device is not used as much and is being deprecated, I'm 
| > not inclined to get one.
| 
| Any security issue that affects code in QEMU that is currently being
| shipped by distros should have a CVE.
| 
| Whether we intend to deprecate & delete it later should not be a factor
| because we are free to cancel the deprecation process at any time if we
| find a reason to keep the feature around.

Okay, will follow up with a CVE process. Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
| Oh, thanks!  I said I was dumb. :)  So the fix is just this:
| 
| diff --git a/hw/audio/fmopl.h b/hw/audio/fmopl.h
| index e7e578a48e..7199afaa3c 100644
| --- a/hw/audio/fmopl.h
| +++ b/hw/audio/fmopl.h
| @@ -72,8 +72,8 @@ typedef struct fm_opl_f {
|   /* Rhythm sention */
|   uint8_t rhythm; /* Rhythm mode , key flag */
|   /* time tables */
| - int32_t AR_TABLE[75];   /* atttack rate tables */
| - int32_t DR_TABLE[75];   /* decay rate tables   */
| + int32_t AR_TABLE[76];   /* atttack rate tables */
| + int32_t DR_TABLE[76];   /* decay rate tables   */
|   uint32_t FN_TABLE[1024];  /* fnumber -> increment counter */
|   /* LFO */
|   int32_t *ams_table;
| 
| and init_timetables will just fill it with the right value?  (I checked
| against another implementation at http://opl3.cozendey.com/).

Gerd has proposed to a patch to deprecate adlib, as it's not used as much. IMO 
deprecation is better option. But if that is not happening, above seems good.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+
| I am dumb and I don't understand.  In set_ar_dr you get
| 
|   v = 0xff
|   ar = 15
|   dr = 15
| 
| and OPL->AR_TABLE[60] is accessed.  The size of the array is 75, which
| seems to be actually 14 more than required.  Likewise OPL->DR_TABLE[60]
| is accessed.
| 
| The next accesses use SLOT->ksr which is 0 so it's fine too.

In set_ar_dr

  SLOT->AR = ar ? >AR_TABLE[ar<<2] : RATE_0;

SLOT->AR is set to point to OPL->DR_TABLE[60] and while so if s->ksr is set to 
15, in CALC_FCSLOT()

  SLOT->evsa = SLOT->AR[ksr];  <= accesses OPL->AR_TABLE[60 + 15];

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
| On Thu, Oct 25, 2018 at 04:26:16PM +0530, P J P wrote:
| > +-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| > | We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
| > | 
| > | Reproducer:
| > | outw(0xff60, 0x220);
| > | outw(0x1020, 0x220);
| > | outw(0xffb0, 0x220);
| > | Result:
| > | Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])
| > 
| > + Reported-by: Wangjunqing 
| 
| So you have a CVE number for this ?

No, since the adlib device is not used as much and is being deprecated, I'm 
not inclined to get one.

--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[P J P]

  Hello Dan, all

+-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
| On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
| > While being at it deprecate cirrus too.
| > 
| > Reason (short version): use stdvga instead.
| > Verbose version:
| > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
| 
| 
| I don't debate the points in the blog post above that stdvga is a
| better choice, but I don't think that's enough to justify deprecating
| cirrus at this point in time, because when it then gets deleted it
| will break way too many existing deployments.
| 
| We need to socialize info in that blog post above more widely and
| especially ensure that apps are not using that by default. I don't
| see it being viable to formally deprecate it in QEMU any time soon
| though given existing usage.

To note, IMO there are other devices/sources in QEMU which are potential 
candidates for deprecation, similar to adlib etc. It'll help if we could 
device a process to deprecate/remove such code base. Other than maintenance it 
invariably also becomes source of security issues.

Ex.(similar to Fedora) we could announce such candidate on qemu-devel list and 
after review over a period of say a month, candidate will be 
deprecated/expunged. (thinking aloud)

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] cirrus: mark as deprecated

[P J P]

+-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| While being at it deprecate cirrus too.
| 
| Reason (short version): use stdvga instead.
| Verbose version:
| https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
| 
| Signed-off-by: Gerd Hoffmann 
| ---
|  hw/display/cirrus_vga.c | 2 ++
|  hw/display/cirrus_vga_isa.c | 2 ++
|  qemu-deprecated.texi| 4 
|  3 files changed, 8 insertions(+)
| 
| diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
| index d9b854d..2f16ba9 100644
| --- a/hw/display/cirrus_vga.c
| +++ b/hw/display/cirrus_vga.c
| @@ -3024,6 +3024,8 @@ static void cirrus_vga_class_init(ObjectClass *klass, 
void *data)
|  dc->vmsd = _pci_cirrus_vga;
|  dc->props = pci_vga_cirrus_properties;
|  dc->hotpluggable = false;
| +dc->deprecation_reason =
| +
"https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful;;
|  }
|  
|  static const TypeInfo cirrus_vga_info = {
| diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
| index fa10b74..c2d853c 100644
| --- a/hw/display/cirrus_vga_isa.c
| +++ b/hw/display/cirrus_vga_isa.c
| @@ -81,6 +81,8 @@ static void isa_cirrus_vga_class_init(ObjectClass *klass, 
void *data)
|  dc->realize = isa_cirrus_vga_realizefn;
|  dc->props = isa_cirrus_vga_properties;
|  set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
| +dc->deprecation_reason =
| +
"https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful;;
|  }
|  
|  static const TypeInfo isa_cirrus_vga_info = {
| diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
| index 7951a4f..1b1d434 100644
| --- a/qemu-deprecated.texi
| +++ b/qemu-deprecated.texi
| @@ -120,6 +120,10 @@ or ``ivshmem-doorbell`` device types.
|  
|  Has known buffer overflow.
|  
| +@subsection cirrus (since 3.1)
| +
| +Use stdvga instead (-vga std or -device VGA).
| +
|  @section System emulator machines
|  
|  @subsection pc-0.10 and pc-0.11 (since 3.0)
| 

Reviewed-by: Prasad J Pandit 

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] adlib: mark as insecure and deprecated.

[P J P]

+-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| We have a lovely, guest-triggerable buffer overflow in opl2 emulation.
| 
| Reproducer:
| outw(0xff60, 0x220);
| outw(0x1020, 0x220);
| outw(0xffb0, 0x220);
| Result:
| Will overflow FM_OPL->AR_TABLE[] (see hw/audio/fmopl.[ch])

+ Reported-by: Wangjunqing 


| diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
| index 97b876c..fb4a29c 100644
| --- a/hw/audio/adlib.c
| +++ b/hw/audio/adlib.c
| @@ -311,6 +311,7 @@ static void adlib_class_initfn (ObjectClass *klass, void 
*data)
|  set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
|  dc->desc = ADLIB_DESC;
|  dc->props = adlib_properties;
| +dc->deprecation_reason = "insecure, buffer overflow in opl2 emulation";
|  }
|  
|  static const TypeInfo adlib_info = {
| diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
| index 11b870c..7951a4f 100644
| --- a/qemu-deprecated.texi
| +++ b/qemu-deprecated.texi
| @@ -116,6 +116,10 @@ The @option{[hub_id name]} parameter tuple of the 
'hostfwd_add' and
|  The ``ivshmem'' device type is replaced by either the ``ivshmem-plain''
|  or ``ivshmem-doorbell`` device types.
|  
| +@subsection adlib (since 3.1)
| +
| +Has known buffer overflow.
| +
|  @section System emulator machines
|  
|  @subsection pc-0.10 and pc-0.11 (since 3.0)

Okay.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] qdev: add deprecation_reason to DeviceClass

[P J P]

+-- On Thu, 25 Oct 2018, Gerd Hoffmann wrote --+
| Simliar to deprecated machine types.
| Print a warning when creating a deprecated device.
| Add deprecation notice to -device help.
| 
| TODO: add to intospection.

 s/intospection/introspection ..?

| diff --git a/hw/core/qdev.c b/hw/core/qdev.c
| index 046d8f1..3b27a74 100644
| --- a/hw/core/qdev.c
| +++ b/hw/core/qdev.c
| @@ -133,11 +133,18 @@ DeviceState *qdev_create(BusState *bus, const char 
*name)
|  
|  DeviceState *qdev_try_create(BusState *bus, const char *type)
|  {
| +DeviceClass *dc;
|  DeviceState *dev;
|  
| -if (object_class_by_name(type) == NULL) {
| +dc = DEVICE_CLASS(object_class_by_name(type));
| +if (dc == NULL) {
|  return NULL;
|  }
| +if (dc->deprecation_reason) {
| +warn_report("device %s is deprecated (%s)",
| +type, dc->deprecation_reason);
| +}
| +
|  dev = DEVICE(object_new(type));
|  if (!dev) {
|  return NULL;
| diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
| index a24d0dd..a352eaa 100644
| --- a/include/hw/qdev-core.h
| +++ b/include/hw/qdev-core.h
| @@ -105,6 +105,7 @@ typedef struct DeviceClass {
|   */
|  bool user_creatable;
|  bool hotpluggable;
| +const char *deprecation_reason;
|  
|  /* callbacks */
|  DeviceReset reset;
| diff --git a/qdev-monitor.c b/qdev-monitor.c
| index 802c18a..bbba2bc 100644
| --- a/qdev-monitor.c
| +++ b/qdev-monitor.c
| @@ -128,6 +128,9 @@ static void qdev_print_devinfo(DeviceClass *dc)
|  if (!dc->user_creatable) {
|  out_printf(", no-user");
|  }
| +if (!dc->deprecation_reason) {
| +out_printf(", deprecated");
| +}
|  out_printf("\n");
|  }
|  
| @@ -579,6 +582,10 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error 
**errp)
|  if (!dc) {
|  return NULL;
|  }
| +if (dc->deprecation_reason) {
| +warn_report("device %s is deprecated (%s)",
| +driver, dc->deprecation_reason);
| +}
|  
|  /* find bus */
|  path = qemu_opt_get(opts, "bus");
| 

Looks good. Should 'deprecation_reason' be listed in qdev_device_help()?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virlog: determine the hostname on startup CVE-2018-XXX

[P J P]

+-- On Mon, 5 Feb 2018, Daniel P. Berrangé wrote --+
| From: Lubomir Rintel 
| 
| At later point it might not be possible or even safe to use getaddrinfo(). It
| can in turn result in a load of NSS module.
| 
| Notably, on a LXC container startup we may find ourselves with the guest
| filesystem already having replaced the host one. Loading a NSS module
| from the guest tree could allow a malicous guest to escape the
| confinement of its container environment because libvirt will not yet
| have locked it down.
| ---
| 
| NB, we're still awaiting CVE allocation before pushing to git

'CVE-2018-6764' has been assigned to this issue by Mitre.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list