diff mbox series

[dunfell,07/16] QEMU: CVE-2022-4144 QXL: qxl_phys2virt unsafe address translation can lead to out-of-bounds read

Message ID 7fdb46a83e117459780d5cd0997b0666b7b1a081.1674657501.git.steve@sakoman.com
State New, archived
Headers show
Series [dunfell,01/16] cve-update-db-native: Allow to overrule the URL in a bbappend. | expand

Commit Message

Steve Sakoman Jan. 25, 2023, 2:41 p.m. UTC 
From: Hitendra Prajapati <hprajapati@mvista.com>

Upstream-Status: Backport from https://gitlab.com/qemu-project/qemu/-/commit/6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622

Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
 meta/recipes-devtools/qemu/qemu.inc           |   1 +
 .../qemu/qemu/CVE-2022-4144.patch             | 103 ++++++++++++++++++
 2 files changed, 104 insertions(+)
 create mode 100644 meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch

Comments

Martin Jansa Jan. 31, 2023, 7:18 a.m. UTC | #1 
I haven't checked yet in dunfell, but in kirkstone this commit merged last
week breaks nativesdk-qemu builds.

This chunk:
+@@ -1486,7 +1505,7 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL
pqxl, int group_id)
+         offset = le64_to_cpu(pqxl) & 0xffffffffffff;
+         return (void *)(intptr_t)offset;
+     case MEMSLOT_GROUP_GUEST:
+-        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset)) {
++        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size)) {
+             return NULL;
+         }
+         ptr = memory_region_get_ram_ptr(qxl->guest_slots[slot].mr);

Uses size parameter which was added in previous commit (which wasn't
backported):
https://gitlab.com/qemu-project/qemu/-/commit/8efec0ef8bbc1e75a7ebf6e325a35806ece9b39f

So either both commits need to be backported or this one reworked not to
use undeclared size as it leads to:

