All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
@ 2023-10-26  7:06 Cédric Le Goater
  2023-10-26  7:06 ` [PATCH v2 1/3] util/uuid: Add UUID_STR_LEN definition Cédric Le Goater
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-10-26  7:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras, Cédric Le Goater

Hello,

This series fixes a buffer overrun in VFIO. The buffer used in
vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
one byte for the trailing NUL.

Instead of adding + 1, as done elsewhere, the changes introduce a
UUID_STR_LEN define for the correct size and use it where required.

Thanks,

C. 

Changes in v2:
 - removal of UUID_FMT_LEN

Cédric Le Goater (3):
  util/uuid: Add UUID_STR_LEN definition
  vfio/pci: Fix buffer overrun when writing the VF token
  util/uuid: Remove UUID_FMT_LEN

 include/qemu/uuid.h              | 2 +-
 block/parallels-ext.c            | 2 +-
 block/vdi.c                      | 2 +-
 hw/core/qdev-properties-system.c | 2 +-
 hw/hyperv/vmbus.c                | 4 ++--
 hw/vfio/pci.c                    | 2 +-
 migration/savevm.c               | 4 ++--
 tests/unit/test-uuid.c           | 2 +-
 util/uuid.c                      | 2 +-
 9 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.41.0



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/3] util/uuid: Add UUID_STR_LEN definition
  2023-10-26  7:06 [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
@ 2023-10-26  7:06 ` Cédric Le Goater
  2023-10-26  7:06 ` [PATCH v2 2/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-10-26  7:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras, Cédric Le Goater,
	Philippe Mathieu-Daudé

qemu_uuid_unparse() includes a trailing NUL when writing the uuid
string and the buffer size should be UUID_FMT_LEN + 1 bytes. Add a
define for this size and use it where required.

Cc: Fam Zheng <fam@euphon.net>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/qemu/uuid.h              | 1 +
 block/parallels-ext.c            | 2 +-
 block/vdi.c                      | 2 +-
 hw/core/qdev-properties-system.c | 2 +-
 hw/hyperv/vmbus.c                | 4 ++--
 migration/savevm.c               | 4 ++--
 tests/unit/test-uuid.c           | 2 +-
 util/uuid.c                      | 2 +-
 8 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
index e24a1099e45f2dfc330a578d3ccbe74f3e52e6c1..4e7afaf1d5bd5d382fefbd6f6275d69cf25e7483 100644
--- a/include/qemu/uuid.h
+++ b/include/qemu/uuid.h
@@ -79,6 +79,7 @@ typedef struct {
                  "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
 
 #define UUID_FMT_LEN 36
+#define UUID_STR_LEN (UUID_FMT_LEN + 1)
 
 #define UUID_NONE "00000000-0000-0000-0000-000000000000"
 
diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 8a109f005ae73e848658e3f044968307a0bfd99d..4d8ecf5047abfe4ba0e7273139638649f5d101a0 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -130,7 +130,7 @@ static BdrvDirtyBitmap *parallels_load_bitmap(BlockDriverState *bs,
     g_autofree uint64_t *l1_table = NULL;
     BdrvDirtyBitmap *bitmap;
     QemuUUID uuid;
-    char uuidstr[UUID_FMT_LEN + 1];
+    char uuidstr[UUID_STR_LEN];
     int i;
 
     if (data_size < sizeof(bf)) {
diff --git a/block/vdi.c b/block/vdi.c
index fd7e3653832f890776e03a845a157fede10655b3..fa6e5e198c5d8f4047f0ecddece2493158fe6bc2 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -239,7 +239,7 @@ static void vdi_header_to_le(VdiHeader *header)
 
 static void vdi_header_print(VdiHeader *header)
 {
-    char uuidstr[37];
+    char uuidstr[UUID_STR_LEN];
     QemuUUID uuid;
     logout("text        %s", header->text);
     logout("signature   0x%08x\n", header->signature);
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 8e0acf50d6ca045938a44d6d72547607f919ca79..e2130c7d989ebcdb3195cc6040025c732acf4338 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -1100,7 +1100,7 @@ static void get_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
 {
     Property *prop = opaque;
     QemuUUID *uuid = object_field_prop_ptr(obj, prop);
-    char buffer[UUID_FMT_LEN + 1];
+    char buffer[UUID_STR_LEN];
     char *p = buffer;
 
     qemu_uuid_unparse(uuid, buffer);
diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
index 271289f902f812ad1aeac3ee426249bba02a9d41..c64eaa5a46a04433dfc33313bbd4fdda8c619868 100644
--- a/hw/hyperv/vmbus.c
+++ b/hw/hyperv/vmbus.c
@@ -2271,7 +2271,7 @@ static void vmbus_dev_realize(DeviceState *dev, Error **errp)
     VMBus *vmbus = VMBUS(qdev_get_parent_bus(dev));
     BusChild *child;
     Error *err = NULL;
-    char idstr[UUID_FMT_LEN + 1];
+    char idstr[UUID_STR_LEN];
 
     assert(!qemu_uuid_is_null(&vdev->instanceid));
 
@@ -2467,7 +2467,7 @@ static char *vmbus_get_dev_path(DeviceState *dev)
 static char *vmbus_get_fw_dev_path(DeviceState *dev)
 {
     VMBusDevice *vdev = VMBUS_DEVICE(dev);
-    char uuid[UUID_FMT_LEN + 1];
+    char uuid[UUID_STR_LEN];
 
     qemu_uuid_unparse(&vdev->instanceid, uuid);
     return g_strdup_printf("%s@%s", qdev_fw_name(dev), uuid);
diff --git a/migration/savevm.c b/migration/savevm.c
index 8622f229e517f2ad8af80d3654146c16827be2e1..d5f3eafe3b15e289fd64ef5b6ded8bb3b1670596 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -469,8 +469,8 @@ static bool vmstate_uuid_needed(void *opaque)
 static int vmstate_uuid_post_load(void *opaque, int version_id)
 {
     SaveState *state = opaque;
-    char uuid_src[UUID_FMT_LEN + 1];
-    char uuid_dst[UUID_FMT_LEN + 1];
+    char uuid_src[UUID_STR_LEN];
+    char uuid_dst[UUID_STR_LEN];
 
     if (!qemu_uuid_set) {
         /*
diff --git a/tests/unit/test-uuid.c b/tests/unit/test-uuid.c
index aedc125ae98fb3a0b343603f2f0d022f4b8161c4..739b91583cfd97bb4d18256408338695fe87ef15 100644
--- a/tests/unit/test-uuid.c
+++ b/tests/unit/test-uuid.c
@@ -145,7 +145,7 @@ static void test_uuid_unparse(void)
     int i;
 
     for (i = 0; i < ARRAY_SIZE(uuid_test_data); i++) {
-        char out[37];
+        char out[UUID_STR_LEN];
 
         if (!uuid_test_data[i].check_unparse) {
             continue;
diff --git a/util/uuid.c b/util/uuid.c
index d71aa79e5ea433a9f3216b0b24d6276086607604..234619dd5e69a694d47bb299eb2536e5790b9863 100644
--- a/util/uuid.c
+++ b/util/uuid.c
@@ -51,7 +51,7 @@ int qemu_uuid_is_equal(const QemuUUID *lhv, const QemuUUID *rhv)
 void qemu_uuid_unparse(const QemuUUID *uuid, char *out)
 {
     const unsigned char *uu = &uuid->data[0];
-    snprintf(out, UUID_FMT_LEN + 1, UUID_FMT,
+    snprintf(out, UUID_STR_LEN, UUID_FMT,
              uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6], uu[7],
              uu[8], uu[9], uu[10], uu[11], uu[12], uu[13], uu[14], uu[15]);
 }
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26  7:06 [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
  2023-10-26  7:06 ` [PATCH v2 1/3] util/uuid: Add UUID_STR_LEN definition Cédric Le Goater