| ../qemu-6.2.0/hw/display/qxl.c: In function 'qxl_phys2virt':
| ../qemu-6.2.0/hw/display/qxl.c:1477:67: error: 'size' undeclared (first
use in this function); did you mean 'gsize'?
|  1477 |         if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset,
size)) {
|       |
^~~~
|       |
gsize

I'm surprised this wasn't caught on autobuilder already, maybe it's
triggered only with extra PACKAGECONFIG options we have enabled:
PACKAGECONFIG:append:class-nativesdk = " virglrenderer epoxy spice libusb
usb-redir"

Regards,


On Wed, Jan 25, 2023 at 3:42 PM Steve Sakoman <steve@sakoman.com> wrote:

> From: Hitendra Prajapati <hprajapati@mvista.com>
>
> Upstream-Status: Backport from
> https://gitlab.com/qemu-project/qemu/-/commit/6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622
>
> Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
> Signed-off-by: Steve Sakoman <steve@sakoman.com>
> ---
>  meta/recipes-devtools/qemu/qemu.inc           |   1 +
>  .../qemu/qemu/CVE-2022-4144.patch             | 103 ++++++++++++++++++
>  2 files changed, 104 insertions(+)
>  create mode 100644 meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
>
> diff --git a/meta/recipes-devtools/qemu/qemu.inc
> b/meta/recipes-devtools/qemu/qemu.inc
> index fff2c87780..898fa1a8d8 100644
> --- a/meta/recipes-devtools/qemu/qemu.inc
> +++ b/meta/recipes-devtools/qemu/qemu.inc
> @@ -115,6 +115,7 @@ SRC_URI = "
> https://download.qemu.org/${BPN}-${PV}.tar.xz \
>            file://CVE-2021-3638.patch \
>            file://CVE-2021-20196.patch \
>            file://CVE-2021-3507.patch \
> +          file://CVE-2022-4144.patch \
>             "
>  UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar"
>
> diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
> b/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
> new file mode 100644
> index 0000000000..3f0d5fbd5c
> --- /dev/null
> +++ b/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
> @@ -0,0 +1,103 @@
> +From 6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= <philmd@linaro.org>
> +Date: Mon, 28 Nov 2022 21:27:40 +0100
> +Subject: [PATCH] hw/display/qxl: Avoid buffer overrun in qxl_phys2virt
> + (CVE-2022-4144)
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Have qxl_get_check_slot_offset() return false if the requested
> +buffer size does not fit within the slot memory region.
> +
> +Similarly qxl_phys2virt() now returns NULL in such case, and
> +qxl_dirty_one_surface() aborts.
> +
> +This avoids buffer overrun in the host pointer returned by
> +memory_region_get_ram_ptr().
> +
> +Fixes: CVE-2022-4144 (out-of-bounds read)
> +Reported-by: Wenxu Yin (@awxylitol)
> +Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1336
> +
> +Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> +Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> +Message-Id: <20221128202741.4945-5-philmd@linaro.org>
> +
> +Upstream-Status: Backport [
> https://gitlab.com/qemu-project/qemu/-/commit/6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622
> ]
> +CVE: CVE-2022-4144
> +Comments: Deleted patch hunk in qxl.h,as it contains change
> +in comments which is not present in current version of qemu.
> +
> +Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
> +---
> + hw/display/qxl.c | 27 +++++++++++++++++++++++----
> + 1 file changed, 23 insertions(+), 4 deletions(-)
> +
> +diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> +index cd7eb39d..6bc8385b 100644
> +--- a/hw/display/qxl.c
> ++++ b/hw/display/qxl.c
> +@@ -1440,11 +1440,13 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
> +
> + /* can be also called from spice server thread context */
> + static bool qxl_get_check_slot_offset(PCIQXLDevice *qxl, QXLPHYSICAL
> pqxl,
> +-                                      uint32_t *s, uint64_t *o)
> ++                                      uint32_t *s, uint64_t *o,
> ++                                      size_t size_requested)
> + {
> +     uint64_t phys   = le64_to_cpu(pqxl);
> +     uint32_t slot   = (phys >> (64 -  8)) & 0xff;
> +     uint64_t offset = phys & 0xffffffffffff;
> ++    uint64_t size_available;
> +
> +     if (slot >= NUM_MEMSLOTS) {
> +         qxl_set_guest_bug(qxl, "slot too large %d >= %d", slot,
> +@@ -1468,6 +1470,23 @@ static bool qxl_get_check_slot_offset(PCIQXLDevice
> *qxl, QXLPHYSICAL pqxl,
> +                           slot, offset, qxl->guest_slots[slot].size);
> +         return false;
> +     }
> ++    size_available = memory_region_size(qxl->guest_slots[slot].mr);
> ++    if (qxl->guest_slots[slot].offset + offset >= size_available) {
> ++        qxl_set_guest_bug(qxl,
> ++                          "slot %d offset %"PRIu64" > region size
> %"PRIu64"\n",
> ++                          slot, qxl->guest_slots[slot].offset + offset,
> ++                          size_available);
> ++        return false;
> ++    }
> ++    size_available -= qxl->guest_slots[slot].offset + offset;
> ++    if (size_requested > size_available) {
> ++        qxl_set_guest_bug(qxl,
> ++                          "slot %d offset %"PRIu64" size %zu: "
> ++                          "overrun by %"PRIu64" bytes\n",
> ++                          slot, offset, size_requested,
> ++                          size_requested - size_available);
> ++        return false;
> ++    }
> +
> +     *s = slot;
> +     *o = offset;
> +@@ -1486,7 +1505,7 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL
> pqxl, int group_id)
> +         offset = le64_to_cpu(pqxl) & 0xffffffffffff;
> +         return (void *)(intptr_t)offset;
> +     case MEMSLOT_GROUP_GUEST:
> +-        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset)) {
> ++        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size))
> {
> +             return NULL;
> +         }
> +         ptr = memory_region_get_ram_ptr(qxl->guest_slots[slot].mr);
> +@@ -1944,9 +1963,9 @@ static void qxl_dirty_one_surface(PCIQXLDevice
> *qxl, QXLPHYSICAL pqxl,
> +     uint32_t slot;
> +     bool rc;
> +
> +-    rc = qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset);
> +-    assert(rc == true);
> +     size = (uint64_t)height * abs(stride);
> ++    rc = qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size);
> ++    assert(rc == true);
> +     trace_qxl_surfaces_dirty(qxl->id, offset, size);
> +     qxl_set_dirty(qxl->guest_slots[slot].mr,
> +                   qxl->guest_slots[slot].offset + offset,
> +--
> +2.25.1
> +
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#176358):
> https://lists.openembedded.org/g/openembedded-core/message/176358
> Mute This Topic: https://lists.openembedded.org/mt/96521255/3617156
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> Martin.Jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Martin Jansa Jan. 31, 2023, 7:39 a.m. UTC | #2 
On Tue, Jan 31, 2023 at 8:18 AM Martin Jansa via lists.openembedded.org
<Martin.Jansa=gmail.com@lists.openembedded.org> wrote:

> I haven't checked yet in dunfell, but in kirkstone this commit merged last
> week breaks nativesdk-qemu builds.
>

I can confirm it fails the same with this commit from Hitendra in dunfell:

nativesdk-qemu/4.2.0-r0/qemu-4.2.0/hw/display/qxl.c:1508:67: error: 'size'
undeclared (first use in this function); did you mean 'gsize'?
|  1508 |         if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset,
size)) {
|       |
^~~~
|       |
gsize

as well as the same backport from Bhabu in kirkstone (already merged):
https://git.openembedded.org/openembedded-core/commit/?h=kirkstone&id=4cb3874abf4fdeb04337a48a14c765ba9b2269d4

To reproduce this just enable "spice" PACKAGECONFIG in nativesdk-qemu.

Hitendra, Bhabu: please have a look.

Regards,


>
> This chunk:
> +@@ -1486,7 +1505,7 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL
> pqxl, int group_id)
> +         offset = le64_to_cpu(pqxl) & 0xffffffffffff;
> +         return (void *)(intptr_t)offset;
> +     case MEMSLOT_GROUP_GUEST:
> +-        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset)) {
> ++        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size))
> {
> +             return NULL;
> +         }
> +         ptr = memory_region_get_ram_ptr(qxl->guest_slots[slot].mr);
>
> Uses size parameter which was added in previous commit (which wasn't
> backported):
>
> https://gitlab.com/qemu-project/qemu/-/commit/8efec0ef8bbc1e75a7ebf6e325a35806ece9b39f
>
> So either both commits need to be backported or this one reworked not to
> use undeclared size as it leads to:
>
> | ../qemu-6.2.0/hw/display/qxl.c: In function 'qxl_phys2virt':
> | ../qemu-6.2.0/hw/display/qxl.c:1477:67: error: 'size' undeclared (first
> use in this function); did you mean 'gsize'?
> |  1477 |         if (!qxl_get_check_slot_offset(qxl, pqxl, &slot,
> &offset, size)) {
> |       |
>   ^~~~
> |       |
>   gsize
>
> I'm surprised this wasn't caught on autobuilder already, maybe it's
> triggered only with extra PACKAGECONFIG options we have enabled:
> PACKAGECONFIG:append:class-nativesdk = " virglrenderer epoxy spice libusb
> usb-redir"
>
> Regards,
>
>
> On Wed, Jan 25, 2023 at 3:42 PM Steve Sakoman <steve@sakoman.com> wrote:
>
>> From: Hitendra Prajapati <hprajapati@mvista.com>
>>
>> Upstream-Status: Backport from
>> https://gitlab.com/qemu-project/qemu/-/commit/6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622
>>
>> Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
>> Signed-off-by: Steve Sakoman <steve@sakoman.com>
>> ---
>>  meta/recipes-devtools/qemu/qemu.inc           |   1 +
>>  .../qemu/qemu/CVE-2022-4144.patch             | 103 ++++++++++++++++++
>>  2 files changed, 104 insertions(+)
>>  create mode 100644 meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
>>
>> diff --git a/meta/recipes-devtools/qemu/qemu.inc
>> b/meta/recipes-devtools/qemu/qemu.inc
>> index fff2c87780..898fa1a8d8 100644
>> --- a/meta/recipes-devtools/qemu/qemu.inc
>> +++ b/meta/recipes-devtools/qemu/qemu.inc
>> @@ -115,6 +115,7 @@ SRC_URI = "
>> https://download.qemu.org/${BPN}-${PV}.tar.xz \
>>            file://CVE-2021-3638.patch \
>>            file://CVE-2021-20196.patch \
>>            file://CVE-2021-3507.patch \
>> +          file://CVE-2022-4144.patch \
>>             "
>>  UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar"
>>
>> diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
>> b/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
>> new file mode 100644
>> index 0000000000..3f0d5fbd5c
>> --- /dev/null
>> +++ b/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
>> @@ -0,0 +1,103 @@
>> +From 6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622 Mon Sep 17 00:00:00 2001
>> +From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= <philmd@linaro.org>
>> +Date: Mon, 28 Nov 2022 21:27:40 +0100
>> +Subject: [PATCH] hw/display/qxl: Avoid buffer overrun in qxl_phys2virt
>> + (CVE-2022-4144)
>> +MIME-Version: 1.0
>> +Content-Type: text/plain; charset=UTF-8
>> +Content-Transfer-Encoding: 8bit
>> +
>> +Have qxl_get_check_slot_offset() return false if the requested
>> +buffer size does not fit within the slot memory region.
>> +
>> +Similarly qxl_phys2virt() now returns NULL in such case, and
>> +qxl_dirty_one_surface() aborts.
>> +
>> +This avoids buffer overrun in the host pointer returned by
>> +memory_region_get_ram_ptr().
>> +
>> +Fixes: CVE-2022-4144 (out-of-bounds read)
>> +Reported-by: Wenxu Yin (@awxylitol)
>> +Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1336
>> +
>> +Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> +Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> +Message-Id: <20221128202741.4945-5-philmd@linaro.org>
>> +
>> +Upstream-Status: Backport [
>> https://gitlab.com/qemu-project/qemu/-/commit/6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622
>> ]
>> +CVE: CVE-2022-4144
>> +Comments: Deleted patch hunk in qxl.h,as it contains change
>> +in comments which is not present in current version of qemu.
>> +
>> +Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
>> +---
>> + hw/display/qxl.c | 27 +++++++++++++++++++++++----
>> + 1 file changed, 23 insertions(+), 4 deletions(-)
>> +
>> +diff --git a/hw/display/qxl.c b/hw/display/qxl.c
>> +index cd7eb39d..6bc8385b 100644
>> +--- a/hw/display/qxl.c
>> ++++ b/hw/display/qxl.c
>> +@@ -1440,11 +1440,13 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
>> +
>> + /* can be also called from spice server thread context */
>> + static bool qxl_get_check_slot_offset(PCIQXLDevice *qxl, QXLPHYSICAL
>> pqxl,
>> +-                                      uint32_t *s, uint64_t *o)
>> ++                                      uint32_t *s, uint64_t *o,
>> ++                                      size_t size_requested)
>> + {
>> +     uint64_t phys   = le64_to_cpu(pqxl);
>> +     uint32_t slot   = (phys >> (64 -  8)) & 0xff;
>> +     uint64_t offset = phys & 0xffffffffffff;
>> ++    uint64_t size_available;
>> +
>> +     if (slot >= NUM_MEMSLOTS) {
>> +         qxl_set_guest_bug(qxl, "slot too large %d >= %d", slot,
>> +@@ -1468,6 +1470,23 @@ static bool
>> qxl_get_check_slot_offset(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
>> +                           slot, offset, qxl->guest_slots[slot].size);
>> +         return false;
>> +     }
>> ++    size_available = memory_region_size(qxl->guest_slots[slot].mr);
>> ++    if (qxl->guest_slots[slot].offset + offset >= size_available) {
>> ++        qxl_set_guest_bug(qxl,
>> ++                          "slot %d offset %"PRIu64" > region size
>> %"PRIu64"\n",
>> ++                          slot, qxl->guest_slots[slot].offset + offset,
>> ++                          size_available);
>> ++        return false;
>> ++    }
>> ++    size_available -= qxl->guest_slots[slot].offset + offset;
>> ++    if (size_requested > size_available) {
>> ++        qxl_set_guest_bug(qxl,
>> ++                          "slot %d offset %"PRIu64" size %zu: "
>> ++                          "overrun by %"PRIu64" bytes\n",
>> ++                          slot, offset, size_requested,
>> ++                          size_requested - size_available);
>> ++        return false;
>> ++    }
>> +
>> +     *s = slot;
>> +     *o = offset;
>> +@@ -1486,7 +1505,7 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL
>> pqxl, int group_id)
>> +         offset = le64_to_cpu(pqxl) & 0xffffffffffff;
>> +         return (void *)(intptr_t)offset;
>> +     case MEMSLOT_GROUP_GUEST:
>> +-        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset)) {
>> ++        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset,
>> size)) {
>> +             return NULL;
>> +         }
>> +         ptr = memory_region_get_ram_ptr(qxl->guest_slots[slot].mr);
>> +@@ -1944,9 +1963,9 @@ static void qxl_dirty_one_surface(PCIQXLDevice
>> *qxl, QXLPHYSICAL pqxl,
>> +     uint32_t slot;
>> +     bool rc;
>> +
>> +-    rc = qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset);
>> +-    assert(rc == true);
>> +     size = (uint64_t)height * abs(stride);
>> ++    rc = qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size);
>> ++    assert(rc == true);
>> +     trace_qxl_surfaces_dirty(qxl->id, offset, size);
>> +     qxl_set_dirty(qxl->guest_slots[slot].mr,
>> +                   qxl->guest_slots[slot].offset + offset,
>> +--
>> +2.25.1
>> +
>> --
>> 2.25.1
>>
>>
>>
>>
>>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#176506):
> https://lists.openembedded.org/g/openembedded-core/message/176506
> Mute This Topic: https://lists.openembedded.org/mt/96521255/3617156
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> Martin.Jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Steve Sakoman Feb. 3, 2023, 3:59 p.m. UTC | #3 
On Mon, Jan 30, 2023 at 9:39 PM Martin Jansa <Martin.Jansa@gmail.com> wrote:
>
> On Tue, Jan 31, 2023 at 8:18 AM Martin Jansa via lists.openembedded.org <Martin.Jansa=gmail.com@lists.openembedded.org> wrote:
>>
>> I haven't checked yet in dunfell, but in kirkstone this commit merged last week breaks nativesdk-qemu builds.
>
>
> I can confirm it fails the same with this commit from Hitendra in dunfell:
>
> nativesdk-qemu/4.2.0-r0/qemu-4.2.0/hw/display/qxl.c:1508:67: error: 'size' undeclared (first use in this function); did you mean 'gsize'?
> |  1508 |         if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size)) {
> |       |                                                                   ^~~~
> |       |                                                                   gsize
>
> as well as the same backport from Bhabu in kirkstone (already merged):
> https://git.openembedded.org/openembedded-core/commit/?h=kirkstone&id=4cb3874abf4fdeb04337a48a14c765ba9b2269d4
>
> To reproduce this just enable "spice" PACKAGECONFIG in nativesdk-qemu.
>
> Hitendra, Bhabu: please have a look.

Yes, please do!  Unless I get a fix for the spice regression sometime
soon I will revert the patch in kirkstone and of course won't take the
dunfell version.

Steve

>> This chunk:
>> +@@ -1486,7 +1505,7 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
>> +         offset = le64_to_cpu(pqxl) & 0xffffffffffff;
>> +         return (void *)(intptr_t)offset;
>> +     case MEMSLOT_GROUP_GUEST:
>> +-        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset)) {
>> ++        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size)) {
>> +             return NULL;
>> +         }
>> +         ptr = memory_region_get_ram_ptr(qxl->guest_slots[slot].mr);
>>
>> Uses size parameter which was added in previous commit (which wasn't backported):
>> https://gitlab.com/qemu-project/qemu/-/commit/8efec0ef8bbc1e75a7ebf6e325a35806ece9b39f
>>
>> So either both commits need to be backported or this one reworked not to use undeclared size as it leads to:
>>
>> | ../qemu-6.2.0/hw/display/qxl.c: In function 'qxl_phys2virt':
>> | ../qemu-6.2.0/hw/display/qxl.c:1477:67: error: 'size' undeclared (first use in this function); did you mean 'gsize'?
>> |  1477 |         if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size)) {
>> |       |                                                                   ^~~~
>> |       |                                                                   gsize
>>
>> I'm surprised this wasn't caught on autobuilder already, maybe it's triggered only with extra PACKAGECONFIG options we have enabled:
>> PACKAGECONFIG:append:class-nativesdk = " virglrenderer epoxy spice libusb usb-redir"
>>
>> Regards,
>>
>>
>> On Wed, Jan 25, 2023 at 3:42 PM Steve Sakoman <steve@sakoman.com> wrote:
>>>
>>> From: Hitendra Prajapati <hprajapati@mvista.com>
>>>
>>> Upstream-Status: Backport from https://gitlab.com/qemu-project/qemu/-/commit/6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622
>>>
>>> Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
>>> Signed-off-by: Steve Sakoman <steve@sakoman.com>
>>> ---
>>>  meta/recipes-devtools/qemu/qemu.inc           |   1 +
>>>  .../qemu/qemu/CVE-2022-4144.patch             | 103 ++++++++++++++++++
>>>  2 files changed, 104 insertions(+)
>>>  create mode 100644 meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
>>>
>>> diff --git a/meta/recipes-devtools/qemu/qemu.inc b/meta/recipes-devtools/qemu/qemu.inc
>>> index fff2c87780..898fa1a8d8 100644
>>> --- a/meta/recipes-devtools/qemu/qemu.inc
>>> +++ b/meta/recipes-devtools/qemu/qemu.inc
>>> @@ -115,6 +115,7 @@ SRC_URI = "https://download.qemu.org/${BPN}-${PV}.tar.xz \
>>>            file://CVE-2021-3638.patch \
>>>            file://CVE-2021-20196.patch \
>>>            file://CVE-2021-3507.patch \
>>> +          file://CVE-2022-4144.patch \
>>>             "
>>>  UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar"
>>>
>>> diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch b/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
>>> new file mode 100644
>>> index 0000000000..3f0d5fbd5c
>>> --- /dev/null
>>> +++ b/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
>>> @@ -0,0 +1,103 @@
>>> +From 6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622 Mon Sep 17 00:00:00 2001
>>> +From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= <philmd@linaro.org>
>>> +Date: Mon, 28 Nov 2022 21:27:40 +0100
>>> +Subject: [PATCH] hw/display/qxl: Avoid buffer overrun in qxl_phys2virt
>>> + (CVE-2022-4144)
>>> +MIME-Version: 1.0
>>> +Content-Type: text/plain; charset=UTF-8
>>> +Content-Transfer-Encoding: 8bit
>>> +
>>> +Have qxl_get_check_slot_offset() return false if the requested
>>> +buffer size does not fit within the slot memory region.
>>> +
>>> +Similarly qxl_phys2virt() now returns NULL in such case, and
>>> +qxl_dirty_one_surface() aborts.
>>> +
>>> +This avoids buffer overrun in the host pointer returned by
>>> +memory_region_get_ram_ptr().
>>> +
>>> +Fixes: CVE-2022-4144 (out-of-bounds read)
>>> +Reported-by: Wenxu Yin (@awxylitol)
>>> +Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1336
>>> +
>>> +Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> +Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> +Message-Id: <20221128202741.4945-5-philmd@linaro.org>
>>> +
>>> +Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622]
>>> +CVE: CVE-2022-4144
>>> +Comments: Deleted patch hunk in qxl.h,as it contains change
>>> +in comments which is not present in current version of qemu.
>>> +
>>> +Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
>>> +---
>>> + hw/display/qxl.c | 27 +++++++++++++++++++++++----
>>> + 1 file changed, 23 insertions(+), 4 deletions(-)
>>> +
>>> +diff --git a/hw/display/qxl.c b/hw/display/qxl.c
>>> +index cd7eb39d..6bc8385b 100644
>>> +--- a/hw/display/qxl.c
>>> ++++ b/hw/display/qxl.c
>>> +@@ -1440,11 +1440,13 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
>>> +
>>> + /* can be also called from spice server thread context */
>>> + static bool qxl_get_check_slot_offset(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
>>> +-                                      uint32_t *s, uint64_t *o)
>>> ++                                      uint32_t *s, uint64_t *o,
>>> ++                                      size_t size_requested)
>>> + {
>>> +     uint64_t phys   = le64_to_cpu(pqxl);
>>> +     uint32_t slot   = (phys >> (64 -  8)) & 0xff;
>>> +     uint64_t offset = phys & 0xffffffffffff;
>>> ++    uint64_t size_available;
>>> +
>>> +     if (slot >= NUM_MEMSLOTS) {
>>> +         qxl_set_guest_bug(qxl, "slot too large %d >= %d", slot,
>>> +@@ -1468,6 +1470,23 @@ static bool qxl_get_check_slot_offset(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
>>> +                           slot, offset, qxl->guest_slots[slot].size);
>>> +         return false;
>>> +     }
>>> ++    size_available = memory_region_size(qxl->guest_slots[slot].mr);
>>> ++    if (qxl->guest_slots[slot].offset + offset >= size_available) {
>>> ++        qxl_set_guest_bug(qxl,
>>> ++                          "slot %d offset %"PRIu64" > region size %"PRIu64"\n",
>>> ++                          slot, qxl->guest_slots[slot].offset + offset,
>>> ++                          size_available);
>>> ++        return false;
>>> ++    }
>>> ++    size_available -= qxl->guest_slots[slot].offset + offset;
>>> ++    if (size_requested > size_available) {
>>> ++        qxl_set_guest_bug(qxl,
>>> ++                          "slot %d offset %"PRIu64" size %zu: "
>>> ++                          "overrun by %"PRIu64" bytes\n",
>>> ++                          slot, offset, size_requested,
>>> ++                          size_requested - size_available);
>>> ++        return false;
>>> ++    }
>>> +
>>> +     *s = slot;
>>> +     *o = offset;
>>> +@@ -1486,7 +1505,7 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
>>> +         offset = le64_to_cpu(pqxl) & 0xffffffffffff;
>>> +         return (void *)(intptr_t)offset;
>>> +     case MEMSLOT_GROUP_GUEST:
>>> +-        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset)) {
>>> ++        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size)) {
>>> +             return NULL;
>>> +         }
>>> +         ptr = memory_region_get_ram_ptr(qxl->guest_slots[slot].mr);
>>> +@@ -1944,9 +1963,9 @@ static void qxl_dirty_one_surface(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
>>> +     uint32_t slot;
>>> +     bool rc;
>>> +
>>> +-    rc = qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset);
>>> +-    assert(rc == true);
>>> +     size = (uint64_t)height * abs(stride);
>>> ++    rc = qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size);
>>> ++    assert(rc == true);
>>> +     trace_qxl_surfaces_dirty(qxl->id, offset, size);
>>> +     qxl_set_dirty(qxl->guest_slots[slot].mr,
>>> +                   qxl->guest_slots[slot].offset + offset,
>>> +--
>>> +2.25.1
>>> +
>>> --
>>> 2.25.1
>>>
>>>
>>>
>>>
>>
>>
>>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#176508): https://lists.openembedded.org/g/openembedded-core/message/176508
> Mute This Topic: https://lists.openembedded.org/mt/96521255/3617601
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [sakoman@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/recipes-devtools/qemu/qemu.inc b/meta/recipes-devtools/qemu/qemu.inc
index fff2c87780..898fa1a8d8 100644
--- a/meta/recipes-devtools/qemu/qemu.inc
+++ b/meta/recipes-devtools/qemu/qemu.inc
@@ -115,6 +115,7 @@  SRC_URI = "https://download.qemu.org/${BPN}-${PV}.tar.xz \
 	   file://CVE-2021-3638.patch \
 	   file://CVE-2021-20196.patch \
 	   file://CVE-2021-3507.patch \
+	   file://CVE-2022-4144.patch \
            "
 UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar"
 
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch b/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
new file mode 100644
index 0000000000..3f0d5fbd5c
--- /dev/null
+++ b/meta/recipes-devtools/qemu/qemu/CVE-2022-4144.patch
@@ -0,0 +1,103 @@ 
+From 6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= <philmd@linaro.org>
+Date: Mon, 28 Nov 2022 21:27:40 +0100
+Subject: [PATCH] hw/display/qxl: Avoid buffer overrun in qxl_phys2virt
+ (CVE-2022-4144)
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Have qxl_get_check_slot_offset() return false if the requested
+buffer size does not fit within the slot memory region.
+
+Similarly qxl_phys2virt() now returns NULL in such case, and
+qxl_dirty_one_surface() aborts.
+
+This avoids buffer overrun in the host pointer returned by
+memory_region_get_ram_ptr().
+
+Fixes: CVE-2022-4144 (out-of-bounds read)
+Reported-by: Wenxu Yin (@awxylitol)
+Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1336
+
+Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+Message-Id: <20221128202741.4945-5-philmd@linaro.org>
+
+Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/6dbbf055148c6f1b7d8a3251a65bd6f3d1e1f622]
+CVE: CVE-2022-4144
+Comments: Deleted patch hunk in qxl.h,as it contains change
+in comments which is not present in current version of qemu.
+
+Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
+---
+ hw/display/qxl.c | 27 +++++++++++++++++++++++----
+ 1 file changed, 23 insertions(+), 4 deletions(-)
+
+diff --git a/hw/display/qxl.c b/hw/display/qxl.c
+index cd7eb39d..6bc8385b 100644
+--- a/hw/display/qxl.c
++++ b/hw/display/qxl.c
+@@ -1440,11 +1440,13 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
+ 
+ /* can be also called from spice server thread context */
+ static bool qxl_get_check_slot_offset(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
+-                                      uint32_t *s, uint64_t *o)
++                                      uint32_t *s, uint64_t *o,
++                                      size_t size_requested)
+ {
+     uint64_t phys   = le64_to_cpu(pqxl);
+     uint32_t slot   = (phys >> (64 -  8)) & 0xff;
+     uint64_t offset = phys & 0xffffffffffff;
++    uint64_t size_available;
+ 
+     if (slot >= NUM_MEMSLOTS) {
+         qxl_set_guest_bug(qxl, "slot too large %d >= %d", slot,
+@@ -1468,6 +1470,23 @@ static bool qxl_get_check_slot_offset(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
+                           slot, offset, qxl->guest_slots[slot].size);
+         return false;
+     }
++    size_available = memory_region_size(qxl->guest_slots[slot].mr);
++    if (qxl->guest_slots[slot].offset + offset >= size_available) {
++        qxl_set_guest_bug(qxl,
++                          "slot %d offset %"PRIu64" > region size %"PRIu64"\n",
++                          slot, qxl->guest_slots[slot].offset + offset,
++                          size_available);
++        return false;
++    }
++    size_available -= qxl->guest_slots[slot].offset + offset;
++    if (size_requested > size_available) {
++        qxl_set_guest_bug(qxl,
++                          "slot %d offset %"PRIu64" size %zu: "
++                          "overrun by %"PRIu64" bytes\n",
++                          slot, offset, size_requested,
++                          size_requested - size_available);
++        return false;
++    }
+ 
+     *s = slot;
+     *o = offset;
+@@ -1486,7 +1505,7 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
+         offset = le64_to_cpu(pqxl) & 0xffffffffffff;
+         return (void *)(intptr_t)offset;
+     case MEMSLOT_GROUP_GUEST:
+-        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset)) {
++        if (!qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size)) {
+             return NULL;
+         }
+         ptr = memory_region_get_ram_ptr(qxl->guest_slots[slot].mr);
+@@ -1944,9 +1963,9 @@ static void qxl_dirty_one_surface(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
+     uint32_t slot;
+     bool rc;
+ 
+-    rc = qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset);
+-    assert(rc == true);
+     size = (uint64_t)height * abs(stride);
++    rc = qxl_get_check_slot_offset(qxl, pqxl, &slot, &offset, size);
++    assert(rc == true);
+     trace_qxl_surfaces_dirty(qxl->id, offset, size);
+     qxl_set_dirty(qxl->guest_slots[slot].mr,
+                   qxl->guest_slots[slot].offset + offset,
+-- 
+2.25.1
+