@ 2023-10-26  7:06 ` Cédric Le Goater
  2023-10-26 11:28   ` Peter Maydell
  2023-10-26  7:06 ` [PATCH v2 3/3] util/uuid: Remove UUID_FMT_LEN Cédric Le Goater
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Cédric Le Goater @ 2023-10-26  7:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras, Cédric Le Goater,
	Alex Williamson

qemu_uuid_unparse() includes a trailing NUL when writing the uuid
string and the buffer size should be UUID_FMT_LEN + 1 bytes. Use the
recently added UUID_STR_LEN which defines the correct size.

Fixes: CID 1522913
Fixes: 2dca1b37a760 ("vfio/pci: add support for VF token")
Cc: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 9bfa83aca1a87952e18743c9ca951b1bfc873507..c02a5d70f5e1b8e4d22051285748f514f1b9f008 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3274,7 +3274,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
     Error *err = NULL;
     int i, ret;
     bool is_mdev;
-    char uuid[UUID_FMT_LEN];
+    char uuid[UUID_STR_LEN];
     char *name;
 
     if (vbasedev->fd < 0 && !vbasedev->sysfsdev) {
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/3] util/uuid: Remove UUID_FMT_LEN
  2023-10-26  7:06 [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
  2023-10-26  7:06 ` [PATCH v2 1/3] util/uuid: Add UUID_STR_LEN definition Cédric Le Goater
  2023-10-26  7:06 ` [PATCH v2 2/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
@ 2023-10-26  7:06 ` Cédric Le Goater
  2023-10-26  7:18   ` Philippe Mathieu-Daudé
  2023-10-26 10:57   ` Juan Quintela
  2023-10-26  8:41 ` [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Denis V. Lunev
  2023-10-26 14:00 ` Cédric Le Goater
  4 siblings, 2 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-10-26  7:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras, Cédric Le Goater

Dangerous and now unused.

Cc: Fam Zheng <fam@euphon.net>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/qemu/uuid.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
index 4e7afaf1d5bd5d382fefbd6f6275d69cf25e7483..356efe7b5797911640ed347fc08f4ef5ebbd0476 100644
--- a/include/qemu/uuid.h
+++ b/include/qemu/uuid.h
@@ -78,8 +78,7 @@ typedef struct {
                  "%02hhx%02hhx-" \
                  "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
 
-#define UUID_FMT_LEN 36
-#define UUID_STR_LEN (UUID_FMT_LEN + 1)
+#define UUID_STR_LEN (36 + 1)
 
 #define UUID_NONE "00000000-0000-0000-0000-000000000000"
 
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/3] util/uuid: Remove UUID_FMT_LEN
  2023-10-26  7:06 ` [PATCH v2 3/3] util/uuid: Remove UUID_FMT_LEN Cédric Le Goater
@ 2023-10-26  7:18   ` Philippe Mathieu-Daudé
  2023-10-26 10:57   ` Juan Quintela
  1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-26  7:18 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras

On 26/10/23 09:06, Cédric Le Goater wrote:
> Dangerous and now unused.
> 
> Cc: Fam Zheng <fam@euphon.net>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   include/qemu/uuid.h | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
> index 4e7afaf1d5bd5d382fefbd6f6275d69cf25e7483..356efe7b5797911640ed347fc08f4ef5ebbd0476 100644
> --- a/include/qemu/uuid.h
> +++ b/include/qemu/uuid.h
> @@ -78,8 +78,7 @@ typedef struct {
>                    "%02hhx%02hhx-" \
>                    "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
>   
> -#define UUID_FMT_LEN 36
> -#define UUID_STR_LEN (UUID_FMT_LEN + 1)
> +#define UUID_STR_LEN (36 + 1)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>   
>   #define UUID_NONE "00000000-0000-0000-0000-000000000000"

but just noticing that ^^^ :) So I'd go with:

   QEMU_BUILD_BUG_ON(sizeof(UUID_NONE) - 1 != 36);
   #define UUID_STR_LEN sizeof(UUID_NONE)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26  7:06 [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
                   ` (2 preceding siblings ...)
  2023-10-26  7:06 ` [PATCH v2 3/3] util/uuid: Remove UUID_FMT_LEN Cédric Le Goater
@ 2023-10-26  8:41 ` Denis V. Lunev
  2023-10-26  9:58   ` Cédric Le Goater
  2023-10-26 13:42   ` Konstantin Ryabitsev
  2023-10-26 14:00 ` Cédric Le Goater
  4 siblings, 2 replies; 15+ messages in thread
From: Denis V. Lunev @ 2023-10-26  8:41 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras

On 10/26/23 09:06, Cédric Le Goater wrote:
> Hello,
>
> This series fixes a buffer overrun in VFIO. The buffer used in
> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
> one byte for the trailing NUL.
>
> Instead of adding + 1, as done elsewhere, the changes introduce a
> UUID_STR_LEN define for the correct size and use it where required.
>
> Thanks,
>
> C.
>
> Changes in v2:
>   - removal of UUID_FMT_LEN
>
> Cédric Le Goater (3):
>    util/uuid: Add UUID_STR_LEN definition
>    vfio/pci: Fix buffer overrun when writing the VF token
>    util/uuid: Remove UUID_FMT_LEN
>
>   include/qemu/uuid.h              | 2 +-
>   block/parallels-ext.c            | 2 +-
>   block/vdi.c                      | 2 +-
>   hw/core/qdev-properties-system.c | 2 +-
>   hw/hyperv/vmbus.c                | 4 ++--
>   hw/vfio/pci.c                    | 2 +-
>   migration/savevm.c               | 4 ++--
>   tests/unit/test-uuid.c           | 2 +-
>   util/uuid.c                      | 2 +-
>   9 files changed, 11 insertions(+), 11 deletions(-)
>
Reviwed-by: Denis V. Lunev <den@openvz.org>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26  8:41 ` [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Denis V. Lunev
@ 2023-10-26  9:58   ` Cédric Le Goater
  2023-10-26 13:42   ` Konstantin Ryabitsev
  1 sibling, 0 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-10-26  9:58 UTC (permalink / raw)
  To: Denis V. Lunev, qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras

On 10/26/23 10:41, Denis V. Lunev wrote:
> On 10/26/23 09:06, Cédric Le Goater wrote:
>> Hello,
>>
>> This series fixes a buffer overrun in VFIO. The buffer used in
>> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
>> one byte for the trailing NUL.
>>
>> Instead of adding + 1, as done elsewhere, the changes introduce a
>> UUID_STR_LEN define for the correct size and use it where required.
>>
>> Thanks,
>>
>> C.
>>
>> Changes in v2:
>>   - removal of UUID_FMT_LEN
>>
>> Cédric Le Goater (3):
>>    util/uuid: Add UUID_STR_LEN definition
>>    vfio/pci: Fix buffer overrun when writing the VF token
>>    util/uuid: Remove UUID_FMT_LEN
>>
>>   include/qemu/uuid.h              | 2 +-
>>   block/parallels-ext.c            | 2 +-
>>   block/vdi.c                      | 2 +-
>>   hw/core/qdev-properties-system.c | 2 +-
>>   hw/hyperv/vmbus.c                | 4 ++--
>>   hw/vfio/pci.c                    | 2 +-
>>   migration/savevm.c               | 4 ++--
>>   tests/unit/test-uuid.c           | 2 +-
>>   util/uuid.c                      | 2 +-
>>   9 files changed, 11 insertions(+), 11 deletions(-)
>>
> Reviwed-by: Denis V. Lunev <den@openvz.org>

I changed that to "Reviewed-by".

Interesting to see that b4 was ok with this new tag.

Thanks,

C.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/3] util/uuid: Remove UUID_FMT_LEN
  2023-10-26  7:06 ` [PATCH v2 3/3] util/uuid: Remove UUID_FMT_LEN Cédric Le Goater
  2023-10-26  7:18   ` Philippe Mathieu-Daudé
@ 2023-10-26 10:57   ` Juan Quintela
  1 sibling, 0 replies; 15+ messages in thread
From: Juan Quintela @ 2023-10-26 10:57 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: qemu-devel, Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf,
	Hanna Reitz, Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Peter Xu,
	Fabiano Rosas, Leonardo Bras

Cédric Le Goater <clg@redhat.com> wrote:
> Dangerous and now unused.
>
> Cc: Fam Zheng <fam@euphon.net>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26  7:06 ` [PATCH v2 2/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
@ 2023-10-26 11:28   ` Peter Maydell
  2023-10-26 13:33     ` Cédric Le Goater
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2023-10-26 11:28 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: qemu-devel, Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf,
	Hanna Reitz, Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras, Alex Williamson

On Thu, 26 Oct 2023 at 08:08, Cédric Le Goater <clg@redhat.com> wrote:
>
> qemu_uuid_unparse() includes a trailing NUL when writing the uuid
> string and the buffer size should be UUID_FMT_LEN + 1 bytes. Use the
> recently added UUID_STR_LEN which defines the correct size.
>
> Fixes: CID 1522913
> Fixes: 2dca1b37a760 ("vfio/pci: add support for VF token")
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>  hw/vfio/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 9bfa83aca1a87952e18743c9ca951b1bfc873507..c02a5d70f5e1b8e4d22051285748f514f1b9f008 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3274,7 +3274,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>      Error *err = NULL;
>      int i, ret;
>      bool is_mdev;
> -    char uuid[UUID_FMT_LEN];
> +    char uuid[UUID_STR_LEN];
>      char *name;
>
>      if (vbasedev->fd < 0 && !vbasedev->sysfsdev) {

This patch (and its dependency) should probably be cc qemu-stable ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26 11:28   ` Peter Maydell
@ 2023-10-26 13:33     ` Cédric Le Goater
  0 siblings, 0 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-10-26 13:33 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel, Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf,
	Hanna Reitz, Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras, Alex Williamson

On 10/26/23 13:28, Peter Maydell wrote:
> On Thu, 26 Oct 2023 at 08:08, Cédric Le Goater <clg@redhat.com> wrote:
>>
>> qemu_uuid_unparse() includes a trailing NUL when writing the uuid
>> string and the buffer size should be UUID_FMT_LEN + 1 bytes. Use the
>> recently added UUID_STR_LEN which defines the correct size.
>>
>> Fixes: CID 1522913
>> Fixes: 2dca1b37a760 ("vfio/pci: add support for VF token")
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   hw/vfio/pci.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 9bfa83aca1a87952e18743c9ca951b1bfc873507..c02a5d70f5e1b8e4d22051285748f514f1b9f008 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -3274,7 +3274,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>>       Error *err = NULL;
>>       int i, ret;
>>       bool is_mdev;
>> -    char uuid[UUID_FMT_LEN];
>> +    char uuid[UUID_STR_LEN];
>>       char *name;
>>
>>       if (vbasedev->fd < 0 && !vbasedev->sysfsdev) {
> 
> This patch (and its dependency) should probably be cc qemu-stable ?

yes. 8.1 is impacted. I was waiting for some feedback.

Thanks,

C.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26  8:41 ` [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Denis V. Lunev
  2023-10-26  9:58   ` Cédric Le Goater
@ 2023-10-26 13:42   ` Konstantin Ryabitsev
  2023-10-26 15:36     ` Peter Maydell
  1 sibling, 1 reply; 15+ messages in thread
From: Konstantin Ryabitsev @ 2023-10-26 13:42 UTC (permalink / raw)
  To: Cédric Le Goater, Denis V. Lunev, qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras

October 26, 2023 at 5:58 AM, "Cédric Le Goater" <clg@redhat.com> wrote:
> >  Reviwed-by: Denis V. Lunev <den@openvz.org>
> > 
> 
> I changed that to "Reviewed-by".
> 
> Interesting to see that b4 was ok with this new tag.

When we see an email address in the trailer contents, we don't check it against a known-trailers list, because there are just too many things like "Co-developed-by", "Reviewed-and-acked-by", etc. We could add some kind of logic to break these apart and compare individual parts to a list of known person-trailers (e.g. ["co", "reviewed", "developed", "and", "by", ...]), but we currently don't, which is why typos like this one sneak through.

-K


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26  7:06 [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
                   ` (3 preceding siblings ...)
  2023-10-26  8:41 ` [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Denis V. Lunev
@ 2023-10-26 14:00 ` Cédric Le Goater
  2023-10-27  5:01   ` Philippe Mathieu-Daudé
  2023-10-30  8:59   ` Cédric Le Goater
  4 siblings, 2 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-10-26 14:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras, qemu-stable,
	Michael Tokarev

On 10/26/23 09:06, Cédric Le Goater wrote:
> Hello,
> 
> This series fixes a buffer overrun in VFIO. The buffer used in
> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
> one byte for the trailing NUL.
> 
> Instead of adding + 1, as done elsewhere, the changes introduce a
> UUID_STR_LEN define for the correct size and use it where required.

Cc: qemu-stable@nongnu.org # 8.1+

I propose to take this series in vfio-next if no one objects.

Thanks,

C.


> 
> Thanks,
> 
> C.
> 
> Changes in v2:
>   - removal of UUID_FMT_LEN
> 
> Cédric Le Goater (3):
>    util/uuid: Add UUID_STR_LEN definition
>    vfio/pci: Fix buffer overrun when writing the VF token
>    util/uuid: Remove UUID_FMT_LEN
> 
>   include/qemu/uuid.h              | 2 +-
>   block/parallels-ext.c            | 2 +-
>   block/vdi.c                      | 2 +-
>   hw/core/qdev-properties-system.c | 2 +-
>   hw/hyperv/vmbus.c                | 4 ++--
>   hw/vfio/pci.c                    | 2 +-
>   migration/savevm.c               | 4 ++--
>   tests/unit/test-uuid.c           | 2 +-
>   util/uuid.c                      | 2 +-
>   9 files changed, 11 insertions(+), 11 deletions(-)
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26 13:42   ` Konstantin Ryabitsev
@ 2023-10-26 15:36     ` Peter Maydell
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2023-10-26 15:36 UTC (permalink / raw)
  To: Konstantin Ryabitsev
  Cc: Cédric Le Goater, Denis V. Lunev, qemu-devel,
	Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras

On Thu, 26 Oct 2023 at 16:06, Konstantin Ryabitsev
<konstantin.ryabitsev@linux.dev> wrote:
>
> October 26, 2023 at 5:58 AM, "Cédric Le Goater" <clg@redhat.com> wrote:
> > >  Reviwed-by: Denis V. Lunev <den@openvz.org>
> > >
> >
> > I changed that to "Reviewed-by".
> >
> > Interesting to see that b4 was ok with this new tag.
>
> When we see an email address in the trailer contents, we don't check it against a known-trailers list, because there are just too many things like "Co-developed-by", "Reviewed-and-acked-by", etc. We could add some kind of logic to break these apart and compare individual parts to a list of known person-trailers (e.g. ["co", "reviewed", "developed", "and", "by", ...]), but we currently don't, which is why typos like this one sneak through.

From the QEMU development perspective, I would be mildly in favour
of having checkpatch at least warn about unusual trailers, because
I don't think the profusion of oddball stuff is actually helpful,
and nudging towards standardization and guarding against typos in
the tag string would be better (eg if you want to do both a review
and an ack, provide both tags, not an -and- tag that no tooling
that might be asking "did this get review?" will be looking for).
But I don't care enough to have actually looked at getting our
checkpatch to do this :-)

-- PMM


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26 14:00 ` Cédric Le Goater
@ 2023-10-27  5:01   ` Philippe Mathieu-Daudé
  2023-10-30  8:59   ` Cédric Le Goater
  1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-27  5:01 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras, qemu-stable,
	Michael Tokarev

On 26/10/23 16:00, Cédric Le Goater wrote:
> On 10/26/23 09:06, Cédric Le Goater wrote:
>> Hello,
>>
>> This series fixes a buffer overrun in VFIO. The buffer used in
>> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
>> one byte for the trailing NUL.
>>
>> Instead of adding + 1, as done elsewhere, the changes introduce a
>> UUID_STR_LEN define for the correct size and use it where required.
> 
> Cc: qemu-stable@nongnu.org # 8.1+

Hopefully 8.2 shouldn't be affected ;)

> 
> I propose to take this series in vfio-next if no one objects.
> 
> Thanks,
> 
> C.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
  2023-10-26 14:00 ` Cédric Le Goater
  2023-10-27  5:01   ` Philippe Mathieu-Daudé
@ 2023-10-30  8:59   ` Cédric Le Goater
  1 sibling, 0 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-10-30  8:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Denis V . Lunev, Kevin Wolf, Hanna Reitz,
	Stefan Weil, Paolo Bonzini, Daniel P . Berrangé,
	Eduardo Habkost, Maciej S . Szmigiero, Fam Zheng, Juan Quintela,
	Peter Xu, Fabiano Rosas, Leonardo Bras, qemu-stable,
	Michael Tokarev

On 10/26/23 16:00, Cédric Le Goater wrote:
> On 10/26/23 09:06, Cédric Le Goater wrote:
>> Hello,
>>
>> This series fixes a buffer overrun in VFIO. The buffer used in
>> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
>> one byte for the trailing NUL.
>>
>> Instead of adding + 1, as done elsewhere, the changes introduce a
>> UUID_STR_LEN define for the correct size and use it where required.
> 
> Cc: qemu-stable@nongnu.org # 8.1+
> 
> I propose to take this series in vfio-next if no one objects.


Applied to vfio-next.

Thanks,

C.



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-10-30  9:00 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-26  7:06 [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
2023-10-26  7:06 ` [PATCH v2 1/3] util/uuid: Add UUID_STR_LEN definition Cédric Le Goater
2023-10-26  7:06 ` [PATCH v2 2/3] vfio/pci: Fix buffer overrun when writing the VF token Cédric Le Goater
2023-10-26 11:28   ` Peter Maydell
2023-10-26 13:33     ` Cédric Le Goater
2023-10-26  7:06 ` [PATCH v2 3/3] util/uuid: Remove UUID_FMT_LEN Cédric Le Goater
2023-10-26  7:18   ` Philippe Mathieu-Daudé
2023-10-26 10:57   ` Juan Quintela
2023-10-26  8:41 ` [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token Denis V. Lunev
2023-10-26  9:58   ` Cédric Le Goater
2023-10-26 13:42   ` Konstantin Ryabitsev
2023-10-26 15:36     ` Peter Maydell
2023-10-26 14:00 ` Cédric Le Goater
2023-10-27  5:01   ` Philippe Mathieu-Daudé
2023-10-30  8:59   ` Cédric Le Goater

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.