Re: [PATCH-for-9.0? v2] hw/net/net_tx_pkt: Fix overrun in update_sctp_checksum()
Hi,
On Wed, Apr 10, 2024 at 9:05 AM Philippe Mathieu-Daudé
wrote:
>
> If a fragmented packet size is too short, do not try to
> calculate its checksum.
This was assigned CVE-2024-3567.
Thanks,
> Reproduced using:
>
> $ cat << EOF | qemu-system-i386 -display none -nodefaults \
> -machine q35,accel=qtest -m 32M \
> -device igb,netdev=net0 \
> -netdev user,id=net0 \
> -qtest stdio
> outl 0xcf8 0x8810
> outl 0xcfc 0xe000
> outl 0xcf8 0x8804
> outw 0xcfc 0x06
> write 0xe403 0x1 0x02
> writel 0xe0003808 0x
> write 0xe000381a 0x1 0x5b
> write 0xe000381b 0x1 0x00
> EOF
> Assertion failed: (offset == 0), function iov_from_buf_full, file
> util/iov.c, line 39.
> #1 0x5575e81e952a in iov_from_buf_full qemu/util/iov.c:39:5
> #2 0x5575e6500768 in net_tx_pkt_update_sctp_checksum
> qemu/hw/net/net_tx_pkt.c:144:9
> #3 0x5575e659f3e1 in igb_setup_tx_offloads qemu/hw/net/igb_core.c:478:11
> #4 0x5575e659f3e1 in igb_tx_pkt_send qemu/hw/net/igb_core.c:552:10
> #5 0x5575e659f3e1 in igb_process_tx_desc qemu/hw/net/igb_core.c:671:17
> #6 0x5575e659f3e1 in igb_start_xmit qemu/hw/net/igb_core.c:903:9
> #7 0x5575e659f3e1 in igb_set_tdt qemu/hw/net/igb_core.c:2812:5
> #8 0x5575e657d6a4 in igb_core_write qemu/hw/net/igb_core.c:4248:9
>
> Cc: qemu-sta...@nongnu.org
> Reported-by: Zheyu Ma
> Fixes: f199b13bc1 ("igb: Implement Tx SCTP CSO")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2273
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Since v1: check at offset 8 (Akihiko)
> ---
> hw/net/net_tx_pkt.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> index 2134a18c4c..b7b1de816d 100644
> --- a/hw/net/net_tx_pkt.c
> +++ b/hw/net/net_tx_pkt.c
> @@ -141,6 +141,10 @@ bool net_tx_pkt_update_sctp_checksum(struct NetTxPkt
> *pkt)
> uint32_t csum = 0;
> struct iovec *pl_start_frag = pkt->vec + NET_TX_PKT_PL_START_FRAG;
>
> +if (iov_size(pl_start_frag, pkt->payload_frags) < 8 + sizeof(csum)) {
> + return false;
> +}
> +
> if (iov_from_buf(pl_start_frag, pkt->payload_frags, 8, ,
> sizeof(csum)) < sizeof(csum)) {
> return false;
> }
> --
> 2.41.0
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH-for-9.0? 0/3] hw/block/nand: Fix out-of-bound access in NAND block buffer
On Mon, Apr 8, 2024 at 10:36 AM Philippe Mathieu-Daudé wrote: > > Fix for https://gitlab.com/qemu-project/qemu/-/issues/1446 Does hw/block/nand meet the security requirements for CVE assignment? => https://www.qemu.org/docs/master/system/security.html > Philippe Mathieu-Daudé (3): > hw/block/nand: Factor nand_load_iolen() method out > hw/block/nand: Have blk_load() return boolean indicating success > hw/block/nand: Fix out-of-bound access in NAND block buffer > > hw/block/nand.c | 50 + > 1 file changed, 34 insertions(+), 16 deletions(-) > > -- > 2.41.0 > -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH-for-9.0] hw/sd/sdhci: Discard excess of data written to Buffer Data Port register
unsigned i;
> +unsigned i, available;
>
> /* Check that there is free space left in a buffer */
> if (!(s->prnsts & SDHC_SPACE_AVAILABLE)) {
> @@ -560,6 +560,14 @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t
> value, unsigned size)
> return;
> }
>
> +available = s->buf_maxsz - s->data_count;
> +if (size > available) {
> +qemu_log_mask(LOG_GUEST_ERROR, "SDHC buffer data full (size:
> %"PRIu32")"
> + " discarding %u byte%s\n",
> + s->buf_maxsz, size - available,
> + size - available > 1 ? "s" : "");
> +size = available; /* Excess data of the last write is ignored. */
> +}
> for (i = 0; i < size; i++) {
> s->fifo_buffer[s->data_count] = value & 0xFF;
> s->data_count++;
> --
> 2.41.0
>
Thank you Philippe. This was assigned CVE-2024-3447.
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH-for-9.0 0/4] hw/virtio: Protect from more DMA re-entrancy bugs
Hi, On Thu, Apr 4, 2024 at 9:13 PM Philippe Mathieu-Daudé wrote: > > Gerd suggested to use the transport guard to protect the > device from DMA re-entrancy abuses. This was assigned CVE-2024-3446. > Philippe Mathieu-Daudé (4): > hw/virtio: Introduce virtio_bh_new_guarded() helper > hw/display/virtio-gpu: Protect from DMA re-entrancy bugs > hw/char/virtio-serial-bus: Protect from DMA re-entrancy bugs > hw/virtio/virtio-crypto: Protect from DMA re-entrancy bugs > > include/hw/virtio/virtio.h | 7 +++ > hw/char/virtio-serial-bus.c | 3 +-- > hw/display/virtio-gpu.c | 6 ++ > hw/virtio/virtio-crypto.c | 4 ++-- > hw/virtio/virtio.c | 10 ++ > 5 files changed, 22 insertions(+), 8 deletions(-) > > -- > 2.41.0 > Thanks, -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] virtio-net: correctly copy vnet header when flushing TX
On Tue, Jan 2, 2024 at 12:20 PM Yuri Benditovich
wrote:
>
> I agree, thank you.
>
> Where is this CVE-2023-6693 available?
Here: https://bugzilla.redhat.com/show_bug.cgi?id=2254580.
> Thanks,
> Yuri
>
> On Tue, Jan 2, 2024 at 5:29 AM Jason Wang wrote:
>>
>> When HASH_REPORT is negotiated, the guest_hdr_len might be larger than
>> the size of the mergeable rx buffer header. Using
>> virtio_net_hdr_mrg_rxbuf during the header swap might lead a stack
>> overflow in this case. Fixing this by using virtio_net_hdr_v1_hash
>> instead.
>>
>> Reported-by: Xiao Lei
>> Cc: Yuri Benditovich
>> Cc: qemu-sta...@nongnu.org
>> Cc: Mauro Matteo Cascella
>> Fixes: CVE-2023-6693
>> Fixes: e22f0603fb2f ("virtio-net: reference implementation of hash report")
>> Signed-off-by: Jason Wang
>> ---
>> hw/net/virtio-net.c | 13 +
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 80c56f0cfc..73024babd4 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -674,6 +674,11 @@ static void virtio_net_set_mrg_rx_bufs(VirtIONet *n,
>> int mergeable_rx_bufs,
>>
>> n->mergeable_rx_bufs = mergeable_rx_bufs;
>>
>> +/*
>> + * Note: when extending the vnet header, please make sure to
>> + * change the vnet header copying logic in virtio_net_flush_tx()
>> + * as well.
>> + */
>> if (version_1) {
>> n->guest_hdr_len = hash_report ?
>> sizeof(struct virtio_net_hdr_v1_hash) :
>> @@ -2693,7 +2698,7 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>> ssize_t ret;
>> unsigned int out_num;
>> struct iovec sg[VIRTQUEUE_MAX_SIZE], sg2[VIRTQUEUE_MAX_SIZE + 1],
>> *out_sg;
>> -struct virtio_net_hdr_mrg_rxbuf mhdr;
>> +struct virtio_net_hdr_v1_hash vhdr;
>>
>> elem = virtqueue_pop(q->tx_vq, sizeof(VirtQueueElement));
>> if (!elem) {
>> @@ -2710,7 +2715,7 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>> }
>>
>> if (n->has_vnet_hdr) {
>> -if (iov_to_buf(out_sg, out_num, 0, , n->guest_hdr_len) <
>> +if (iov_to_buf(out_sg, out_num, 0, , n->guest_hdr_len) <
>> n->guest_hdr_len) {
>> virtio_error(vdev, "virtio-net header incorrect");
>> virtqueue_detach_element(q->tx_vq, elem, 0);
>> @@ -2718,8 +2723,8 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>> return -EINVAL;
>> }
>> if (n->needs_vnet_hdr_swap) {
>> -virtio_net_hdr_swap(vdev, (void *) );
>> -sg2[0].iov_base =
>> +virtio_net_hdr_swap(vdev, (void *) );
>> +sg2[0].iov_base =
>> sg2[0].iov_len = n->guest_hdr_len;
>> out_num = iov_copy([1], ARRAY_SIZE(sg2) - 1,
>> out_sg, out_num,
>> --
>> 2.42.0
>>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: virtio...@redhat.com bouncing
Hi, On Wed, Nov 8, 2023 at 12:40 PM Stefan Hajnoczi wrote: > > On Wed, 8 Nov 2023 at 19:05, Philippe Mathieu-Daudé wrote: > > > > Hi Stefan, > > > > You are listed as virtiofs maintainer, see MAINTAINERS: > > > >virtiofs > >M: Stefan Hajnoczi > >S: Supported > >F: hw/virtio/vhost-user-fs* > >F: include/hw/virtio/vhost-user-fs.h > >L: virtio...@redhat.com > > > > Mails sent to this list address are bouncing: > > > > : host int-mx-rdu2.corp.redhat.com[10.11.203.6] > > said: 550 > > 5.1.1 : Recipient address rejected: User > > unknown in > > local recipient table (in reply to RCPT TO command) > > > > Maybe the list need to be updated, like Daniel did with libvir-list@? > > https://lore.kernel.org/qemu-devel/20231027095643.2842382-1-berra...@redhat.com/ > > Yes, it does. The new mailing list address has not been created yet. > I'll update the MAINTAINERS file when the new mailing list becomes > available in the coming days. > > > Looking at other lists, I also see secalert@: > > > >$ git grep L:.*redhat MAINTAINERS > >MAINTAINERS:86:L: secal...@redhat.com > >MAINTAINERS:2231:L: virtio...@redhat.com > > > > So Cc'ing Michael / Mauro. > > I'm not sure about secalert. Thanks for CCing Michael and Mauro. secalert@ is regularly monitored and working fine. Perhaps we can add qemu-secur...@nongnu.org, in addition to secalert, to match what's stated in the 'Security Process' page. ``` diff --git a/MAINTAINERS b/MAINTAINERS index 8e8a7d5be5..af74d5487f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -83,6 +83,7 @@ Responsible Disclosure, Reporting Security Issues ----- W: https://wiki.qemu.org/SecurityProcess M: Michael S. Tsirkin +L: qemu-secur...@nongnu.org L: secal...@redhat.com Trivial patches ``` > Thanks, > Stefan > Thank you, -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
[Bug 1863025] Re: Use-after-free after flush in TCG accelerator
** CVE added: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2022-36648 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1863025 Title: Use-after-free after flush in TCG accelerator Status in QEMU: Fix Released Bug description: I believe I found a UAF in TCG that can lead to a guest VM escape. The security list informed me "This can not be treated as a security issue." and to post it here. I am looking at the 4.2.0 source code. The issue requires a race and I will try to describe it in terms of three concurrent threads. Thread A: A1. qemu_tcg_cpu_thread_fn runs work loop A2. qemu_wait_io_event => qemu_wait_io_event_common => process_queued_cpu_work A3. start_exclusive critical section entered A4. do_tb_flush is called, TB memory freed/re-allocated A5. end_exclusive exits critical section Thread B: B1. qemu_tcg_cpu_thread_fn runs work loop B2. tcg_cpu_exec => cpu_exec => tb_find => tb_gen_code B3. tcg_tb_alloc obtains a new TB Thread C: C1. qemu_tcg_cpu_thread_fn runs work loop C2. cpu_exec_step_atomic executes C3. TB obtained with tb_lookup__cpu_state or tb_gen_code C4. start_exclusive critical section entered C5. cpu_tb_exec executes the TB code C6. end_exclusive exits critical section Consider the following sequence of events: B2 => B3 => C3 (same TB as B2) => A3 => A4 (TB freed) => A5 => B2 => B3 (re-allocates TB from B2) => C4 => C5 (freed/reused TB now executing) => C6 In short, because thread C uses the TB in the critical section, there is no guarantee that the pointer has not been "freed" (rather the memory is marked as re-usable) and therefore a use-after-free occurs. Since the TCG generated code can be in the same memory as the TB data structure, it is possible for an attacker to overwrite the UAF pointer with code generated from TCG. This can overwrite key pointer values and could lead to code execution on the host outside of the TCG sandbox. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1863025/+subscriptions
Re: [Bug 1863025] Re: Use-after-free after flush in TCG accelerator
On Thu, Aug 31, 2023 at 3:57 PM Daniel P. Berrangé wrote:
>
> On Thu, Aug 31, 2023 at 03:40:25PM +0200, Philippe Mathieu-Daudé wrote:
> > Hi Samuel,
> >
> > On 31/8/23 14:48, Samuel Henrique wrote:
> > > CVE-2020-24165 was assigned to this:
> > > https://nvd.nist.gov/vuln/detail/CVE-2020-24165
> > >
> > > I had no involvement in the assignment, posting here for reference only.
> > >
> > > ** CVE added: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2020-24165
> >
> > QEMU 4.2.0 was released in 2019. The issue you report
> > has been fixed in commit 886cc68943 ("accel/tcg: fix race
> > in cpu_exec_step_atomic (bug 1863025)") which is included
> > in QEMU v5.0, released in April 2020, more than 3 years ago.
> >
> > What do you expect us to do here? I'm not sure whether assigning
> > CVE for 3 years old code is a good use of engineering time.
>
> In any case per our stated security policy, we do not consider TCG to
> be providing a security boundary between host and guest, and thus bugs
> in TCG aren't considered security flaws:
>
>
> https://www.qemu.org/docs/master/system/security.html#non-virtualization-use-case
Right, and it is clearly indicated in the referenced launchpad bug:
'The security list informed me "This can not be treated as a security
issue"'.
This adds up to CVE-2022-36648, which is also a mystery to me in terms
of CVE assignment and CVSS scoring (rated as Critical). I don't know
what's going on with NVD, there must be something wrong on their side.
I disputed both CVEs via https://cveform.mitre.org/.
> With regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] hw/net/rocker: avoid NULL pointer dereference in of_dpa_cmd_add_l2_flood
On Mon, Aug 28, 2023 at 6:11 PM Philippe Mathieu-Daudé
wrote:
>
> On 27/8/23 13:07, Mauro Matteo Cascella wrote:
> > On Sat, Aug 26, 2023 at 4:31 PM Mauro Matteo Cascella
> > wrote:
> >>
> >> On Fri, Jun 24, 2022 at 4:40 PM Mauro Matteo Cascella
> >> wrote:
> >>>
> >>> rocker_tlv_parse_nested could return early because of no group ids in
> >>> the group_tlvs. In such case tlvs is NULL; tlvs[i + 1] in the next
> >>> for-loop will deref the NULL pointer.
> >
> > Looking at the code once again, tlvs is a pointer to a g_new0
> > allocated memory, so I don't know how it can be NULL after
> > rocker_tlv_parse_nested (unless g_new0 returns NULL in the first
> > place). I do not recall the details of this bug. Arayz?
>
> Per :
>
>If any call to allocate memory using functions g_new(), g_new0(),
>g_renew(), g_malloc(), g_malloc0(), g_malloc0_n(), g_realloc(), and
>g_realloc_n() fails, the application is terminated. This also means
>that there is no need to check if the call succeeded. On the other
>hand, g_try_...() family of functions returns NULL on failure that
>can be used as a check for unsuccessful memory allocation. The
>application is not terminated in this case.
>
>
> group->l2_flood.group_count is a uint16_t, so up to UINT16_MAX = 0x.
>
> So:
>
>tlvs = g_new0(RockerTlv *, group->l2_flood.group_count + 1);
>
> is at most an malloc(0x1 * sizeof(void *)) = 0x8 = 512 KiB.
>
> QEMU use way bigger heap allocations.
>
> I don't know the net/ subsystem enough to have an idea about how many
> concurrent packets can be processed by this device model, but I'd be
> surprised if this triggers a OOM.
>
> As usual, do you have a reproducer?
I just found the original bug report and opened a new issue upstream.
See https://gitlab.com/qemu-project/qemu/-/issues/1851.
> >> Someone somehow reserved a new CVE for this bug, published a few days
> >> ago here: https://nvd.nist.gov/vuln/detail/CVE-2022-36648.
> >>
> >> Not only is this not CVE worthy (rocker code does not fall under the
> >> KVM virtualization use case [1]) but what's most concerning is that it
> >> got a CVSS score of 10 :/
> >>
> >> I'm going to dispute this CVE. Hopefully, it will be rejected soon. In
> >> any case, can we get this patch merged?
> >>
> >> [1] https://www.qemu.org/docs/master/system/security.html
> >>
> >> Thanks,
> >>
> >>> Signed-off-by: Mauro Matteo Cascella
> >>> Reported-by:
> >>> ---
> >>> hw/net/rocker/rocker_of_dpa.c | 5 +
> >>> 1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
> >>> index b3b8c5bb6d..1611b79227 100644
> >>> --- a/hw/net/rocker/rocker_of_dpa.c
> >>> +++ b/hw/net/rocker/rocker_of_dpa.c
> >>> @@ -2039,6 +2039,11 @@ static int of_dpa_cmd_add_l2_flood(OfDpa *of_dpa,
> >>> OfDpaGroup *group,
> >>> rocker_tlv_parse_nested(tlvs, group->l2_flood.group_count,
> >>> group_tlvs[ROCKER_TLV_OF_DPA_GROUP_IDS]);
> >>>
> >>> +if (!tlvs) {
> >>> +err = -ROCKER_EINVAL;
> >>> +goto err_out;
> >>> +}
> >>> +
> >>> for (i = 0; i < group->l2_flood.group_count; i++) {
> >>> group->l2_flood.group_ids[i] = rocker_tlv_get_le32(tlvs[i + 1]);
> >>> }
> >>> --
> >>> 2.35.3
> >>>
> >>
> >
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] hw/net/rocker: avoid NULL pointer dereference in of_dpa_cmd_add_l2_flood
On Sat, Aug 26, 2023 at 4:31 PM Mauro Matteo Cascella
wrote:
>
> On Fri, Jun 24, 2022 at 4:40 PM Mauro Matteo Cascella
> wrote:
> >
> > rocker_tlv_parse_nested could return early because of no group ids in
> > the group_tlvs. In such case tlvs is NULL; tlvs[i + 1] in the next
> > for-loop will deref the NULL pointer.
Looking at the code once again, tlvs is a pointer to a g_new0
allocated memory, so I don't know how it can be NULL after
rocker_tlv_parse_nested (unless g_new0 returns NULL in the first
place). I do not recall the details of this bug. Arayz?
> Someone somehow reserved a new CVE for this bug, published a few days
> ago here: https://nvd.nist.gov/vuln/detail/CVE-2022-36648.
>
> Not only is this not CVE worthy (rocker code does not fall under the
> KVM virtualization use case [1]) but what's most concerning is that it
> got a CVSS score of 10 :/
>
> I'm going to dispute this CVE. Hopefully, it will be rejected soon. In
> any case, can we get this patch merged?
>
> [1] https://www.qemu.org/docs/master/system/security.html
>
> Thanks,
>
> > Signed-off-by: Mauro Matteo Cascella
> > Reported-by:
> > ---
> > hw/net/rocker/rocker_of_dpa.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
> > index b3b8c5bb6d..1611b79227 100644
> > --- a/hw/net/rocker/rocker_of_dpa.c
> > +++ b/hw/net/rocker/rocker_of_dpa.c
> > @@ -2039,6 +2039,11 @@ static int of_dpa_cmd_add_l2_flood(OfDpa *of_dpa,
> > OfDpaGroup *group,
> > rocker_tlv_parse_nested(tlvs, group->l2_flood.group_count,
> > group_tlvs[ROCKER_TLV_OF_DPA_GROUP_IDS]);
> >
> > +if (!tlvs) {
> > +err = -ROCKER_EINVAL;
> > +goto err_out;
> > +}
> > +
> > for (i = 0; i < group->l2_flood.group_count; i++) {
> > group->l2_flood.group_ids[i] = rocker_tlv_get_le32(tlvs[i + 1]);
> > }
> > --
> > 2.35.3
> >
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] hw/net/rocker: avoid NULL pointer dereference in of_dpa_cmd_add_l2_flood
On Fri, Jun 24, 2022 at 4:40 PM Mauro Matteo Cascella
wrote:
>
> rocker_tlv_parse_nested could return early because of no group ids in
> the group_tlvs. In such case tlvs is NULL; tlvs[i + 1] in the next
> for-loop will deref the NULL pointer.
Someone somehow reserved a new CVE for this bug, published a few days
ago here: https://nvd.nist.gov/vuln/detail/CVE-2022-36648.
Not only is this not CVE worthy (rocker code does not fall under the
KVM virtualization use case [1]) but what's most concerning is that it
got a CVSS score of 10 :/
I'm going to dispute this CVE. Hopefully, it will be rejected soon. In
any case, can we get this patch merged?
[1] https://www.qemu.org/docs/master/system/security.html
Thanks,
> Signed-off-by: Mauro Matteo Cascella
> Reported-by:
> ---
> hw/net/rocker/rocker_of_dpa.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
> index b3b8c5bb6d..1611b79227 100644
> --- a/hw/net/rocker/rocker_of_dpa.c
> +++ b/hw/net/rocker/rocker_of_dpa.c
> @@ -2039,6 +2039,11 @@ static int of_dpa_cmd_add_l2_flood(OfDpa *of_dpa,
> OfDpaGroup *group,
> rocker_tlv_parse_nested(tlvs, group->l2_flood.group_count,
> group_tlvs[ROCKER_TLV_OF_DPA_GROUP_IDS]);
>
> +if (!tlvs) {
> +err = -ROCKER_EINVAL;
> +goto err_out;
> +}
> +
> for (i = 0; i < group->l2_flood.group_count; i++) {
> group->l2_flood.group_ids[i] = rocker_tlv_get_le32(tlvs[i + 1]);
> }
> --
> 2.35.3
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH 00/21] Patch Round-up for stable 7.2.2, freeze on 2023-04-20
Hi Michael, > 13.04.2023 23:50, Konstantin Kostiuk wrote: > > Hi Michael, > > > > You cherry-picked one of my patch qga/win32: Remove change action from MSI > > installer > > but it is part of the CVE fix. > > > > Please cherry-pick one more patch. > > > > Original mail: > > https://patchew.org/QEMU/20230303192008.109549-1-kkost...@redhat.com/ > > <https://patchew.org/QEMU/20230303192008.109549-1-kkost...@redhat.com/> > > I planned to pick both but somehow ended up with just one. > > Thank you very much for this, picked up now! I do not see the second part of the fix (commit 07ce178a "qga/win32: Use rundll for VSS installation") in stable-7.2. Has it somehow fallen through the cracks? Thanks, > /mjt > -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)
On Tue, Jul 4, 2023 at 11:03 AM Marc-André Lureau
wrote:
>
>
>
> On Tue, Jul 4, 2023 at 10:42 AM Mauro Matteo Cascella
> wrote:
>>
>> A wrong exit condition may lead to an infinite loop when inflating a
>> valid zlib buffer containing some extra bytes in the `inflate_buffer`
>> function. The bug only occurs post-authentication. Return the buffer
>> immediately if the end of the compressed data has been reached
>> (Z_STREAM_END).
>>
>> Fixes: CVE-2023-3255
>> Fixes: 0bf41cab ("ui/vnc: clipboard support")
>> Reported-by: Kevin Denis
>> Signed-off-by: Mauro Matteo Cascella
>
>
> Tested-by: Marc-André Lureau
> Reviewed-by: Marc-André Lureau
>
> Note: we may want to disconnect the client when there are extra bytes in the
> message, or print some warnings.
Sure, I guess we can call vnc_disconnect_finish or vnc_client_error
for disconnecting, not sure how to properly print warnings. Feel free
to add that yourself when applying the patch. Or I can try to send v2
if you prefer.
Thanks,
>>
>> ---
>> ui/vnc-clipboard.c | 10 --
>> 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
>> index 8aeadfaa21..c759be3438 100644
>> --- a/ui/vnc-clipboard.c
>> +++ b/ui/vnc-clipboard.c
>> @@ -50,8 +50,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t
>> in_len, uint32_t *size)
>> ret = inflate(, Z_FINISH);
>> switch (ret) {
>> case Z_OK:
>> -case Z_STREAM_END:
>> break;
>> +case Z_STREAM_END:
>> +*size = stream.total_out;
>> +inflateEnd();
>> +return out;
>> case Z_BUF_ERROR:
>> out_len <<= 1;
>> if (out_len > (1 << 20)) {
>> @@ -66,11 +69,6 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t
>> in_len, uint32_t *size)
>> }
>> }
>>
>> -*size = stream.total_out;
>> -inflateEnd();
>> -
>> -return out;
>> -
>> err_end:
>> inflateEnd();
>> err:
>> --
>> 2.41.0
>>
>>
>
>
> --
> Marc-André Lureau
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)
A wrong exit condition may lead to an infinite loop when inflating a
valid zlib buffer containing some extra bytes in the `inflate_buffer`
function. The bug only occurs post-authentication. Return the buffer
immediately if the end of the compressed data has been reached
(Z_STREAM_END).
Fixes: CVE-2023-3255
Fixes: 0bf41cab ("ui/vnc: clipboard support")
Reported-by: Kevin Denis
Signed-off-by: Mauro Matteo Cascella
---
ui/vnc-clipboard.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
index 8aeadfaa21..c759be3438 100644
--- a/ui/vnc-clipboard.c
+++ b/ui/vnc-clipboard.c
@@ -50,8 +50,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len,
uint32_t *size)
ret = inflate(, Z_FINISH);
switch (ret) {
case Z_OK:
-case Z_STREAM_END:
break;
+case Z_STREAM_END:
+*size = stream.total_out;
+inflateEnd();
+return out;
case Z_BUF_ERROR:
out_len <<= 1;
if (out_len > (1 << 20)) {
@@ -66,11 +69,6 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len,
uint32_t *size)
}
}
-*size = stream.total_out;
-inflateEnd();
-
-return out;
-
err_end:
inflateEnd();
err:
--
2.41.0
Re: [PATCH v1] hw/pvrdma: Protect against buggy or malicious guest driver
On Mon, May 15, 2023 at 6:13 PM Michael Tokarev wrote: > > 01.03.2023 17:29, Yuval Shaia wrote: > > Guest driver allocates and initialize page tables to be used as a ring > > of descriptors for CQ and async events. > > The page table that represents the ring, along with the number of pages > > in the page table is passed to the device. > > Currently our device supports only one page table for a ring. > > > > Let's make sure that the number of page table entries the driver > > reports, do not exceeds the one page table size. > > > > Reported-by: Soul Chen > > Signed-off-by: Yuval Shaia > > --- > > v0 -> v1: > > * Take ring-state into account > > * Add Reported-by > > --- > > hw/rdma/vmw/pvrdma_main.c | 16 +++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > Fixes: CVE-2023-1544 > > Ping ^2? Ping ^3? > Laurent, maybe you can take this one too? > I understand the fact you picked up the previous one in this area > does not make you pvrdma maintainer, but it is definitely being stuck.. :) > > /mjt > -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] hw/sd/sdhci: reset data count in sdhci_buff_access_is_sequential()
On Sat, May 27, 2023 at 11:00 AM Michael Tokarev wrote: > > Mon, 7 Nov 2022 11:35:10 +0100, you wrote: > > Make sure to reset data_count if it's equal to (or exceeds) block_size. > > This prevents an off-by-one read / write when accessing s->fifo_buffer > > in sdhci_read_dataport / sdhci_write_dataport, both called right after > > sdhci_buff_access_is_sequential. > > > > Fixes: CVE-2022-3872 > > .. > > Has this been forgotten, or maybe a better fix is needed? > > https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg01068.html There was a better patch proposed by Philippe: https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg01161.html Which was later dropped due to a CI failure: https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg01504.html Not sure what's the current status. > Thanks, > > /mjt > -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
[PATCH v2] ui/cursor: make width/height unsigned 16-bit integer
Although not actually exploitable at the moment, a negative width/height
could make datasize wrap around and potentially lead to buffer overflow.
Since there is no reason a negative width/height is ever appropriate,
modify QEMUCursor struct and cursor_alloc prototype to accept uint16_t.
This protects us against accidentally introducing future bugs.
Signed-off-by: Mauro Matteo Cascella
Reported-by: Jacek Halon
Reported-by: Yair Mizrahi
Reported-by: Elsayed El-Refa'ei
Reviewed-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
---
v2:
- update commit summary/description
- add Reported-by/Reviewed-by
- use uint16_t instead of uint32_t
- add comment in cursor_alloc
include/ui/console.h | 4 ++--
ui/cursor.c | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 2a8fab091f..ae5ec466c1 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
/* cursor data format is 32bit RGBA */
typedef struct QEMUCursor {
-int width, height;
+uint16_twidth, height;
int hot_x, hot_y;
int refcount;
uint32_tdata[];
} QEMUCursor;
-QEMUCursor *cursor_alloc(int width, int height);
+QEMUCursor *cursor_alloc(uint16_t width, uint16_t height);
QEMUCursor *cursor_ref(QEMUCursor *c);
void cursor_unref(QEMUCursor *c);
QEMUCursor *cursor_builtin_hidden(void);
diff --git a/ui/cursor.c b/ui/cursor.c
index 6fe67990e2..29717b3ecb 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -90,11 +90,12 @@ QEMUCursor *cursor_builtin_left_ptr(void)
return cursor_parse_xpm(cursor_left_ptr_xpm);
}
-QEMUCursor *cursor_alloc(int width, int height)
+QEMUCursor *cursor_alloc(uint16_t width, uint16_t height)
{
QEMUCursor *c;
size_t datasize = width * height * sizeof(uint32_t);
+/* Modern physical hardware typically uses 512x512 sprites */
if (width > 512 || height > 512) {
return NULL;
}
--
2.40.1
Re: [PATCH] ui/cursor: incomplete check for integer overflow in cursor_alloc
On Tue, May 23, 2023 at 4:07 PM Philippe Mathieu-Daudé
wrote:
>
> On 23/5/23 14:57, Mauro Matteo Cascella wrote:
> > On Tue, May 23, 2023 at 10:37 AM Philippe Mathieu-Daudé
> > wrote:
> >>
> >> On 23/5/23 10:09, Daniel P. Berrangé wrote:
> >>> On Mon, May 22, 2023 at 08:55:02PM +0200, Philippe Mathieu-Daudé wrote:
> >>>> On 9/5/23 09:13, Marc-André Lureau wrote:
> >>>>> Hi
> >>>>>
> >>>>> On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
> >>>>> mailto:mcasc...@redhat.com>> wrote:
> >>>>>
> >>>>> The cursor_alloc function still accepts a signed integer for both
> >>>>> the cursor
> >>>>> width and height. A specially crafted negative width/height could
> >>>>> make datasize
> >>>>> wrap around and cause the next allocation to be 0, potentially
> >>>>> leading to a
> >>>>> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
> >>>>> prototype to
> >>>>> accept unsigned ints.
> >>>>>
> >>>>> Fixes: CVE-2023-1601
> >>>>> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> >>>>> (CVE-2021-4206)")
> >>>>> Signed-off-by: Mauro Matteo Cascella >>>>> <mailto:mcasc...@redhat.com>>
> >>>>> Reported-by: Jacek Halon >>>>> <mailto:jacek.ha...@gmail.com>>
> >>>>>
> >>>>>
> >>>>> Reviewed-by: Marc-André Lureau >>>>> <mailto:marcandre.lur...@redhat.com>>
> >>>>>
> >>>>> It looks like this is not exploitable, QXL code uses u16 types, and
> >>>>
> >>>> 0x * 0x * 4 still overflows on 32-bit host, right?
> >>>
> >>> cursor_alloc() will reject 0x:
> >>>
> >>> if (width > 512 || height > 512) {
> >>> return NULL;
> >>> }
> >>
> >> I hadn't looked at the source file (the 'datasize' assignation
> >> made me incorrectly think it'd be use before sanitized).
> >>
> >> Still I wonder why can't we use a simple 'unsigned' type instead
> >> of a uint32_t, but I won't insist.
> >
> > I can send v2 with s/uint32_t/uint16_t/ if you think it's a relevant change.
>
> Specifying the word size doesn't really add any (security) value IMHO.
No security benefit, I know, it just seems more reasonable given what
Gerd said about 512x512 sprites.
> I'll stop bikeshedding here.
>
> Regards,
>
> Phil.
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] ui/cursor: incomplete check for integer overflow in cursor_alloc
On Tue, May 23, 2023 at 3:03 PM Daniel P. Berrangé wrote:
>
> On Tue, May 23, 2023 at 02:50:09PM +0200, Mauro Matteo Cascella wrote:
> > On Tue, May 23, 2023 at 10:16 AM Daniel P. Berrangé
> > wrote:
> > >
> > > On Mon, May 08, 2023 at 04:18:13PM +0200, Mauro Matteo Cascella wrote:
> > > > The cursor_alloc function still accepts a signed integer for both the
> > > > cursor
> > > > width and height. A specially crafted negative width/height could make
> > > > datasize
> > > > wrap around and cause the next allocation to be 0, potentially leading
> > > > to a
> > > > heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
> > > > prototype to
> > > > accept unsigned ints.
> > > >
> > > I concur with Marc-Andre that there is no code path that can
> > > actually trigger an overflow:
> > >
> > >
> > > hw/display/ati.c:s->cursor = cursor_alloc(64, 64);
> > > hw/display/vhost-user-gpu.c:s->current_cursor =
> > > cursor_alloc(64, 64);
> > > hw/display/virtio-gpu.c:s->current_cursor =
> > > cursor_alloc(64, 64);
> > >
> > > Not exploitable as fixed size
> > >
> > > hw/display/qxl-render.c:c = cursor_alloc(cursor->header.width,
> > > cursor->header.height);
> > >
> > > Cursor header defined as:
> > >
> > > typedef struct SPICE_ATTR_PACKED QXLCursorHeader {
> > > uint64_t unique;
> > > uint16_t type;
> > > uint16_t width;
> > > uint16_t height;
> > > uint16_t hot_spot_x;
> > > uint16_t hot_spot_y;
> > > } QXLCursorHeader;
> > >
> > > So no negative values can be passed to cursor_alloc()
>
> > >
> > > > Fixes: CVE-2023-1601
> > > > Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> > > > (CVE-2021-4206)")
> > >
> > > Given there is no possible codepath that can overflow, CVE-2023-1601
> > > looks invalid to me. It should be clsoed as not-a-bug and these two
> > > Fixes lines removed.
> >
> > I think you can tweak the original PoC [1] to trigger this bug.
> > Setting width/height to 0x8000 (versus 0x8000) should do the
> > trick. You should be able to overflow datasize while bypassing the
> > sanity check (width > 512 || height > 512) as width/height are signed
> > prior to this patch. I haven't tested it, though.
>
> The QXLCursorHeader width/height fields are uint16_t, so 0x8000
> will get truncated. No matter what value the guest sets, when we
> interpret this in qxl_cursor when calling cursor_alloc, the value
> will be in the range 0-65535, as that's the bounds of uint16_t.
>
> We'll pass this unsigned value to cursor_alloc() which converts from
> uint16_t, to (signed) int. 'int' is larger than uint16_t, so the
> result will still be positive in the range 0-65535, and so the sanity
> check > 512 will fire and protect us.
Oh, you are right! Then yes, feel free to drop the two 'Fixes' lines.
This is more of a hardening bug than a real security issue. I'll
reject the newly assigned CVE.
Thanks,
> I still see no bug, let alone a CVE.
>
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] ui/cursor: incomplete check for integer overflow in cursor_alloc
On Tue, May 23, 2023 at 10:37 AM Philippe Mathieu-Daudé
wrote:
>
> On 23/5/23 10:09, Daniel P. Berrangé wrote:
> > On Mon, May 22, 2023 at 08:55:02PM +0200, Philippe Mathieu-Daudé wrote:
> >> On 9/5/23 09:13, Marc-André Lureau wrote:
> >>> Hi
> >>>
> >>> On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
> >>> mailto:mcasc...@redhat.com>> wrote:
> >>>
> >>> The cursor_alloc function still accepts a signed integer for both
> >>> the cursor
> >>> width and height. A specially crafted negative width/height could
> >>> make datasize
> >>> wrap around and cause the next allocation to be 0, potentially
> >>> leading to a
> >>> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
> >>> prototype to
> >>> accept unsigned ints.
> >>>
> >>> Fixes: CVE-2023-1601
> >>> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> >>> (CVE-2021-4206)")
> >>> Signed-off-by: Mauro Matteo Cascella >>> <mailto:mcasc...@redhat.com>>
> >>> Reported-by: Jacek Halon >>> <mailto:jacek.ha...@gmail.com>>
> >>>
> >>>
> >>> Reviewed-by: Marc-André Lureau >>> <mailto:marcandre.lur...@redhat.com>>
> >>>
> >>> It looks like this is not exploitable, QXL code uses u16 types, and
> >>
> >> 0x * 0x * 4 still overflows on 32-bit host, right?
> >
> > cursor_alloc() will reject 0x:
> >
> > if (width > 512 || height > 512) {
> > return NULL;
> > }
>
> I hadn't looked at the source file (the 'datasize' assignation
> made me incorrectly think it'd be use before sanitized).
>
> Still I wonder why can't we use a simple 'unsigned' type instead
> of a uint32_t, but I won't insist.
I can send v2 with s/uint32_t/uint16_t/ if you think it's a relevant change.
> >>
> >>> VMWare VGA checks for values > 256. Other paths use fixed size.
> >>>
> >>> ---
> >>>include/ui/console.h | 4 ++--
> >>>ui/cursor.c | 2 +-
> >>>2 files changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/include/ui/console.h b/include/ui/console.h
> >>> index 2a8fab091f..92a4d90a1b 100644
> >>> --- a/include/ui/console.h
> >>> +++ b/include/ui/console.h
> >>> @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
> >>>
> >>>/* cursor data format is 32bit RGBA */
> >>>typedef struct QEMUCursor {
> >>> -int width, height;
> >>> +uint32_twidth, height;
> >>>int hot_x, hot_y;
> >>>int refcount;
> >>>uint32_tdata[];
> >>>} QEMUCursor;
> >>>
> >>> -QEMUCursor *cursor_alloc(int width, int height);
> >>> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> >>>QEMUCursor *cursor_ref(QEMUCursor *c);
> >>>void cursor_unref(QEMUCursor *c);
> >>>QEMUCursor *cursor_builtin_hidden(void);
> >>> diff --git a/ui/cursor.c b/ui/cursor.c
> >>> index 6fe67990e2..b5fcb64839 100644
> >>> --- a/ui/cursor.c
> >>> +++ b/ui/cursor.c
> >>> @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> >>>return cursor_parse_xpm(cursor_left_ptr_xpm);
> >>>}
> >>>
> >>> -QEMUCursor *cursor_alloc(int width, int height)
> >>> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> >>>{
> >>>QEMUCursor *c;
> >>
> >> Can't we check width/height > 0 && <= SOME_LIMIT_THAT_MAKES_SENSE?
> >>
> >> Maybe a 16K * 16K cursor is future proof and safe enough.
> >>
> >>>size_t datasize = width * height * sizeof(uint32_t);
>
> ---^
>
> >>> -- 2.40.1
> >>>
> >>>
> >>>
> >>>
> >>> --
> >>> Marc-André Lureau
> >>
> >>
> >
> > With regards,
> > Daniel
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] ui/cursor: incomplete check for integer overflow in cursor_alloc
On Tue, May 23, 2023 at 10:16 AM Daniel P. Berrangé wrote:
>
> On Mon, May 08, 2023 at 04:18:13PM +0200, Mauro Matteo Cascella wrote:
> > The cursor_alloc function still accepts a signed integer for both the cursor
> > width and height. A specially crafted negative width/height could make
> > datasize
> > wrap around and cause the next allocation to be 0, potentially leading to a
> > heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
> > accept unsigned ints.
> >
> I concur with Marc-Andre that there is no code path that can
> actually trigger an overflow:
>
>
> hw/display/ati.c:s->cursor = cursor_alloc(64, 64);
> hw/display/vhost-user-gpu.c:s->current_cursor =
> cursor_alloc(64, 64);
> hw/display/virtio-gpu.c:s->current_cursor = cursor_alloc(64,
> 64);
>
> Not exploitable as fixed size
>
> hw/display/qxl-render.c:c = cursor_alloc(cursor->header.width,
> cursor->header.height);
>
> Cursor header defined as:
>
> typedef struct SPICE_ATTR_PACKED QXLCursorHeader {
> uint64_t unique;
> uint16_t type;
> uint16_t width;
> uint16_t height;
> uint16_t hot_spot_x;
> uint16_t hot_spot_y;
> } QXLCursorHeader;
>
> So no negative values can be passed to cursor_alloc()
>
>
> hw/display/vmware_vga.c:qc = cursor_alloc(c->width, c->height);
>
> Where 'c' is defined as:
>
> struct vmsvga_cursor_definition_s {
> uint32_t width;
> uint32_t height;
> int id;
> uint32_t bpp;
> int hot_x;
> int hot_y;
> uint32_t mask[1024];
> uint32_t image[4096];
> };
>
> and is also already bounds checked:
>
> if (cursor.width > 256
> || cursor.height > 256
> || cursor.bpp > 32
> || SVGA_BITMAP_SIZE(x, y) > ARRAY_SIZE(cursor.mask)
> || SVGA_PIXMAP_SIZE(x, y, cursor.bpp)
> > ARRAY_SIZE(cursor.image)) {
> goto badcmd;
> }
>
> > Fixes: CVE-2023-1601
> > Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> > (CVE-2021-4206)")
>
> Given there is no possible codepath that can overflow, CVE-2023-1601
> looks invalid to me. It should be clsoed as not-a-bug and these two
> Fixes lines removed.
I think you can tweak the original PoC [1] to trigger this bug.
Setting width/height to 0x8000 (versus 0x8000) should do the
trick. You should be able to overflow datasize while bypassing the
sanity check (width > 512 || height > 512) as width/height are signed
prior to this patch. I haven't tested it, though.
[1] https://github.com/star-sg/CVE/blob/master/CVE-2021-4206/poc.c
[2] https://starlabs.sg/advisories/21/21-4206/
> > Signed-off-by: Mauro Matteo Cascella
> > Reported-by: Jacek Halon
> > ---
> > include/ui/console.h | 4 ++--
> > ui/cursor.c | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
>
> Even though it isn't fixing a bug, the change itself still makes
> sense, because there's no reason a negative width/height is ever
> appropriate. This protects us against accidentally introducing
> future bugs, so with the two CVE Fixes lines removed:
>
> Reviewed-by: Daniel P. Berrangé
>
>
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 2a8fab091f..92a4d90a1b 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
> >
> > /* cursor data format is 32bit RGBA */
> > typedef struct QEMUCursor {
> > -int width, height;
> > +uint32_twidth, height;
> > int hot_x, hot_y;
> > int refcount;
> > uint32_tdata[];
> > } QEMUCursor;
> >
> > -QEMUCursor *cursor_alloc(int width, int height);
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> > QEMUCursor *cursor_ref(QEMUCursor *c);
> > void cursor_unref(QEMUCursor *c);
> > QEMUCursor *cursor_builtin_hidden(void);
> > diff --git a/ui/cursor.c b/ui/cursor.c
> > index 6fe67990e2..b5fcb64839 100644
> > --- a/ui/cursor.c
> > +++ b/ui/cursor.c
> > @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> > return cursor_parse_xpm(cursor_left_ptr_xpm);
> > }
> >
> > -QEMUCursor *cursor_alloc(int width, int height)
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> > {
> > QEMUCursor *c;
> > size_t datasize = width * height * sizeof(uint32_t);
> > --
> > 2.40.1
> >
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] ui/cursor: incomplete check for integer overflow in cursor_alloc
On Mon, May 22, 2023 at 8:55 PM Philippe Mathieu-Daudé
wrote:
>
> On 9/5/23 09:13, Marc-André Lureau wrote:
> > Hi
> >
> > On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
> > mailto:mcasc...@redhat.com>> wrote:
> >
> > The cursor_alloc function still accepts a signed integer for both
> > the cursor
> > width and height. A specially crafted negative width/height could
> > make datasize
> > wrap around and cause the next allocation to be 0, potentially
> > leading to a
> > heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
> > prototype to
> > accept unsigned ints.
> >
> > Fixes: CVE-2023-1601
> > Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> > (CVE-2021-4206)")
> > Signed-off-by: Mauro Matteo Cascella > <mailto:mcasc...@redhat.com>>
> > Reported-by: Jacek Halon > <mailto:jacek.ha...@gmail.com>>
> >
> >
> > Reviewed-by: Marc-André Lureau > <mailto:marcandre.lur...@redhat.com>>
> >
> > It looks like this is not exploitable, QXL code uses u16 types, and
>
> 0x * 0x * 4 still overflows on 32-bit host, right?
>
> > VMWare VGA checks for values > 256. Other paths use fixed size.
> >
> > ---
> > include/ui/console.h | 4 ++--
> > ui/cursor.c | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 2a8fab091f..92a4d90a1b 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
> >
> > /* cursor data format is 32bit RGBA */
> > typedef struct QEMUCursor {
> > -int width, height;
> > +uint32_twidth, height;
> > int hot_x, hot_y;
> > int refcount;
> > uint32_tdata[];
> > } QEMUCursor;
> >
> > -QEMUCursor *cursor_alloc(int width, int height);
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> > QEMUCursor *cursor_ref(QEMUCursor *c);
> > void cursor_unref(QEMUCursor *c);
> > QEMUCursor *cursor_builtin_hidden(void);
> > diff --git a/ui/cursor.c b/ui/cursor.c
> > index 6fe67990e2..b5fcb64839 100644
> > --- a/ui/cursor.c
> > +++ b/ui/cursor.c
> > @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> > return cursor_parse_xpm(cursor_left_ptr_xpm);
> > }
> >
> > -QEMUCursor *cursor_alloc(int width, int height)
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> > {
> > QEMUCursor *c;
>
> Can't we check width/height > 0 && <= SOME_LIMIT_THAT_MAKES_SENSE?
We currently ensure width/height are less than 512 in cursor_alloc.
Checking for positive values is unnecessary if we make width/height
unsigned, isn't it?
> Maybe a 16K * 16K cursor is future proof and safe enough.
>
> > size_t datasize = width * height * sizeof(uint32_t);
> > --
> > 2.40.1
> >
> >
> >
> >
> > --
> > Marc-André Lureau
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] ui/cursor: incomplete check for integer overflow in cursor_alloc
On Mon, May 8, 2023 at 4:20 PM Mauro Matteo Cascella
wrote:
>
> The cursor_alloc function still accepts a signed integer for both the cursor
> width and height. A specially crafted negative width/height could make
> datasize
> wrap around and cause the next allocation to be 0, potentially leading to a
> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
> accept unsigned ints.
>
> Fixes: CVE-2023-1601
> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> (CVE-2021-4206)")
> Signed-off-by: Mauro Matteo Cascella
> Reported-by: Jacek Halon
Addendum:
Reported-by: Yair Mizrahi
Reported-by: Elsayed El-Refa'ei
> ---
> include/ui/console.h | 4 ++--
> ui/cursor.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 2a8fab091f..92a4d90a1b 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
>
> /* cursor data format is 32bit RGBA */
> typedef struct QEMUCursor {
> -int width, height;
> +uint32_twidth, height;
> int hot_x, hot_y;
> int refcount;
> uint32_tdata[];
> } QEMUCursor;
>
> -QEMUCursor *cursor_alloc(int width, int height);
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> QEMUCursor *cursor_ref(QEMUCursor *c);
> void cursor_unref(QEMUCursor *c);
> QEMUCursor *cursor_builtin_hidden(void);
> diff --git a/ui/cursor.c b/ui/cursor.c
> index 6fe67990e2..b5fcb64839 100644
> --- a/ui/cursor.c
> +++ b/ui/cursor.c
> @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> return cursor_parse_xpm(cursor_left_ptr_xpm);
> }
>
> -QEMUCursor *cursor_alloc(int width, int height)
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> {
> QEMUCursor *c;
> size_t datasize = width * height * sizeof(uint32_t);
> --
> 2.40.1
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: RE: [PATCH] virtio-crypto: fix NULL pointer dereference in virtio_crypto_free_request
On Tue, May 9, 2023 at 3:47 AM zhenwei pi wrote:
>
>
>
> On 5/9/23 09:02, Gonglei (Arei) wrote:
> >
> >
> >> -Original Message-
> >> From: Mauro Matteo Cascella [mailto:mcasc...@redhat.com]
> >> Sent: Monday, May 8, 2023 11:02 PM
> >> To: qemu-devel@nongnu.org
> >> Cc: m...@redhat.com; Gonglei (Arei) ;
> >> pizhen...@bytedance.com; ta...@zju.edu.cn; mcasc...@redhat.com
> >> Subject: [PATCH] virtio-crypto: fix NULL pointer dereference in
> >> virtio_crypto_free_request
> >>
> >> Ensure op_info is not NULL in case of QCRYPTODEV_BACKEND_ALG_SYM
> >> algtype.
> >>
> >> Fixes: 02ed3e7c ("virtio-crypto: zeroize the key material before free")
> >
> > I have to say the fixes is incorrect. The bug was introduced by commit
> > 0e660a6f90a, which
> > changed the semantic meaning of request-> flag.
> >
> > Regards,
> > -Gonglei
> >
>
> Hi Mauro
>
> Agree with Lei, could you please change the Fixes as Lei suggested?
Sent v2.
Thanks!
> --
> zhenwei pi
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH v2] virtio-crypto: fix NULL pointer dereference in virtio_crypto_free_request
Ensure op_info is not NULL in case of QCRYPTODEV_BACKEND_ALG_SYM algtype.
Fixes: 0e660a6f90a ("crypto: Introduce RSA algorithm")
Signed-off-by: Mauro Matteo Cascella
Reported-by: Yiming Tao
---
v2:
- updated 'Fixes:' tag
hw/virtio/virtio-crypto.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 2fe804510f..c729a1f79e 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -476,15 +476,17 @@ static void virtio_crypto_free_request(VirtIOCryptoReq
*req)
size_t max_len;
CryptoDevBackendSymOpInfo *op_info = req->op_info.u.sym_op_info;
-max_len = op_info->iv_len +
- op_info->aad_len +
- op_info->src_len +
- op_info->dst_len +
- op_info->digest_result_len;
-
-/* Zeroize and free request data structure */
-memset(op_info, 0, sizeof(*op_info) + max_len);
-g_free(op_info);
+if (op_info) {
+max_len = op_info->iv_len +
+ op_info->aad_len +
+ op_info->src_len +
+ op_info->dst_len +
+ op_info->digest_result_len;
+
+/* Zeroize and free request data structure */
+memset(op_info, 0, sizeof(*op_info) + max_len);
+g_free(op_info);
+}
} else if (req->flags == QCRYPTODEV_BACKEND_ALG_ASYM) {
CryptoDevBackendAsymOpInfo *op_info = req->op_info.u.asym_op_info;
if (op_info) {
--
2.40.1
[PATCH] virtio-crypto: fix NULL pointer dereference in virtio_crypto_free_request
Ensure op_info is not NULL in case of QCRYPTODEV_BACKEND_ALG_SYM algtype.
Fixes: 02ed3e7c ("virtio-crypto: zeroize the key material before free")
Signed-off-by: Mauro Matteo Cascella
Reported-by: Yiming Tao
---
hw/virtio/virtio-crypto.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 2fe804510f..c729a1f79e 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -476,15 +476,17 @@ static void virtio_crypto_free_request(VirtIOCryptoReq
*req)
size_t max_len;
CryptoDevBackendSymOpInfo *op_info = req->op_info.u.sym_op_info;
-max_len = op_info->iv_len +
- op_info->aad_len +
- op_info->src_len +
- op_info->dst_len +
- op_info->digest_result_len;
-
-/* Zeroize and free request data structure */
-memset(op_info, 0, sizeof(*op_info) + max_len);
-g_free(op_info);
+if (op_info) {
+max_len = op_info->iv_len +
+ op_info->aad_len +
+ op_info->src_len +
+ op_info->dst_len +
+ op_info->digest_result_len;
+
+/* Zeroize and free request data structure */
+memset(op_info, 0, sizeof(*op_info) + max_len);
+g_free(op_info);
+}
} else if (req->flags == QCRYPTODEV_BACKEND_ALG_ASYM) {
CryptoDevBackendAsymOpInfo *op_info = req->op_info.u.asym_op_info;
if (op_info) {
--
2.40.1
[PATCH] ui/cursor: incomplete check for integer overflow in cursor_alloc
The cursor_alloc function still accepts a signed integer for both the cursor
width and height. A specially crafted negative width/height could make datasize
wrap around and cause the next allocation to be 0, potentially leading to a
heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
accept unsigned ints.
Fixes: CVE-2023-1601
Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
(CVE-2021-4206)")
Signed-off-by: Mauro Matteo Cascella
Reported-by: Jacek Halon
---
include/ui/console.h | 4 ++--
ui/cursor.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 2a8fab091f..92a4d90a1b 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
/* cursor data format is 32bit RGBA */
typedef struct QEMUCursor {
-int width, height;
+uint32_twidth, height;
int hot_x, hot_y;
int refcount;
uint32_tdata[];
} QEMUCursor;
-QEMUCursor *cursor_alloc(int width, int height);
+QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
QEMUCursor *cursor_ref(QEMUCursor *c);
void cursor_unref(QEMUCursor *c);
QEMUCursor *cursor_builtin_hidden(void);
diff --git a/ui/cursor.c b/ui/cursor.c
index 6fe67990e2..b5fcb64839 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
return cursor_parse_xpm(cursor_left_ptr_xpm);
}
-QEMUCursor *cursor_alloc(int width, int height)
+QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
{
QEMUCursor *c;
size_t datasize = width * height * sizeof(uint32_t);
--
2.40.1
Re: [PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)
On Fri, Mar 17, 2023 at 10:59 PM Philippe Mathieu-Daudé
wrote:
>
> On 17/3/23 19:18, Karl Heubaum wrote:
> > Did this CVE fix fall in the cracks during the QEMU 8.0 merge window?
>
> The patch isn't reviewed, and apparently almost no active contributor
> understand this device enough to be sure this security patch doesn't
> break normal use. Fuzzed bugs are rarely trivial.
I guess Alexander's new patchset [1] does not help fix this one?
[1]
https://patchew.org/QEMU/20230313082417.827484-1-alx...@bu.edu/20230313082417.827484-7-alx...@bu.edu/
> >> On Jan 16, 2023, at 2:42 PM, Mauro Matteo Cascella
> >> wrote:
> >>
> >> This prevents the well known DMA-MMIO reentrancy problem (upstream issue
> >> #556)
> >> leading to memory corruption bugs like stack overflow or use-after-free.
> >>
> >> Fixes: CVE-2023-0330
> >> Signed-off-by: Mauro Matteo Cascella
> >> Reported-by: Zheyu Ma
> >> ---
> >> hw/scsi/lsi53c895a.c | 14 +
> >> tests/qtest/fuzz-lsi53c895a-test.c | 32 ++
> >> 2 files changed, 42 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> >> index af93557a9a..89c52594eb 100644
> >> --- a/hw/scsi/lsi53c895a.c
> >> +++ b/hw/scsi/lsi53c895a.c
> >> @@ -446,22 +446,28 @@ static void lsi_reselect(LSIState *s, lsi_request
> >> *p);
> >> static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
> >> void *buf, dma_addr_t len)
> >> {
> >> +const MemTxAttrs attrs = { .memory = true };
> >> +
> >> if (s->dmode & LSI_DMODE_SIOM) {
> >> -address_space_read(>pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> >> +address_space_read(>pci_io_as, addr, attrs,
> >> buf, len);
> >> } else {
> >> -pci_dma_read(PCI_DEVICE(s), addr, buf, len);
> >> +pci_dma_rw(PCI_DEVICE(s), addr, buf, len,
> >> + DMA_DIRECTION_TO_DEVICE, attrs);
> >> }
> >> }
> >>
> >> static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
> >> const void *buf, dma_addr_t len)
> >> {
> >> +const MemTxAttrs attrs = { .memory = true };
> >> +
> >> if (s->dmode & LSI_DMODE_DIOM) {
> >> -address_space_write(>pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> >> +address_space_write(>pci_io_as, addr, attrs,
> >> buf, len);
> >> } else {
> >> -pci_dma_write(PCI_DEVICE(s), addr, buf, len);
> >> +pci_dma_rw(PCI_DEVICE(s), addr, (void *) buf, len,
> >> + DMA_DIRECTION_FROM_DEVICE, attrs);
> >> }
> >> }
> >>
> >> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c
> >> b/tests/qtest/fuzz-lsi53c895a-test.c
> >> index 392a7ae7ed..35c02e89f3 100644
> >> --- a/tests/qtest/fuzz-lsi53c895a-test.c
> >> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> >> @@ -8,6 +8,35 @@
> >> #include "qemu/osdep.h"
> >> #include "libqtest.h"
> >>
> >> +/*
> >> + * This used to trigger a DMA reentrancy issue
> >> + * leading to memory corruption bugs like stack
> >> + * overflow or use-after-free
> >> + */
> >> +static void test_lsi_dma_reentrancy(void)
> >> +{
> >> +QTestState *s;
> >> +
> >> +s = qtest_init("-M q35 -m 512M -nodefaults "
> >> + "-blockdev driver=null-co,node-name=null0 "
> >> + "-device lsi53c810 -device scsi-cd,drive=null0");
> >> +
> >> +qtest_outl(s, 0xcf8, 0x8804); /* PCI Command Register */
> >> +qtest_outw(s, 0xcfc, 0x7);/* Enables accesses */
> >> +qtest_outl(s, 0xcf8, 0x8814); /* Memory Bar 1 */
> >> +qtest_outl(s, 0xcfc, 0xff10); /* Set MMIO Address*/
> >> +qtest_outl(s, 0xcf8, 0x8818); /* Memory Bar 2 */
> >> +qtest_outl(s, 0xcfc, 0xff00); /* Set RAM Address*/
> >> +qtest_writel(s, 0xff00, 0xc024);
> >> +qtest_writel(s, 0xff000114, 0x0080);
> >> +qtest_writel(s, 0xff00012c, 0xff00);
> >> +qtest_writel(s, 0xff04, 0xff000114);
> >> +qtest_writel(s, 0xff08, 0xff100014);
> >> +qtest_writel(s, 0xff10002f, 0x00ff);
> >> +
> >> +qtest_quit(s);
> >> +}
> >> +
> >> /*
> >> * This used to trigger a UAF in lsi_do_msgout()
> >> * https://gitlab.com/qemu-project/qemu/-/issues/972
> >> @@ -120,5 +149,8 @@ int main(int argc, char **argv)
> >> qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
> >> test_lsi_do_msgout_cancel_req);
> >>
> >> +qtest_add_func("fuzz/lsi53c895a/lsi_dma_reentrancy",
> >> + test_lsi_dma_reentrancy);
> >> +
> >> return g_test_run();
> >> }
> >> --
> >> 2.39.0
> >>
> >>
> >
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH v1] hw/pvrdma: Protect against buggy or malicious guest driver
Hi Yuval, Dropping and . This is CVE-2023-1544. The patch looks good to me. Thank you. On Mon, Mar 20, 2023 at 1:07 PM Yuval Shaia wrote: > > Hi, > Patch is currently under review. > From my end, it was tested and proved to solve the problem. > > To follow up you may need to check qemu-devel@nongnu.org from time to time. > > Marcel, any feedback? -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH 1/2] qga/win32: Remove change action from MSI installer
Hi Philippe, On Tue, Feb 21, 2023 at 9:15 AM Philippe Mathieu-Daudé wrote: > > On 20/2/23 18:41, Konstantin Kostiuk wrote: > > resolves: rhbz#2167436 > > "You are not authorized to access bug #2167436." Please refer to https://bugzilla.redhat.com/show_bug.cgi?id=2167423. It should now be accessible. > > fixes: CVE-2023-0664 > > This commit description is rather scarce... > > I understand you are trying to fix a CVE, but we shouldn't play > the "security by obscurity" card. How can the community and > distributions know this security fix is enough with the bare > "Remove change action from MSI installer" justification? > Can't we do better? CCing Brian Wiltse, who originally found and reported this issue. Reported-by: Brian Wiltse > > Signed-off-by: Konstantin Kostiuk > > --- > > qga/installer/qemu-ga.wxs | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs > > index 51340f7ecc..feb629ec47 100644 > > --- a/qga/installer/qemu-ga.wxs > > +++ b/qga/installer/qemu-ga.wxs > > @@ -31,6 +31,7 @@ > > /> > >> EmbedCab="yes" /> > > 1 > > + > >> DowngradeErrorMessage="Error: A newer version of QEMU guest agent > > is already installed." > > /> > > -- > > 2.25.1 > > > > > -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] usb/dev-wacom: fix OOB write in usb_mouse_poll()
Hi Philippe,
On Mon, Feb 13, 2023 at 7:26 PM Philippe Mathieu-Daudé
wrote:
>
> Hi Mauro,
>
> On 13/2/23 18:41, Mauro Matteo Cascella wrote:
> > The guest can control the size of buf; an OOB write occurs when buf is 1 or
> > 2
> > bytes long. Only fill in the buffer as long as there is enough space, throw
> > away any data which doesn't fit.
>
> Any reproducer?
No qtest reproducer, we do have a PoC module to compile & load from
within the guest. This is ASAN output:
=
==28803==ERROR: AddressSanitizer: heap-buffer-overflow on address 0
WRITE of size 1 at 0x602000fccdd1 thread T2
#0 0x560f4ebba899 in usb_mouse_poll ../hw/usb/dev-wacom.c:256
#1 0x560f4ebbcaf9 in usb_wacom_handle_data ../hw/usb/dev-wacom6
#2 0x560f4eaef297 in usb_device_handle_data ../hw/usb/bus.c:180
#3 0x560f4eb00bbb in usb_process_one ../hw/usb/core.c:406
#4 0x560f4eb01883 in usb_handle_packet ../hw/usb/core.c:438
#5 0x560f4eb94e0c in xhci_submit ../hw/usb/hcd-xhci.c:1801
#6 0x560f4eb9505f in xhci_fire_transfer ../hw/usb/hcd-xhci.c:10
#7 0x560f4eb9773c in xhci_kick_epctx ../hw/usb/hcd-xhci.c:1969
#8 0x560f4eb953f2 in xhci_kick_ep ../hw/usb/hcd-xhci.c:1835
#9 0x560f4eba416d in xhci_doorbell_write ../hw/usb/hcd-xhci.c:7
#10 0x560f4f5343a8 in memory_region_write_accessor ../softmmu/2
#11 0x560f4f53483f in access_with_adjusted_size ../softmmu/mem4
#12 0x560f4f541e69 in memory_region_dispatch_write ../softmmu/4
#13 0x560f4f57afec in flatview_write_continue ../softmmu/physm5
#14 0x560f4f57b40f in flatview_write ../softmmu/physmem.c:2867
#15 0x560f4f579617 in subpage_write ../softmmu/physmem.c:2501
#16 0x560f4f5346dc in memory_region_write_with_attrs_accessor 3
#17 0x560f4f53483f in access_with_adjusted_size ../softmmu/mem4
#18 0x560f4f542075 in memory_region_dispatch_write ../softmmu/1
#19 0x560f4f727735 in io_writex ../accel/tcg/cputlb.c:1429
#20 0x560f4f72c19d in store_helper ../accel/tcg/cputlb.c:2379
#21 0x560f4f72c5ec in full_le_stl_mmu ../accel/tcg/cputlb.c:247
#22 0x560f4f72c62a in helper_le_stl_mmu ../accel/tcg/cputlb.c:3
#23 0x7fcf063941a3 (/memfd:tcg-jit (deleted)+0x27541a3)
Also forgot to give credits:
Reported-by: ningqiang1
Reported-by: SorryMybad of Kunlun Lab
> > Signed-off-by: Mauro Matteo Cascella
> > ---
> > hw/usb/dev-wacom.c | 20 +---
> > 1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c
> > index 7177c17f03..ca9e6aa82f 100644
> > --- a/hw/usb/dev-wacom.c
> > +++ b/hw/usb/dev-wacom.c
> > @@ -252,14 +252,20 @@ static int usb_mouse_poll(USBWacomState *s, uint8_t
> > *buf, int len)
> > if (s->buttons_state & MOUSE_EVENT_MBUTTON)
> > b |= 0x04;
> >
> > -buf[0] = b;
> > -buf[1] = dx;
> > -buf[2] = dy;
> > -l = 3;
> > -if (len >= 4) {
> > -buf[3] = dz;
> > -l = 4;
> > +l = 0;
> > +if (len > l) {
> > +buf[l++] = b;
> > }
> > +if (len > l) {
> > +buf[l++] = dx;
> > +}
>
> else { // the packet is now corrupted... }
>
> > +if (len > l) {
> > +buf[l++] = dy;
> > +}
> > +if (len > l) {
> > +buf[l++] = dz;
> > +}
> > +
> > return l;
> > }
>
> Better is to wait for enough data to process:
>
> -- >8 --
> diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c
> index 7177c17f03..2fe2a9220e 100644
> --- a/hw/usb/dev-wacom.c
> +++ b/hw/usb/dev-wacom.c
> @@ -244,6 +244,9 @@ static int usb_mouse_poll(USBWacomState *s, uint8_t
> *buf, int len)
> s->dy -= dy;
> s->dz -= dz;
>
> +if (len < 3)
> +return 0;
> +
> b = 0;
> if (s->buttons_state & MOUSE_EVENT_LBUTTON)
> b |= 0x01;
> @@ -274,6 +277,9 @@ static int usb_wacom_poll(USBWacomState *s, uint8_t
> *buf, int len)
> s->mouse_grabbed = 1;
> }
>
> +if (len < 7)
> +return 0;
> +
> b = 0;
> if (s->buttons_state & MOUSE_EVENT_LBUTTON)
> b |= 0x01;
> @@ -282,9 +288,6 @@ static int usb_wacom_poll(USBWacomState *s, uint8_t
> *buf, int len)
> if (s->buttons_state & MOUSE_EVENT_MBUTTON)
> b |= 0x20; /* eraser */
>
> -if (len < 7)
> -return 0;
> -
> buf[0] = s->mode;
> buf[5] = 0x00 | (b & 0xf0);
> buf[1] = s->x & 0xff;
> ---
>
I took inspiration from hid_pointer_poll() in hw/input/hid.c which
fills in the buffer as much as possible in a similar way, but your
suggestion makes sense to me. Gerd, wdyt?
Thanks,
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH] usb/dev-wacom: fix OOB write in usb_mouse_poll()
The guest can control the size of buf; an OOB write occurs when buf is 1 or 2
bytes long. Only fill in the buffer as long as there is enough space, throw
away any data which doesn't fit.
Signed-off-by: Mauro Matteo Cascella
---
hw/usb/dev-wacom.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c
index 7177c17f03..ca9e6aa82f 100644
--- a/hw/usb/dev-wacom.c
+++ b/hw/usb/dev-wacom.c
@@ -252,14 +252,20 @@ static int usb_mouse_poll(USBWacomState *s, uint8_t *buf,
int len)
if (s->buttons_state & MOUSE_EVENT_MBUTTON)
b |= 0x04;
-buf[0] = b;
-buf[1] = dx;
-buf[2] = dy;
-l = 3;
-if (len >= 4) {
-buf[3] = dz;
-l = 4;
+l = 0;
+if (len > l) {
+buf[l++] = b;
}
+if (len > l) {
+buf[l++] = dx;
+}
+if (len > l) {
+buf[l++] = dy;
+}
+if (len > l) {
+buf[l++] = dz;
+}
+
return l;
}
--
2.39.1
Re: [PATCH] nubus-device: fix memory leak in nubus_device_realize
On Thu, Dec 22, 2022 at 6:29 PM Mauro Matteo Cascella
wrote:
>
> Local variable "name" is allocated through strdup_printf and should be
> freed with g_free() to avoid memory leak.
>
> Fixes: 3616f424 ("nubus-device: add romfile property for loading declaration
> ROMs")
> Signed-off-by: Mauro Matteo Cascella
> ---
> hw/nubus/nubus-device.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
> index 0f1852f671..49008e4938 100644
> --- a/hw/nubus/nubus-device.c
> +++ b/hw/nubus/nubus-device.c
> @@ -80,6 +80,7 @@ static void nubus_device_realize(DeviceState *dev, Error
> **errp)
> _abort);
> ret = load_image_mr(path, >decl_rom);
> g_free(path);
> +g_free(name);
> if (ret < 0) {
> error_setg(errp, "could not load romfile \"%s\"", nd->romfile);
> return;
> --
> 2.38.1
Hi, any updates here? Is this patch going to be merged?
Thanks,
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)
On Mon, Jan 16, 2023 at 9:42 PM Mauro Matteo Cascella
wrote:
>
> This prevents the well known DMA-MMIO reentrancy problem (upstream issue #556)
> leading to memory corruption bugs like stack overflow or use-after-free.
>
> Fixes: CVE-2023-0330
> Signed-off-by: Mauro Matteo Cascella
> Reported-by: Zheyu Ma
> ---
> hw/scsi/lsi53c895a.c | 14 +
> tests/qtest/fuzz-lsi53c895a-test.c | 32 ++
> 2 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index af93557a9a..89c52594eb 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -446,22 +446,28 @@ static void lsi_reselect(LSIState *s, lsi_request *p);
> static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
> void *buf, dma_addr_t len)
> {
> +const MemTxAttrs attrs = { .memory = true };
> +
> if (s->dmode & LSI_DMODE_SIOM) {
> -address_space_read(>pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> +address_space_read(>pci_io_as, addr, attrs,
> buf, len);
> } else {
> -pci_dma_read(PCI_DEVICE(s), addr, buf, len);
> +pci_dma_rw(PCI_DEVICE(s), addr, buf, len,
> + DMA_DIRECTION_TO_DEVICE, attrs);
> }
> }
>
> static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
> const void *buf, dma_addr_t len)
> {
> +const MemTxAttrs attrs = { .memory = true };
> +
> if (s->dmode & LSI_DMODE_DIOM) {
> -address_space_write(>pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> +address_space_write(>pci_io_as, addr, attrs,
> buf, len);
> } else {
> -pci_dma_write(PCI_DEVICE(s), addr, buf, len);
> +pci_dma_rw(PCI_DEVICE(s), addr, (void *) buf, len,
> + DMA_DIRECTION_FROM_DEVICE, attrs);
> }
> }
>
> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c
> b/tests/qtest/fuzz-lsi53c895a-test.c
> index 392a7ae7ed..35c02e89f3 100644
> --- a/tests/qtest/fuzz-lsi53c895a-test.c
> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> @@ -8,6 +8,35 @@
> #include "qemu/osdep.h"
> #include "libqtest.h"
>
> +/*
> + * This used to trigger a DMA reentrancy issue
> + * leading to memory corruption bugs like stack
> + * overflow or use-after-free
> + */
> +static void test_lsi_dma_reentrancy(void)
> +{
> +QTestState *s;
> +
> +s = qtest_init("-M q35 -m 512M -nodefaults "
> + "-blockdev driver=null-co,node-name=null0 "
> + "-device lsi53c810 -device scsi-cd,drive=null0");
> +
> +qtest_outl(s, 0xcf8, 0x8804); /* PCI Command Register */
> +qtest_outw(s, 0xcfc, 0x7);/* Enables accesses */
> +qtest_outl(s, 0xcf8, 0x8814); /* Memory Bar 1 */
> +qtest_outl(s, 0xcfc, 0xff10); /* Set MMIO Address*/
> +qtest_outl(s, 0xcf8, 0x8818); /* Memory Bar 2 */
> +qtest_outl(s, 0xcfc, 0xff00); /* Set RAM Address*/
> +qtest_writel(s, 0xff00, 0xc024);
> +qtest_writel(s, 0xff000114, 0x0080);
> +qtest_writel(s, 0xff00012c, 0xff00);
> +qtest_writel(s, 0xff04, 0xff000114);
> +qtest_writel(s, 0xff08, 0xff100014);
> +qtest_writel(s, 0xff10002f, 0x00ff);
> +
> +qtest_quit(s);
> +}
> +
> /*
> * This used to trigger a UAF in lsi_do_msgout()
> * https://gitlab.com/qemu-project/qemu/-/issues/972
> @@ -120,5 +149,8 @@ int main(int argc, char **argv)
> qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
> test_lsi_do_msgout_cancel_req);
>
> +qtest_add_func("fuzz/lsi53c895a/lsi_dma_reentrancy",
> + test_lsi_dma_reentrancy);
> +
> return g_test_run();
> }
> --
> 2.39.0
>
Reproducer:
cat << EOF | ./x86_64-softmmu/qemu-system-x86_64 -machine accel=qtest \
-m 512M -machine q35 -nodefaults -device lsi53c810 -device scsi-cd,drive=null0 \
-display none -blockdev driver=null-co,node-name=null0 -qtest stdio
outl 0xcf8 0x8804 /* PCI Command Register */
outl 0xcfc 0x7 /* Enable accesses */
outl 0xcf8 0x8814 /* Memory Bar 1 */
outl 0xcfc 0xff10 /* Set MMIO Address*/
outl 0xcf8 0x8818 /* Memory Bar 2 */
outl 0xcfc 0xff00 /* Set RAM Address*/
writel 0xff00 0xc024
writel 0xff000114 0x0080
writel 0xff00012c 0xff00
writel 0xff04 0xff000114
writel 0xff08 0xff100014
writel 0xff10002f 0x00ff
EOF
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)
This prevents the well known DMA-MMIO reentrancy problem (upstream issue #556)
leading to memory corruption bugs like stack overflow or use-after-free.
Fixes: CVE-2023-0330
Signed-off-by: Mauro Matteo Cascella
Reported-by: Zheyu Ma
---
hw/scsi/lsi53c895a.c | 14 +
tests/qtest/fuzz-lsi53c895a-test.c | 32 ++
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index af93557a9a..89c52594eb 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -446,22 +446,28 @@ static void lsi_reselect(LSIState *s, lsi_request *p);
static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
void *buf, dma_addr_t len)
{
+const MemTxAttrs attrs = { .memory = true };
+
if (s->dmode & LSI_DMODE_SIOM) {
-address_space_read(>pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_read(>pci_io_as, addr, attrs,
buf, len);
} else {
-pci_dma_read(PCI_DEVICE(s), addr, buf, len);
+pci_dma_rw(PCI_DEVICE(s), addr, buf, len,
+ DMA_DIRECTION_TO_DEVICE, attrs);
}
}
static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
const void *buf, dma_addr_t len)
{
+const MemTxAttrs attrs = { .memory = true };
+
if (s->dmode & LSI_DMODE_DIOM) {
-address_space_write(>pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_write(>pci_io_as, addr, attrs,
buf, len);
} else {
-pci_dma_write(PCI_DEVICE(s), addr, buf, len);
+pci_dma_rw(PCI_DEVICE(s), addr, (void *) buf, len,
+ DMA_DIRECTION_FROM_DEVICE, attrs);
}
}
diff --git a/tests/qtest/fuzz-lsi53c895a-test.c
b/tests/qtest/fuzz-lsi53c895a-test.c
index 392a7ae7ed..35c02e89f3 100644
--- a/tests/qtest/fuzz-lsi53c895a-test.c
+++ b/tests/qtest/fuzz-lsi53c895a-test.c
@@ -8,6 +8,35 @@
#include "qemu/osdep.h"
#include "libqtest.h"
+/*
+ * This used to trigger a DMA reentrancy issue
+ * leading to memory corruption bugs like stack
+ * overflow or use-after-free
+ */
+static void test_lsi_dma_reentrancy(void)
+{
+QTestState *s;
+
+s = qtest_init("-M q35 -m 512M -nodefaults "
+ "-blockdev driver=null-co,node-name=null0 "
+ "-device lsi53c810 -device scsi-cd,drive=null0");
+
+qtest_outl(s, 0xcf8, 0x8804); /* PCI Command Register */
+qtest_outw(s, 0xcfc, 0x7);/* Enables accesses */
+qtest_outl(s, 0xcf8, 0x8814); /* Memory Bar 1 */
+qtest_outl(s, 0xcfc, 0xff10); /* Set MMIO Address*/
+qtest_outl(s, 0xcf8, 0x8818); /* Memory Bar 2 */
+qtest_outl(s, 0xcfc, 0xff00); /* Set RAM Address*/
+qtest_writel(s, 0xff00, 0xc024);
+qtest_writel(s, 0xff000114, 0x0080);
+qtest_writel(s, 0xff00012c, 0xff00);
+qtest_writel(s, 0xff04, 0xff000114);
+qtest_writel(s, 0xff08, 0xff100014);
+qtest_writel(s, 0xff10002f, 0x00ff);
+
+qtest_quit(s);
+}
+
/*
* This used to trigger a UAF in lsi_do_msgout()
* https://gitlab.com/qemu-project/qemu/-/issues/972
@@ -120,5 +149,8 @@ int main(int argc, char **argv)
qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
test_lsi_do_msgout_cancel_req);
+qtest_add_func("fuzz/lsi53c895a/lsi_dma_reentrancy",
+ test_lsi_dma_reentrancy);
+
return g_test_run();
}
--
2.39.0
[PATCH] nubus-device: fix memory leak in nubus_device_realize
Local variable "name" is allocated through strdup_printf and should be
freed with g_free() to avoid memory leak.
Fixes: 3616f424 ("nubus-device: add romfile property for loading declaration
ROMs")
Signed-off-by: Mauro Matteo Cascella
---
hw/nubus/nubus-device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index 0f1852f671..49008e4938 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -80,6 +80,7 @@ static void nubus_device_realize(DeviceState *dev, Error
**errp)
_abort);
ret = load_image_mr(path, >decl_rom);
g_free(path);
+g_free(name);
if (ret < 0) {
error_setg(errp, "could not load romfile \"%s\"", nd->romfile);
return;
--
2.38.1
Re: [RFC PATCH-for-7.2 0/4] hw/display/qxl: Avoid buffer overrun in qxl_phys2virt()
On Fri, Nov 25, 2022 at 4:40 PM Philippe Mathieu-Daudé wrote: > > memory_region_get_ram_ptr() returns a host pointer for a > MemoryRegion. Sometimes we do offset calculation using this > pointer without checking the underlying MemoryRegion size. > > Wenxu Yin reported a buffer overrun in QXL. This series > aims to fix it. I haven't audited the other _get_ram_ptr() > uses (yet). Eventually we could rename it _get_ram_ptr_unsafe > and add a safer helper which checks for overrun. This is now CVE-2022-4144. Please add proper "Fixes:" tag, if possible. Thank you for the fix. > Worth considering for 7.2? > > Regards, > > Phil. > > Philippe Mathieu-Daudé (4): > hw/display/qxl: Have qxl_log_command Return early if no log_cmd > handler > hw/display/qxl: Document qxl_phys2virt() > hw/display/qxl: Pass qxl_phys2virt size > hw/display/qxl: Avoid buffer overrun in qxl_phys2virt() > > hw/display/qxl-logger.c | 22 +++--- > hw/display/qxl-render.c | 11 +++ > hw/display/qxl.c| 25 +++-- > hw/display/qxl.h| 23 ++++++- > 4 files changed, 67 insertions(+), 14 deletions(-) > > -- > 2.38.1 > -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] hw/sd/sdhci: reset data count in sdhci_buff_access_is_sequential()
On Wed, Nov 9, 2022 at 5:19 PM Bin Meng wrote: > > On Wed, Nov 9, 2022 at 6:10 PM Mauro Matteo Cascella > wrote: > > > > On Wed, Nov 9, 2022 at 10:45 AM Siqi Chen wrote: > > > > > > Hi, > > > > > > >This reproducer does not crash my QEMU. Am I missing anything? > > > I submitted the reproducer. Because the overflow is only one byte, it may > > > not be detected by the host's heap allocator. Do you compile your qemu > > > with sanitizer? This is my build configuration: "./configure > > > --target-list=x86_64-softmmu --enable-sanitizers" > > > > Right, you need to recompile QEMU with ASAN support. This is an > > excerpt of the stack trace: > > Is this documented somewhere? Is fuzzing.rst the right doc for this > feature? Looking at fuzzing.rst it says --enable-sanitizers is > optional. Not sure it's documented somewhere, this is how I usually compile: $ ../configure --target-list=x86_64-softmmu --enable-sanitizers --extra-cflags=-g3 \ --enable-kvm --disable-tcg --enable-debug --enable-debug-info --disable-werror Then just run the PoC using ./x86_64-softmmu/qemu-system-x86_64 should do the trick. > Turning on --enable-sanitizers makes the build fail: > > FAILED: subprojects/libvduse/libvduse.a.p/libvduse.c.o > cc -m64 -mcx16 -Isubprojects/libvduse/libvduse.a.p > -Isubprojects/libvduse -I../subprojects/libvduse > -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g > -fsanitize=undefined -fsanitize=address -U_FORTIFY > _SOURCE -D_FORTIFY_SOURCE=2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE > -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings > -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv > -Wold-style-declaration -W > old-style-definition -Wtype-limits -Wformat-security -Wformat-y2k > -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs > -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 > -Wno-missing-include-dirs -Wn > o-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE > -D_GNU_SOURCE -MD -MQ subprojects/libvduse/libvduse.a.p/libvduse.c.o > -MF subprojects/libvduse/libvduse.a.p/libvduse.c.o.d -o > subprojects/libvduse/libvduse.a > .p/libvduse.c.o -c ../subprojects/libvduse/libvduse.c > In file included from /usr/include/string.h:495, > from ../subprojects/libvduse/libvduse.c:24: > In function ‘strncpy’, > inlined from ‘vduse_dev_create’ at ../subprojects/libvduse/libvduse.c:1312:5: > /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: > ‘__builtin_strncpy’ specified bound 256 equals destination size > [-Werror=stringop-truncation] > 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); > | ^~ > cc1: all warnings being treated as errors > > I am using GCC 9.4 on Ubuntu 20.04 > > > > > = > > ==39159==ERROR: AddressSanitizer: heap-buffer-overflow on address > > 0x61522880 at pc 0x55b023db5fe1 bp 0x7fffeeef1650 sp > > 0x7fffeeef1648 > > WRITE of size 1 at 0x61522880 thread T0 > > #0 0x55b023db5fe0 in sdhci_write_dataport ../../hw/sd/sdhci.c:562 > > #1 0x55b023dc1cc7 in sdhci_write ../../hw/sd/sdhci.c:1216 > > #2 0x55b024521e01 in memory_region_write_accessor > > ../../softmmu/memory.c:492 > > #3 0x55b0245222ab in access_with_adjusted_size > > ../../softmmu/memory.c:554 > > #4 0x55b02452ff15 in memory_region_dispatch_write > > ../../softmmu/memory.c:1514 > > #5 0x55b024568c67 in flatview_write_continue > > ../../softmmu/physmem.c:2814 > > #6 0x55b02456908d in flatview_write ../../softmmu/physmem.c:2856 > > #7 0x55b024569a74 in address_space_write ../../softmmu/physmem.c:2952 > > #8 0x55b02457a63c in qtest_process_command ../../softmmu/qtest.c:538 > > #9 0x55b02457ef97 in qtest_process_inbuf ../../softmmu/qtest.c:796 > > #10 0x55b02457f089 in qtest_read ../../softmmu/qtest.c:808 > > #11 0x55b0249d4372 in qemu_chr_be_write_impl ../../chardev/char.c:201 > > #12 0x55b0249d4414 in qemu_chr_be_write ../../chardev/char.c:213 > > #13 0x55b0249db586 in fd_chr_read ../../chardev/char-fd.c:72 > > #14 0x55b02466ba5b in qio_channel_fd_source_dispatch > > ../../io/channel-watch.c:84 > > #15 0x7f88093af0ae in g_main_context_dispatch > > (/lib64/libglib-2.0.so.0+0x550ae) > > #16 0x55b024c5ff14 in glib_pollfds_poll ../../util/main-loop.c:297 > > #17 0x55b024c600fa in os_host_main_loop_wait ../../util/main-loop.c:320 > > #18 0x55b024c603f3 in main_loop_wait ../../util/mai
Re: [PATCH] hw/sd/sdhci: reset data count in sdhci_buff_access_is_sequential()
On Wed, Nov 9, 2022 at 10:45 AM Siqi Chen wrote:
>
> Hi,
>
> >This reproducer does not crash my QEMU. Am I missing anything?
> I submitted the reproducer. Because the overflow is only one byte, it may not
> be detected by the host's heap allocator. Do you compile your qemu with
> sanitizer? This is my build configuration: "./configure
> --target-list=x86_64-softmmu --enable-sanitizers"
Right, you need to recompile QEMU with ASAN support. This is an
excerpt of the stack trace:
=
==39159==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x61522880 at pc 0x55b023db5fe1 bp 0x7fffeeef1650 sp
0x7fffeeef1648
WRITE of size 1 at 0x61522880 thread T0
#0 0x55b023db5fe0 in sdhci_write_dataport ../../hw/sd/sdhci.c:562
#1 0x55b023dc1cc7 in sdhci_write ../../hw/sd/sdhci.c:1216
#2 0x55b024521e01 in memory_region_write_accessor ../../softmmu/memory.c:492
#3 0x55b0245222ab in access_with_adjusted_size ../../softmmu/memory.c:554
#4 0x55b02452ff15 in memory_region_dispatch_write
../../softmmu/memory.c:1514
#5 0x55b024568c67 in flatview_write_continue ../../softmmu/physmem.c:2814
#6 0x55b02456908d in flatview_write ../../softmmu/physmem.c:2856
#7 0x55b024569a74 in address_space_write ../../softmmu/physmem.c:2952
#8 0x55b02457a63c in qtest_process_command ../../softmmu/qtest.c:538
#9 0x55b02457ef97 in qtest_process_inbuf ../../softmmu/qtest.c:796
#10 0x55b02457f089 in qtest_read ../../softmmu/qtest.c:808
#11 0x55b0249d4372 in qemu_chr_be_write_impl ../../chardev/char.c:201
#12 0x55b0249d4414 in qemu_chr_be_write ../../chardev/char.c:213
#13 0x55b0249db586 in fd_chr_read ../../chardev/char-fd.c:72
#14 0x55b02466ba5b in qio_channel_fd_source_dispatch
../../io/channel-watch.c:84
#15 0x7f88093af0ae in g_main_context_dispatch
(/lib64/libglib-2.0.so.0+0x550ae)
#16 0x55b024c5ff14 in glib_pollfds_poll ../../util/main-loop.c:297
#17 0x55b024c600fa in os_host_main_loop_wait ../../util/main-loop.c:320
#18 0x55b024c603f3 in main_loop_wait ../../util/main-loop.c:596
#19 0x55b023fcca21 in qemu_main_loop ../../softmmu/runstate.c:726
#20 0x55b023679735 in qemu_main ../../softmmu/main.c:36
#21 0x55b023679766 in main ../../softmmu/main.c:45
#22 0x7f8808728f5f in __libc_start_call_main (/lib64/libc.so.6+0x40f5f)
#23 0x7f880872900f in __libc_start_main_impl (/lib64/libc.so.6+0x4100f)
#24 0x55b023679644 in _start (./qemu-system-x86_64+0x20f2644)
> Thanks,
> Siqi Chen.
>
>
>
> Bin Meng 于2022年11月9日周三 17:30写道：
>>
>> Hi,
>>
>> On Mon, Nov 7, 2022 at 7:08 PM Mauro Matteo Cascella
>> wrote:
>> >
>> > On Mon, Nov 7, 2022 at 11:35 AM Mauro Matteo Cascella
>> > wrote:
>> > >
>> > > Make sure to reset data_count if it's equal to (or exceeds) block_size.
>> > > This prevents an off-by-one read / write when accessing s->fifo_buffer
>> > > in sdhci_read_dataport / sdhci_write_dataport, both called right after
>> > > sdhci_buff_access_is_sequential.
>> > >
>> > > Fixes: CVE-2022-3872
>> > > Reported-by: RivenDell
>> > > Reported-by: Siqi Chen
>> > > Reported-by: ningqiang
>> > > Signed-off-by: Mauro Matteo Cascella
>> > > ---
>> > > hw/sd/sdhci.c | 4
>> > > 1 file changed, 4 insertions(+)
>> > >
>> > > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>> > > index 306070c872..aa2fd79df2 100644
>> > > --- a/hw/sd/sdhci.c
>> > > +++ b/hw/sd/sdhci.c
>> > > @@ -978,6 +978,10 @@ static bool sdhci_can_issue_command(SDHCIState *s)
>> > > static inline bool
>> > > sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num)
>> > > {
>> > > +if (s->data_count >= (s->blksize & BLOCK_SIZE_MASK)) {
>> > > +s->data_count = 0;
>> > > +}
>> > > +
>> > > if ((s->data_count & 0x3) != byte_num) {
>> > > trace_sdhci_error("Non-sequential access to Buffer Data Port
>> > > register"
>> > >"is prohibited\n");
>> > > --
>> > > 2.38.1
>> > >
>> >
>> > Reproducer:
>> >
>> > cat << EOF | ./qemu-system-x86_64 -machine accel=qtest \
>> > -nodefaults -drive if=none,index=0,file=null-co://,format=raw,id=mydrive \
>> > -device sdhci-pci -device sd-card,drive=mydrive -nographic -qtest stdio
>> > outl 0xcf8 0x80001004
>> > outl 0xcfc 0x107
>> > outl 0xcf8 0x80001010
>> > outl 0xcfc 0x
Re: [PATCH-for-7.2 1/2] hw/sd/sdhci: Do not set Buf Wr Ena before writing block (CVE-2022-3872)
On Mon, Nov 7, 2022 at 11:12 PM Philippe Mathieu-Daudé
wrote:
>
> When sdhci_write_block_to_card() is called to transfer data from
> the FIFO to the SD bus, the data is already present in the buffer
> and we have to consume it directly.
>
> See the description of the 'Buffer Write Enable' bit from the
> 'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
> 2.14 from the SDHCI spec v2:
>
> Buffer Write Enable
>
> This status is used for non-DMA write transfers.
>
> The Host Controller can implement multiple buffers to transfer
> data efficiently. This read only flag indicates if space is
> available for write data. If this bit is 1, data can be written
> to the buffer. A change of this bit from 1 to 0 occurs when all
> the block data is written to the buffer. A change of this bit
> from 0 to 1 occurs when top of block data can be written to the
> buffer and generates the Buffer Write Ready interrupt.
>
> In our case, we do not want to overwrite the buffer, so we want
> this bit to be 0, then set it to 1 once the data is written onto
> the bus.
>
> This is probably a copy/paste error from commit d7dfca0807
> ("hw/sdhci: introduce standard SD host controller").
>
> Reproducer:
> https://lore.kernel.org/qemu-devel/caa8xkjxrms0fkr28akvnnpyatm0y0b+5fichpsrhd+mugnu...@mail.gmail.com/
>
> Fixes: CVE-2022-3872
> Reported-by: RivenDell
> Reported-by: Siqi Chen
> Reported-by: ningqiang
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/sd/sdhci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 306070c872..f230e7475f 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -954,7 +954,7 @@ static void sdhci_data_transfer(void *opaque)
> sdhci_read_block_from_card(s);
> } else {
> s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
> -SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
> + SDHC_DATA_INHIBIT;
> sdhci_write_block_to_card(s);
> }
> }
> --
> 2.38.1
>
Tested-by: Mauro Matteo Cascella
Thank you,
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] hw/sd/sdhci: reset data count in sdhci_buff_access_is_sequential()
On Mon, Nov 7, 2022 at 8:12 PM Philippe Mathieu-Daudé wrote:
>
> On 7/11/22 11:35, Mauro Matteo Cascella wrote:
> > Make sure to reset data_count if it's equal to (or exceeds) block_size.
> > This prevents an off-by-one read / write when accessing s->fifo_buffer
> > in sdhci_read_dataport / sdhci_write_dataport, both called right after
> > sdhci_buff_access_is_sequential.
> >
> > Fixes: CVE-2022-3872
> > Reported-by: RivenDell
> > Reported-by: Siqi Chen
> > Reported-by: ningqiang
> > Signed-off-by: Mauro Matteo Cascella
> > ---
> > hw/sd/sdhci.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> > index 306070c872..aa2fd79df2 100644
> > --- a/hw/sd/sdhci.c
> > +++ b/hw/sd/sdhci.c
> > @@ -978,6 +978,10 @@ static bool sdhci_can_issue_command(SDHCIState *s)
> > static inline bool
> > sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num)
> > {
> > +if (s->data_count >= (s->blksize & BLOCK_SIZE_MASK)) {
> > +s->data_count = 0;
> > +}
>
> You avoid an off-by-one but now the model doesn't work per the spec.
Note that a similar thing is done in sdhci_{read,write}_dataport. But
it's true that this is probably not enough (e.g. no update of
s->prnsts). Thank you for sending
https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg01161.html.
> Not really what the best fix IMHO.
>
> > if ((s->data_count & 0x3) != byte_num) {
> > trace_sdhci_error("Non-sequential access to Buffer Data Port
> > register"
> > "is prohibited\n");
>
> I wonder why sdhci_data_transfer() indiscriminately sets
> SDHC_SPACE_AVAILABLE in the write path (at least without
> clearing the FIFO first).
>
> The fix could be:
>
> -- >8 --
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> @@ -955,5 +955,5 @@ static void sdhci_data_transfer(void *opaque)
> } else {
> s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
> -SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
> + SDHC_DATA_INHIBIT;
> sdhci_write_block_to_card(s);
> }
> ---
>
> Bin, what do you think?
>
> Regards,
>
> Phil.
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] hw/sd/sdhci: reset data count in sdhci_buff_access_is_sequential()
On Mon, Nov 7, 2022 at 11:35 AM Mauro Matteo Cascella
wrote:
>
> Make sure to reset data_count if it's equal to (or exceeds) block_size.
> This prevents an off-by-one read / write when accessing s->fifo_buffer
> in sdhci_read_dataport / sdhci_write_dataport, both called right after
> sdhci_buff_access_is_sequential.
>
> Fixes: CVE-2022-3872
> Reported-by: RivenDell
> Reported-by: Siqi Chen
> Reported-by: ningqiang
> Signed-off-by: Mauro Matteo Cascella
> ---
> hw/sd/sdhci.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 306070c872..aa2fd79df2 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -978,6 +978,10 @@ static bool sdhci_can_issue_command(SDHCIState *s)
> static inline bool
> sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num)
> {
> +if (s->data_count >= (s->blksize & BLOCK_SIZE_MASK)) {
> +s->data_count = 0;
> +}
> +
> if ((s->data_count & 0x3) != byte_num) {
> trace_sdhci_error("Non-sequential access to Buffer Data Port
> register"
>"is prohibited\n");
> --
> 2.38.1
>
Reproducer:
cat << EOF | ./qemu-system-x86_64 -machine accel=qtest \
-nodefaults -drive if=none,index=0,file=null-co://,format=raw,id=mydrive \
-device sdhci-pci -device sd-card,drive=mydrive -nographic -qtest stdio
outl 0xcf8 0x80001004
outl 0xcfc 0x107
outl 0xcf8 0x80001010
outl 0xcfc 0xfebf1000
writel 0xfebf102c 0x7
writel 0xfebf1004 0x10200
writel 0xfebf100c 0x20
writel 0xfebf1028 0x1
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbeef
writel 0xfebf1020 0xdeadbe
[PATCH] hw/sd/sdhci: reset data count in sdhci_buff_access_is_sequential()
Make sure to reset data_count if it's equal to (or exceeds) block_size.
This prevents an off-by-one read / write when accessing s->fifo_buffer
in sdhci_read_dataport / sdhci_write_dataport, both called right after
sdhci_buff_access_is_sequential.
Fixes: CVE-2022-3872
Reported-by: RivenDell
Reported-by: Siqi Chen
Reported-by: ningqiang
Signed-off-by: Mauro Matteo Cascella
---
hw/sd/sdhci.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 306070c872..aa2fd79df2 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -978,6 +978,10 @@ static bool sdhci_can_issue_command(SDHCIState *s)
static inline bool
sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num)
{
+if (s->data_count >= (s->blksize & BLOCK_SIZE_MASK)) {
+s->data_count = 0;
+}
+
if ((s->data_count & 0x3) != byte_num) {
trace_sdhci_error("Non-sequential access to Buffer Data Port register"
"is prohibited\n");
--
2.38.1
[PATCH] hw/sd/sdhci: further prohibit DMA accesses to devices
Commit 799f7f01 left some DMA calls unprotected. Let's patch them.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1282
Reported-by: Siqi Chen
Signed-off-by: Mauro Matteo Cascella
---
hw/sd/sdhci.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 0e5e988927..c6ee24cde1 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -585,6 +585,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
const uint16_t block_size = s->blksize & BLOCK_SIZE_MASK;
uint32_t boundary_chk = 1 << (((s->blksize & ~BLOCK_SIZE_MASK) >> 12) +
12);
uint32_t boundary_count = boundary_chk - (s->sdmasysad % boundary_chk);
+const MemTxAttrs attrs = { .memory = true };
if (!(s->trnmod & SDHC_TRNS_BLK_CNT_EN) || !s->blkcnt) {
qemu_log_mask(LOG_UNIMP, "infinite transfer is not supported\n");
@@ -617,7 +618,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
}
}
dma_memory_write(s->dma_as, s->sdmasysad, >fifo_buffer[begin],
- s->data_count - begin, MEMTXATTRS_UNSPECIFIED);
+ s->data_count - begin, attrs);
s->sdmasysad += s->data_count - begin;
if (s->data_count == block_size) {
s->data_count = 0;
@@ -638,7 +639,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
boundary_count -= block_size - begin;
}
dma_memory_read(s->dma_as, s->sdmasysad, >fifo_buffer[begin],
-s->data_count - begin, MEMTXATTRS_UNSPECIFIED);
+s->data_count - begin, attrs);
s->sdmasysad += s->data_count - begin;
if (s->data_count == block_size) {
sdbus_write_data(>sdbus, s->fifo_buffer, block_size);
@@ -667,14 +668,15 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState
*s)
static void sdhci_sdma_transfer_single_block(SDHCIState *s)
{
uint32_t datacnt = s->blksize & BLOCK_SIZE_MASK;
+const MemTxAttrs attrs = { .memory = true };
if (s->trnmod & SDHC_TRNS_READ) {
sdbus_read_data(>sdbus, s->fifo_buffer, datacnt);
dma_memory_write(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt,
- MEMTXATTRS_UNSPECIFIED);
+ attrs);
} else {
dma_memory_read(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt,
-MEMTXATTRS_UNSPECIFIED);
+attrs);
sdbus_write_data(>sdbus, s->fifo_buffer, datacnt);
}
s->blkcnt--;
@@ -693,11 +695,13 @@ static void get_adma_description(SDHCIState *s, ADMADescr
*dscr)
{
uint32_t adma1 = 0;
uint64_t adma2 = 0;
+const MemTxAttrs attrs = { .memory = true };
hwaddr entry_addr = (hwaddr)s->admasysaddr;
+
switch (SDHC_DMA_TYPE(s->hostctl1)) {
case SDHC_CTRL_ADMA2_32:
dma_memory_read(s->dma_as, entry_addr, , sizeof(adma2),
-MEMTXATTRS_UNSPECIFIED);
+attrs);
adma2 = le64_to_cpu(adma2);
/* The spec does not specify endianness of descriptor table.
* We currently assume that it is LE.
@@ -709,7 +713,7 @@ static void get_adma_description(SDHCIState *s, ADMADescr
*dscr)
break;
case SDHC_CTRL_ADMA1_32:
dma_memory_read(s->dma_as, entry_addr, , sizeof(adma1),
-MEMTXATTRS_UNSPECIFIED);
+attrs);
adma1 = le32_to_cpu(adma1);
dscr->addr = (hwaddr)(adma1 & 0xF000);
dscr->attr = (uint8_t)extract32(adma1, 0, 7);
@@ -722,12 +726,12 @@ static void get_adma_description(SDHCIState *s, ADMADescr
*dscr)
break;
case SDHC_CTRL_ADMA2_64:
dma_memory_read(s->dma_as, entry_addr, >attr, 1,
-MEMTXATTRS_UNSPECIFIED);
+attrs);
dma_memory_read(s->dma_as, entry_addr + 2, >length, 2,
-MEMTXATTRS_UNSPECIFIED);
+attrs);
dscr->length = le16_to_cpu(dscr->length);
dma_memory_read(s->dma_as, entry_addr + 4, >addr, 8,
-MEMTXATTRS_UNSPECIFIED);
+attrs);
dscr->addr = le64_to_cpu(dscr->addr);
dscr->attr &= (uint8_t) ~0xC0;
dscr->incr = 12;
--
2.37.3
Re: [PATCH] ui/vnc-clipboard: fix integer underflow in vnc_client_cut_text_ext
On Sun, Sep 25, 2022 at 10:45 PM Mauro Matteo Cascella
wrote:
>
> Extended ClientCutText messages start with a 4-byte header. If len < 4,
> an integer underflow occurs in vnc_client_cut_text_ext. The result is
> used to decompress data in a while loop in inflate_buffer, leading to
> CPU consumption and denial of service. Prevent this by checking dlen in
> protocol_client_msg.
>
> Fixes: CVE-2022-3165
> Fixes: 0bf41cab93e5 ("ui/vnc: clipboard support")
> Reported-by: TangPeng
> Signed-off-by: Mauro Matteo Cascella
> ---
> Extended Clipboard Pseudo-Encoding:
> https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#extended-clipboard-pseudo-encoding
>
> ui/vnc.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 6a05d06147..acb3629cd8 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2442,8 +2442,8 @@ static int protocol_client_msg(VncState *vs, uint8_t
> *data, size_t len)
> if (len == 1) {
> return 8;
> }
> +uint32_t dlen = abs(read_s32(data, 4));
> if (len == 8) {
> -uint32_t dlen = abs(read_s32(data, 4));
> if (dlen > (1 << 20)) {
> error_report("vnc: client_cut_text msg payload has %u bytes"
> " which exceeds our limit of 1MB.", dlen);
> @@ -2456,8 +2456,13 @@ static int protocol_client_msg(VncState *vs, uint8_t
> *data, size_t len)
> }
>
> if (read_s32(data, 4) < 0) {
> -vnc_client_cut_text_ext(vs, abs(read_s32(data, 4)),
> -read_u32(data, 8), data + 12);
> +if (dlen < 4) {
> +error_report("vnc: malformed payload (header less than 4
> bytes)"
> + " in extended clipboard pseudo-encoding.");
> +vnc_client_error(vs);
> +break;
> +}
> +vnc_client_cut_text_ext(vs, dlen, read_u32(data, 8), data + 12);
> break;
> }
> vnc_client_cut_text(vs, read_u32(data, 4), data + 8);
> --
> 2.37.3
>
Any updates here?
Thanks,
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH] ui/vnc-clipboard: fix integer underflow in vnc_client_cut_text_ext
Extended ClientCutText messages start with a 4-byte header. If len < 4,
an integer underflow occurs in vnc_client_cut_text_ext. The result is
used to decompress data in a while loop in inflate_buffer, leading to
CPU consumption and denial of service. Prevent this by checking dlen in
protocol_client_msg.
Fixes: CVE-2022-3165
Fixes: 0bf41cab93e5 ("ui/vnc: clipboard support")
Reported-by: TangPeng
Signed-off-by: Mauro Matteo Cascella
---
Extended Clipboard Pseudo-Encoding:
https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#extended-clipboard-pseudo-encoding
ui/vnc.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 6a05d06147..acb3629cd8 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2442,8 +2442,8 @@ static int protocol_client_msg(VncState *vs, uint8_t
*data, size_t len)
if (len == 1) {
return 8;
}
+uint32_t dlen = abs(read_s32(data, 4));
if (len == 8) {
-uint32_t dlen = abs(read_s32(data, 4));
if (dlen > (1 << 20)) {
error_report("vnc: client_cut_text msg payload has %u bytes"
" which exceeds our limit of 1MB.", dlen);
@@ -2456,8 +2456,13 @@ static int protocol_client_msg(VncState *vs, uint8_t
*data, size_t len)
}
if (read_s32(data, 4) < 0) {
-vnc_client_cut_text_ext(vs, abs(read_s32(data, 4)),
-read_u32(data, 8), data + 12);
+if (dlen < 4) {
+error_report("vnc: malformed payload (header less than 4
bytes)"
+ " in extended clipboard pseudo-encoding.");
+vnc_client_error(vs);
+break;
+}
+vnc_client_cut_text_ext(vs, dlen, read_u32(data, 8), data + 12);
break;
}
vnc_client_cut_text(vs, read_u32(data, 4), data + 8);
--
2.37.3
[PATCH] qtest/fuzz-lsi53c895a-test: set guest RAM to 2G
test_lsi_do_msgout_cancel_req does not run on machines with small size
memory. Reduce guest memory from 4G to 2G to alleviate the problem.
Reported-by: Bin Meng
Signed-off-by: Mauro Matteo Cascella
---
tests/qtest/fuzz-lsi53c895a-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/fuzz-lsi53c895a-test.c
b/tests/qtest/fuzz-lsi53c895a-test.c
index b23d3ecf45..434c16bf42 100644
--- a/tests/qtest/fuzz-lsi53c895a-test.c
+++ b/tests/qtest/fuzz-lsi53c895a-test.c
@@ -21,7 +21,7 @@ static void test_lsi_do_msgout_cancel_req(void)
return;
}
-s = qtest_init("-M q35 -m 4G -display none -nodefaults "
+s = qtest_init("-M q35 -m 2G -display none -nodefaults "
"-device lsi53c895a,id=scsi "
"-device scsi-hd,drive=disk0 "
"-drive file=null-co://,id=disk0,if=none,format=raw");
--
2.37.2
Re: [PATCH] scsi/lsi53c895a: really fix use-after-free in lsi_do_msgout (CVE-2022-0216)
Hi Bin,
On Fri, Sep 2, 2022 at 3:56 AM Bin Meng wrote:
>
> Hi,
>
> On Wed, Jul 13, 2022 at 8:45 PM Paolo Bonzini wrote:
> >
> > From: Mauro Matteo Cascella
> >
> > Set current_req to NULL, not current_req->req, to prevent reusing a free'd
> > buffer in case of repeated SCSI cancel requests. Also apply the fix to
> > CLEAR QUEUE and BUS DEVICE RESET messages as well, since they also cancel
> > the request.
> >
> > Thanks to Alexander Bulekov for providing a reproducer.
> >
> > Fixes: CVE-2022-0216
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
> > Signed-off-by: Mauro Matteo Cascella
> > Tested-by: Alexander Bulekov
> > Message-Id: <20220711123316.421279-1-mcasc...@redhat.com>
> > Signed-off-by: Paolo Bonzini
> > ---
> > Adjust the patch from v1 to v2 since the changes crossed
> > with the pull request.
> >
> > hw/scsi/lsi53c895a.c | 3 +-
> > tests/qtest/fuzz-lsi53c895a-test.c | 71 ++
> > 2 files changed, 73 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> > index 99ea42d49b..ad5f5e5f39 100644
> > --- a/hw/scsi/lsi53c895a.c
> > +++ b/hw/scsi/lsi53c895a.c
> > @@ -1030,7 +1030,7 @@ static void lsi_do_msgout(LSIState *s)
> > trace_lsi_do_msgout_abort(current_tag);
> > if (current_req && current_req->req) {
> > scsi_req_cancel(current_req->req);
> > -current_req->req = NULL;
> > +current_req = NULL;
> > }
> > lsi_disconnect(s);
> > break;
> > @@ -1056,6 +1056,7 @@ static void lsi_do_msgout(LSIState *s)
> > /* clear the current I/O process */
> > if (s->current) {
> > scsi_req_cancel(s->current->req);
> > +current_req = NULL;
> > }
> >
> > /* As the current implemented devices scsi_disk and
> > scsi_generic
> > diff --git a/tests/qtest/fuzz-lsi53c895a-test.c
> > b/tests/qtest/fuzz-lsi53c895a-test.c
> > index 2e8e67859e..6872c70d3a 100644
> > --- a/tests/qtest/fuzz-lsi53c895a-test.c
> > +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> > @@ -8,6 +8,74 @@
> > #include "qemu/osdep.h"
> > #include "libqtest.h"
> >
> > +/*
> > + * This used to trigger a UAF in lsi_do_msgout()
> > + * https://gitlab.com/qemu-project/qemu/-/issues/972
> > + */
> > +static void test_lsi_do_msgout_cancel_req(void)
> > +{
> > +QTestState *s;
> > +
> > +s = qtest_init("-M q35 -m 4G -display none -nodefaults "
>
> This test does not run on machines with small size memory. Is 4G a
> must have for this test?
4G comes from [1], I don't think it's a must have. Would 2G be okay?
For some reason ASAN does not trigger the UAF when I run the test
locally with less than 2G (1.7G to be precise). I didn't really
investigate why that is the case.
[1] https://gitlab.com/qemu-project/qemu/-/issues/972#note_1019851430.
>
>
> > + "-device lsi53c895a,id=scsi "
> > + "-device scsi-hd,drive=disk0 "
> > + "-drive file=null-co://,id=disk0,if=none,format=raw");
> > +
> > +qtest_outl(s, 0xcf8, 0x8810);
> > +qtest_outl(s, 0xcf8, 0xc000);
> > +qtest_outl(s, 0xcf8, 0x8810);
> > +qtest_outw(s, 0xcfc, 0x7);
> > +qtest_outl(s, 0xcf8, 0x8810);
> > +qtest_outl(s, 0xcfc, 0xc000);
> > +qtest_outl(s, 0xcf8, 0x8804);
> > +qtest_outw(s, 0xcfc, 0x05);
> > +qtest_writeb(s, 0x69736c10, 0x08);
> > +qtest_writeb(s, 0x69736c13, 0x58);
> > +qtest_writeb(s, 0x69736c1a, 0x01);
> > +qtest_writeb(s, 0x69736c1b, 0x06);
> > +qtest_writeb(s, 0x69736c22, 0x01);
> > +qtest_writeb(s, 0x69736c23, 0x07);
> > +qtest_writeb(s, 0x69736c2b, 0x02);
> > +qtest_writeb(s, 0x69736c48, 0x08);
> > +qtest_writeb(s, 0x69736c4b, 0x58);
> > +qtest_writeb(s, 0x69736c52, 0x04);
> > +qtest_writeb(s, 0x69736c53, 0x06);
> > +qtest_writeb(s, 0x69736c5b, 0x02);
> > +qtest_outl(s, 0xc02d, 0x697300);
> > +qtest_writeb(s, 0x5a554662, 0x01);
> > +qtest_writeb(s, 0x5a554663, 0x07);
> > +qtest_writeb(s, 0x5a55466a, 0x10);
> > +qtest_writeb(s, 0x5a55466b, 0x22);
> > +qtest_writeb(s, 0x5a55466c, 0x5a);
> > +qtest_writeb(s, 0x5a5546
Re: [PATCH v2 for-7.1] hw/usb/hcd-xhci: Fix unbounded loop in xhci_ring_chain_length() (CVE-2020-14394)
On Thu, Aug 4, 2022 at 3:13 PM Thomas Huth wrote:
>
> The loop condition in xhci_ring_chain_length() is under control of
> the guest, and additionally the code does not check for failed DMA
> transfers (e.g. if reaching the end of the RAM), so the loop there
> could run for a very long time or even forever. Fix it by checking
> the return value of dma_memory_read() and by introducing a maximum
> loop length.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/646
> Signed-off-by: Thomas Huth
> ---
> v2:
> - Reworded subject and commit description
> - Focus on xhci_ring_chain_length() since that's the only function
>where an endless loop can currently occur. The other spots that
>ignore the return value of dma_memory_read() should be fixed as
>well later, but that's rather something for QEMU 7.2.
> - Added an real limit for the loop, so that it also ends after a
>while in case there are no DMA errors
> - Use "return -1" instead of "return -length" since the latter
>is somewhat weird (could be sometimes 0, sometimes negative)
>
> hw/usb/hcd-xhci.c | 23 +++
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 5a1ddf8c3e..b5669bc234 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -21,6 +21,7 @@
>
> #include "qemu/osdep.h"
> #include "qemu/timer.h"
> +#include "qemu/log.h"
> #include "qemu/module.h"
> #include "qemu/queue.h"
> #include "migration/vmstate.h"
> @@ -729,10 +730,14 @@ static int xhci_ring_chain_length(XHCIState *xhci,
> const XHCIRing *ring)
> bool control_td_set = 0;
> uint32_t link_cnt = 0;
>
> -while (1) {
> +do {
> TRBType type;
> -dma_memory_read(xhci->as, dequeue, , TRB_SIZE,
> -MEMTXATTRS_UNSPECIFIED);
> +if (dma_memory_read(xhci->as, dequeue, , TRB_SIZE,
> +MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: DMA memory access failed!\n",
> + __func__);
> +return -1;
> +}
> le64_to_cpus();
> le32_to_cpus();
> le32_to_cpus();
> @@ -766,7 +771,17 @@ static int xhci_ring_chain_length(XHCIState *xhci, const
> XHCIRing *ring)
> if (!control_td_set && !(trb.control & TRB_TR_CH)) {
> return length;
> }
> -}
> +
> +/*
> + * According to the xHCI spec, Transfer Ring segments should have
> + * a maximum size of 64 kB (see chapter "6 Data Structures")
> + */
> +} while (length < TRB_LINK_LIMIT * 65536 / TRB_SIZE);
> +
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: exceeded maximum tranfer ring
> size!\n",
> + __func__);
> +
> +return -1;
> }
>
> static void xhci_er_reset(XHCIState *xhci, int v)
> --
> 2.31.1
>
Reviewed-by: Mauro Matteo Cascella
Thanks,
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] hw/usb/hcd-xhci: Fix endless loop in case the DMA access fails (CVE-2020-14394)
On Tue, Aug 2, 2022 at 3:48 PM Thomas Huth wrote:
>
> The XHCI code could enter an endless loop in case the guest points
> QEMU to fetch TRBs from invalid memory areas. Fix it by properly
> checking the return value of dma_memory_read().
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/646
> Signed-off-by: Thomas Huth
> ---
> hw/usb/hcd-xhci.c | 17 +
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 296cc6c8e6..63d428a444 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -21,6 +21,7 @@
>
> #include "qemu/osdep.h"
> #include "qemu/timer.h"
> +#include "qemu/log.h"
> #include "qemu/module.h"
> #include "qemu/queue.h"
> #include "migration/vmstate.h"
> @@ -679,8 +680,12 @@ static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing
> *ring, XHCITRB *trb,
>
> while (1) {
> TRBType type;
> -dma_memory_read(xhci->as, ring->dequeue, trb, TRB_SIZE,
> -MEMTXATTRS_UNSPECIFIED);
> +if (dma_memory_read(xhci->as, ring->dequeue, trb, TRB_SIZE,
> +MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: DMA memory access failed!\n",
> + __func__);
> +return 0;
> +}
> trb->addr = ring->dequeue;
> trb->ccs = ring->ccs;
> le64_to_cpus(>parameter);
> @@ -727,8 +732,12 @@ static int xhci_ring_chain_length(XHCIState *xhci, const
> XHCIRing *ring)
>
> while (1) {
> TRBType type;
> -dma_memory_read(xhci->as, dequeue, , TRB_SIZE,
> -MEMTXATTRS_UNSPECIFIED);
> +if (dma_memory_read(xhci->as, dequeue, , TRB_SIZE,
> +MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: DMA memory access failed!\n",
> + __func__);
> + return -length;
Not strictly related to this issue, but what's the point of returning
-length instead of e.g. -1? Apart from that, LGTM. Thank you.
> +}
> le64_to_cpus();
> le32_to_cpus();
> le32_to_cpus();
> --
> 2.31.1
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH v2] scsi/lsi53c895a: fix use-after-free in lsi_do_msgout (CVE-2022-0216)
Set current_req to NULL to prevent reusing a free'd buffer in case of repeated
SCSI cancel requests. Thanks to Thomas Huth for suggesting the first version of
the patch and Alexander Bulekov for providing a reproducer.
Fixes: CVE-2022-0216
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
Signed-off-by: Mauro Matteo Cascella
---
v2:
- handle CLEAR QUEUE and BUS DEVICE RESET messages
- new qtest: test_lsi_do_msgout_cancel_req
hw/scsi/lsi53c895a.c | 2 +
tests/qtest/fuzz-lsi53c895a-test.c | 71 ++
2 files changed, 73 insertions(+)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index c8773f73f7..6934040c59 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1030,6 +1030,7 @@ static void lsi_do_msgout(LSIState *s)
trace_lsi_do_msgout_abort(current_tag);
if (current_req) {
scsi_req_cancel(current_req->req);
+current_req = NULL;
}
lsi_disconnect(s);
break;
@@ -1055,6 +1056,7 @@ static void lsi_do_msgout(LSIState *s)
/* clear the current I/O process */
if (s->current) {
scsi_req_cancel(s->current->req);
+current_req = NULL;
}
/* As the current implemented devices scsi_disk and scsi_generic
diff --git a/tests/qtest/fuzz-lsi53c895a-test.c
b/tests/qtest/fuzz-lsi53c895a-test.c
index 2e8e67859e..6872c70d3a 100644
--- a/tests/qtest/fuzz-lsi53c895a-test.c
+++ b/tests/qtest/fuzz-lsi53c895a-test.c
@@ -8,6 +8,74 @@
#include "qemu/osdep.h"
#include "libqtest.h"
+/*
+ * This used to trigger a UAF in lsi_do_msgout()
+ * https://gitlab.com/qemu-project/qemu/-/issues/972
+ */
+static void test_lsi_do_msgout_cancel_req(void)
+{
+QTestState *s;
+
+s = qtest_init("-M q35 -m 4G -display none -nodefaults "
+ "-device lsi53c895a,id=scsi "
+ "-device scsi-hd,drive=disk0 "
+ "-drive file=null-co://,id=disk0,if=none,format=raw");
+
+qtest_outl(s, 0xcf8, 0x8810);
+qtest_outl(s, 0xcf8, 0xc000);
+qtest_outl(s, 0xcf8, 0x8810);
+qtest_outw(s, 0xcfc, 0x7);
+qtest_outl(s, 0xcf8, 0x8810);
+qtest_outl(s, 0xcfc, 0xc000);
+qtest_outl(s, 0xcf8, 0x8804);
+qtest_outw(s, 0xcfc, 0x05);
+qtest_writeb(s, 0x69736c10, 0x08);
+qtest_writeb(s, 0x69736c13, 0x58);
+qtest_writeb(s, 0x69736c1a, 0x01);
+qtest_writeb(s, 0x69736c1b, 0x06);
+qtest_writeb(s, 0x69736c22, 0x01);
+qtest_writeb(s, 0x69736c23, 0x07);
+qtest_writeb(s, 0x69736c2b, 0x02);
+qtest_writeb(s, 0x69736c48, 0x08);
+qtest_writeb(s, 0x69736c4b, 0x58);
+qtest_writeb(s, 0x69736c52, 0x04);
+qtest_writeb(s, 0x69736c53, 0x06);
+qtest_writeb(s, 0x69736c5b, 0x02);
+qtest_outl(s, 0xc02d, 0x697300);
+qtest_writeb(s, 0x5a554662, 0x01);
+qtest_writeb(s, 0x5a554663, 0x07);
+qtest_writeb(s, 0x5a55466a, 0x10);
+qtest_writeb(s, 0x5a55466b, 0x22);
+qtest_writeb(s, 0x5a55466c, 0x5a);
+qtest_writeb(s, 0x5a55466d, 0x5a);
+qtest_writeb(s, 0x5a55466e, 0x34);
+qtest_writeb(s, 0x5a55466f, 0x5a);
+qtest_writeb(s, 0x5a345a5a, 0x77);
+qtest_writeb(s, 0x5a345a5b, 0x55);
+qtest_writeb(s, 0x5a345a5c, 0x51);
+qtest_writeb(s, 0x5a345a5d, 0x27);
+qtest_writeb(s, 0x27515577, 0x41);
+qtest_outl(s, 0xc02d, 0x5a5500);
+qtest_writeb(s, 0x364001d0, 0x08);
+qtest_writeb(s, 0x364001d3, 0x58);
+qtest_writeb(s, 0x364001da, 0x01);
+qtest_writeb(s, 0x364001db, 0x26);
+qtest_writeb(s, 0x364001dc, 0x0d);
+qtest_writeb(s, 0x364001dd, 0xae);
+qtest_writeb(s, 0x364001de, 0x41);
+qtest_writeb(s, 0x364001df, 0x5a);
+qtest_writeb(s, 0x5a41ae0d, 0xf8);
+qtest_writeb(s, 0x5a41ae0e, 0x36);
+qtest_writeb(s, 0x5a41ae0f, 0xd7);
+qtest_writeb(s, 0x5a41ae10, 0x36);
+qtest_writeb(s, 0x36d736f8, 0x0c);
+qtest_writeb(s, 0x36d736f9, 0x80);
+qtest_writeb(s, 0x36d736fa, 0x0d);
+qtest_outl(s, 0xc02d, 0x364000);
+
+qtest_quit(s);
+}
+
/*
* This used to trigger the assert in lsi_do_dma()
* https://bugs.launchpad.net/qemu/+bug/697510
@@ -44,5 +112,8 @@ int main(int argc, char **argv)
qtest_add_func("fuzz/lsi53c895a/lsi_do_dma_empty_queue",
test_lsi_do_dma_empty_queue);
+qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
+ test_lsi_do_msgout_cancel_req);
+
return g_test_run();
}
--
2.35.3
Re: [PATCH] scsi/lsi53c895a: fix use-after-free in lsi_do_msgout (CVE-2022-0216)
Hi Alexander,
Thanks for the reproducer! It looks like ABORT, CLEAR QUEUE and BUS
DEVICE RESET messages can all cancel the current request, so yes I
guess a similar change is needed there, too. Will try to send a v2
soon.
Best regards.
On Sat, Jul 9, 2022 at 2:22 AM Alexander Bulekov wrote:
>
> On 220705 2205, Mauro Matteo Cascella wrote:
> > Set current_req->req to NULL to prevent reusing a free'd buffer in case of
> > repeated SCSI cancel requests. Thanks to Thomas Huth for suggesting the
> > patch.
> >
> > Fixes: CVE-2022-0216
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
> > Signed-off-by: Mauro Matteo Cascella
> > ---
> > hw/scsi/lsi53c895a.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> > index c8773f73f7..99ea42d49b 100644
> > --- a/hw/scsi/lsi53c895a.c
> > +++ b/hw/scsi/lsi53c895a.c
> > @@ -1028,8 +1028,9 @@ static void lsi_do_msgout(LSIState *s)
> > case 0x0d:
> > /* The ABORT TAG message clears the current I/O process only.
> > */
> > trace_lsi_do_msgout_abort(current_tag);
> > -if (current_req) {
> > +if (current_req && current_req->req) {
> > scsi_req_cancel(current_req->req);
> > +current_req->req = NULL;
> > }
> > lsi_disconnect(s);
> > break;
> > --
> > 2.35.3
> >
> >
>
> Hi Mauro,
> https://gitlab.com/qemu-project/qemu/-/issues/972#note_1019851430
> This reproducer crashes, with this patch applied. Maybe it is some
> different bug though - I'm not sure.
>
> With -trace lsi*
>
> lsi_reg_write Write reg DSP1 0x2d = 0x00
> lsi_reg_write Write reg DSP2 0x2e = 0x40
> lsi_reg_write Write reg DSP3 0x2f = 0x36
> lsi_execute_script SCRIPTS dsp=0x364001d0 opcode 0x5808 arg 0x0
> lsi_execute_script_io_set Set ATN
> lsi_execute_script SCRIPTS dsp=0x364001d8 opcode 0x2601 arg 0x5a41ae0d
> lsi_do_msgout MSG out len=65536
> lsi_do_msgout_busdevicereset MSG: BUS DEVICE RESET tag=0x0
> lsi_do_msgout_select Select LUN 0
> lsi_do_msgout_abort MSG: ABORT TAG tag=0x0
>
> In busdevicereset, there are also scsi_req_cancel calls. Do they need
> similar changes?
>
> -Alex
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH] scsi/lsi53c895a: fix use-after-free in lsi_do_msgout (CVE-2022-0216)
Set current_req->req to NULL to prevent reusing a free'd buffer in case of
repeated SCSI cancel requests. Thanks to Thomas Huth for suggesting the patch.
Fixes: CVE-2022-0216
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
Signed-off-by: Mauro Matteo Cascella
---
hw/scsi/lsi53c895a.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index c8773f73f7..99ea42d49b 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1028,8 +1028,9 @@ static void lsi_do_msgout(LSIState *s)
case 0x0d:
/* The ABORT TAG message clears the current I/O process only. */
trace_lsi_do_msgout_abort(current_tag);
-if (current_req) {
+if (current_req && current_req->req) {
scsi_req_cancel(current_req->req);
+current_req->req = NULL;
}
lsi_disconnect(s);
break;
--
2.35.3
[PATCH] usb/hcd-xhci: check slotid in xhci_wakeup_endpoint()
This prevents an OOB read (followed by an assertion failure in
xhci_kick_ep) when slotid > xhci->numslots.
Reported-by: Soul Chen
Signed-off-by: Mauro Matteo Cascella
---
hw/usb/hcd-xhci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 0cd0a5e540..cecf8a0d96 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3269,7 +3269,8 @@ static void xhci_wakeup_endpoint(USBBus *bus, USBEndpoint
*ep,
DPRINTF("%s\n", __func__);
slotid = ep->dev->addr;
-if (slotid == 0 || !xhci->slots[slotid-1].enabled) {
+if (slotid == 0 || slotid > xhci->numslots ||
+!xhci->slots[slotid-1].enabled) {
DPRINTF("%s: oops, no slot for dev %d\n", __func__, ep->dev->addr);
return;
}
--
2.35.3
[PATCH] hw/net/rocker: avoid NULL pointer dereference in of_dpa_cmd_add_l2_flood
rocker_tlv_parse_nested could return early because of no group ids in
the group_tlvs. In such case tlvs is NULL; tlvs[i + 1] in the next
for-loop will deref the NULL pointer.
Signed-off-by: Mauro Matteo Cascella
Reported-by:
---
hw/net/rocker/rocker_of_dpa.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
index b3b8c5bb6d..1611b79227 100644
--- a/hw/net/rocker/rocker_of_dpa.c
+++ b/hw/net/rocker/rocker_of_dpa.c
@@ -2039,6 +2039,11 @@ static int of_dpa_cmd_add_l2_flood(OfDpa *of_dpa,
OfDpaGroup *group,
rocker_tlv_parse_nested(tlvs, group->l2_flood.group_count,
group_tlvs[ROCKER_TLV_OF_DPA_GROUP_IDS]);
+if (!tlvs) {
+err = -ROCKER_EINVAL;
+goto err_out;
+}
+
for (i = 0; i < group->l2_flood.group_count; i++) {
group->l2_flood.group_ids[i] = rocker_tlv_get_le32(tlvs[i + 1]);
}
--
2.35.3
Re: [PATCH v3] ui/cursor: fix integer overflow in cursor_alloc (CVE-2021-4206)
On Thu, Apr 7, 2022 at 11:17 AM Marc-André Lureau
wrote:
>
>
>
> On Thu, Apr 7, 2022 at 12:23 PM Mauro Matteo Cascella
> wrote:
>>
>> Prevent potential integer overflow by limiting 'width' and 'height' to
>> 512x512. Also change 'datasize' type to size_t. Refer to security
>> advisory https://starlabs.sg/advisories/22-4206/ for more information.
>>
>> Fixes: CVE-2021-4206
>
>
> (the Starlabs advisory has 2022, I guess it's wrong then)
Yep, that is wrong. I asked them to update the page.
Thanks.
>> Signed-off-by: Mauro Matteo Cascella
>
>
> Reviewed-by: Marc-André Lureau
>
>
>>
>> ---
>> v3:
>> - fix CVE id (CVE-2021-4206 instead of CVE-2022-4206)
>>
>> hw/display/qxl-render.c | 7 +++
>> hw/display/vmware_vga.c | 2 ++
>> ui/cursor.c | 8 +++-
>> 3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
>> index d28849b121..dc3c4edd05 100644
>> --- a/hw/display/qxl-render.c
>> +++ b/hw/display/qxl-render.c
>> @@ -247,6 +247,13 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl,
>> QXLCursor *cursor,
>> size_t size;
>>
>> c = cursor_alloc(cursor->header.width, cursor->header.height);
>> +
>> +if (!c) {
>> +qxl_set_guest_bug(qxl, "%s: cursor %ux%u alloc error", __func__,
>> +cursor->header.width, cursor->header.height);
>> +goto fail;
>> +}
>> +
>> c->hot_x = cursor->header.hot_spot_x;
>> c->hot_y = cursor->header.hot_spot_y;
>> switch (cursor->header.type) {
>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>> index 98c83474ad..45d06cbe25 100644
>> --- a/hw/display/vmware_vga.c
>> +++ b/hw/display/vmware_vga.c
>> @@ -515,6 +515,8 @@ static inline void vmsvga_cursor_define(struct
>> vmsvga_state_s *s,
>> int i, pixels;
>>
>> qc = cursor_alloc(c->width, c->height);
>> +assert(qc != NULL);
>> +
>> qc->hot_x = c->hot_x;
>> qc->hot_y = c->hot_y;
>> switch (c->bpp) {
>> diff --git a/ui/cursor.c b/ui/cursor.c
>> index 1d62ddd4d0..835f0802f9 100644
>> --- a/ui/cursor.c
>> +++ b/ui/cursor.c
>> @@ -46,6 +46,8 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
>>
>> /* parse pixel data */
>> c = cursor_alloc(width, height);
>> +assert(c != NULL);
>> +
>> for (pixel = 0, y = 0; y < height; y++, line++) {
>> for (x = 0; x < height; x++, pixel++) {
>> idx = xpm[line][x];
>> @@ -91,7 +93,11 @@ QEMUCursor *cursor_builtin_left_ptr(void)
>> QEMUCursor *cursor_alloc(int width, int height)
>> {
>> QEMUCursor *c;
>> -int datasize = width * height * sizeof(uint32_t);
>> +size_t datasize = width * height * sizeof(uint32_t);
>> +
>> +if (width > 512 || height > 512) {
>> +return NULL;
>> +}
>>
>> c = g_malloc0(sizeof(QEMUCursor) + datasize);
>> c->width = width;
>> --
>> 2.35.1
>>
>>
>
>
> --
> Marc-André Lureau
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH v3] ui/cursor: fix integer overflow in cursor_alloc (CVE-2021-4206)
Prevent potential integer overflow by limiting 'width' and 'height' to
512x512. Also change 'datasize' type to size_t. Refer to security
advisory https://starlabs.sg/advisories/22-4206/ for more information.
Fixes: CVE-2021-4206
Signed-off-by: Mauro Matteo Cascella
---
v3:
- fix CVE id (CVE-2021-4206 instead of CVE-2022-4206)
hw/display/qxl-render.c | 7 +++
hw/display/vmware_vga.c | 2 ++
ui/cursor.c | 8 +++-
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index d28849b121..dc3c4edd05 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -247,6 +247,13 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor
*cursor,
size_t size;
c = cursor_alloc(cursor->header.width, cursor->header.height);
+
+if (!c) {
+qxl_set_guest_bug(qxl, "%s: cursor %ux%u alloc error", __func__,
+cursor->header.width, cursor->header.height);
+goto fail;
+}
+
c->hot_x = cursor->header.hot_spot_x;
c->hot_y = cursor->header.hot_spot_y;
switch (cursor->header.type) {
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 98c83474ad..45d06cbe25 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -515,6 +515,8 @@ static inline void vmsvga_cursor_define(struct
vmsvga_state_s *s,
int i, pixels;
qc = cursor_alloc(c->width, c->height);
+assert(qc != NULL);
+
qc->hot_x = c->hot_x;
qc->hot_y = c->hot_y;
switch (c->bpp) {
diff --git a/ui/cursor.c b/ui/cursor.c
index 1d62ddd4d0..835f0802f9 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -46,6 +46,8 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
/* parse pixel data */
c = cursor_alloc(width, height);
+assert(c != NULL);
+
for (pixel = 0, y = 0; y < height; y++, line++) {
for (x = 0; x < height; x++, pixel++) {
idx = xpm[line][x];
@@ -91,7 +93,11 @@ QEMUCursor *cursor_builtin_left_ptr(void)
QEMUCursor *cursor_alloc(int width, int height)
{
QEMUCursor *c;
-int datasize = width * height * sizeof(uint32_t);
+size_t datasize = width * height * sizeof(uint32_t);
+
+if (width > 512 || height > 512) {
+return NULL;
+}
c = g_malloc0(sizeof(QEMUCursor) + datasize);
c->width = width;
--
2.35.1
[PATCH v2] display/qxl-render: fix race condition in qxl_cursor (CVE-2021-4207)
Avoid fetching 'width' and 'height' a second time to prevent possible
race condition. Refer to security advisory
https://starlabs.sg/advisories/22-4207/ for more information.
Fixes: CVE-2021-4207
Signed-off-by: Mauro Matteo Cascella
---
v2:
- fix CVE id (CVE-2021-4207 instead of CVE-2022-4207)
hw/display/qxl-render.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index d28849b121..237ed293ba 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -266,7 +266,7 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor
*cursor,
}
break;
case SPICE_CURSOR_TYPE_ALPHA:
-size = sizeof(uint32_t) * cursor->header.width * cursor->header.height;
+size = sizeof(uint32_t) * c->width * c->height;
qxl_unpack_chunks(c->data, size, qxl, >chunk, group_id);
if (qxl->debug > 2) {
cursor_print_ascii_art(c, "qxl/alpha");
--
2.35.1
[PATCH v2] ui/cursor: fix integer overflow in cursor_alloc (CVE-2022-4206)
Prevent potential integer overflow by limiting 'width' and 'height' to
512x512. Also change 'datasize' type to size_t. Refer to security
advisory https://starlabs.sg/advisories/22-4206/ for more information.
Fixes: CVE-2022-4206
Signed-off-by: Mauro Matteo Cascella
---
v2:
- braces on if statement in cursor_alloc
- assert in vmsvga_cursor_define and cursor_parse_xpm
hw/display/qxl-render.c | 7 +++
hw/display/vmware_vga.c | 2 ++
ui/cursor.c | 8 +++-
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index d28849b121..dc3c4edd05 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -247,6 +247,13 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor
*cursor,
size_t size;
c = cursor_alloc(cursor->header.width, cursor->header.height);
+
+if (!c) {
+qxl_set_guest_bug(qxl, "%s: cursor %ux%u alloc error", __func__,
+cursor->header.width, cursor->header.height);
+goto fail;
+}
+
c->hot_x = cursor->header.hot_spot_x;
c->hot_y = cursor->header.hot_spot_y;
switch (cursor->header.type) {
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 98c83474ad..45d06cbe25 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -515,6 +515,8 @@ static inline void vmsvga_cursor_define(struct
vmsvga_state_s *s,
int i, pixels;
qc = cursor_alloc(c->width, c->height);
+assert(qc != NULL);
+
qc->hot_x = c->hot_x;
qc->hot_y = c->hot_y;
switch (c->bpp) {
diff --git a/ui/cursor.c b/ui/cursor.c
index 1d62ddd4d0..835f0802f9 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -46,6 +46,8 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
/* parse pixel data */
c = cursor_alloc(width, height);
+assert(c != NULL);
+
for (pixel = 0, y = 0; y < height; y++, line++) {
for (x = 0; x < height; x++, pixel++) {
idx = xpm[line][x];
@@ -91,7 +93,11 @@ QEMUCursor *cursor_builtin_left_ptr(void)
QEMUCursor *cursor_alloc(int width, int height)
{
QEMUCursor *c;
-int datasize = width * height * sizeof(uint32_t);
+size_t datasize = width * height * sizeof(uint32_t);
+
+if (width > 512 || height > 512) {
+return NULL;
+}
c = g_malloc0(sizeof(QEMUCursor) + datasize);
c->width = width;
--
2.35.1
Re: [PATCH] ui/cursor: fix integer overflow in cursor_alloc (CVE-2022-4206)
On Tue, Apr 5, 2022 at 1:10 PM Gerd Hoffmann wrote:
>
> > > +++ b/ui/cursor.c
> > > @@ -46,6 +46,13 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
> > >
> > > /* parse pixel data */
> > > c = cursor_alloc(width, height);
> > > +
> > > +if (!c) {
> > > +fprintf(stderr, "%s: cursor %ux%u alloc error\n",
> > > +__func__, width, height);
> > > +return NULL;
> > > +}
> > >
> >
> > I think you could simply abort() in this function. It is used with static
> > data (ui/cursor*.xpm)
>
> Yes, that should never happen.
>
> Missing: vmsvga_cursor_define() calls cursor_alloc() with guest-supplied
> values too.
I skipped that because the check (cursor.width > 256 || cursor.height
> 256) is already done in vmsvga_fifo_run before calling
vmsvga_cursor_define. You want me to add another check in
vmsvga_cursor_define and return NULL if cursor_alloc fails?
> take care,
> Gerd
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH] display/qxl-render: fix race condition in qxl_cursor (CVE-2022-4207)
Avoid fetching 'width' and 'height' a second time to prevent possible
race condition. Refer to security advisory
https://starlabs.sg/advisories/22-4207/ for more information.
Fixes: CVE-2022-4207
Signed-off-by: Mauro Matteo Cascella
---
hw/display/qxl-render.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index d28849b121..237ed293ba 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -266,7 +266,7 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor
*cursor,
}
break;
case SPICE_CURSOR_TYPE_ALPHA:
-size = sizeof(uint32_t) * cursor->header.width * cursor->header.height;
+size = sizeof(uint32_t) * c->width * c->height;
qxl_unpack_chunks(c->data, size, qxl, >chunk, group_id);
if (qxl->debug > 2) {
cursor_print_ascii_art(c, "qxl/alpha");
--
2.35.1
[PATCH] ui/cursor: fix integer overflow in cursor_alloc (CVE-2022-4206)
Prevent potential integer overflow by limiting 'width' and 'height' to
512x512. Also change 'datasize' type to size_t. Refer to security
advisory https://starlabs.sg/advisories/22-4206/ for more information.
Fixes: CVE-2022-4206
Signed-off-by: Mauro Matteo Cascella
---
hw/display/qxl-render.c | 7 +++
ui/cursor.c | 12 +++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index d28849b121..dc3c4edd05 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -247,6 +247,13 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor
*cursor,
size_t size;
c = cursor_alloc(cursor->header.width, cursor->header.height);
+
+if (!c) {
+qxl_set_guest_bug(qxl, "%s: cursor %ux%u alloc error", __func__,
+cursor->header.width, cursor->header.height);
+goto fail;
+}
+
c->hot_x = cursor->header.hot_spot_x;
c->hot_y = cursor->header.hot_spot_y;
switch (cursor->header.type) {
diff --git a/ui/cursor.c b/ui/cursor.c
index 1d62ddd4d0..7cfb08a030 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -46,6 +46,13 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
/* parse pixel data */
c = cursor_alloc(width, height);
+
+if (!c) {
+fprintf(stderr, "%s: cursor %ux%u alloc error\n",
+__func__, width, height);
+return NULL;
+}
+
for (pixel = 0, y = 0; y < height; y++, line++) {
for (x = 0; x < height; x++, pixel++) {
idx = xpm[line][x];
@@ -91,7 +98,10 @@ QEMUCursor *cursor_builtin_left_ptr(void)
QEMUCursor *cursor_alloc(int width, int height)
{
QEMUCursor *c;
-int datasize = width * height * sizeof(uint32_t);
+size_t datasize = width * height * sizeof(uint32_t);
+
+if (width > 512 || height > 512)
+return NULL;
c = g_malloc0(sizeof(QEMUCursor) + datasize);
c->width = width;
--
2.35.1
Re: [PATCH] acpi: validate hotplug selector on access
On Thu, Dec 23, 2021 at 2:43 PM Michael S. Tsirkin wrote:
>
> On Thu, Dec 23, 2021 at 10:58:14AM +0100, Mauro Matteo Cascella wrote:
> > Hi,
> >
> > On Wed, Dec 22, 2021 at 9:52 PM Michael S. Tsirkin wrote:
> > >
> > > On Wed, Dec 22, 2021 at 09:27:51PM +0100, Philippe Mathieu-Daudé wrote:
> > > > On Wed, Dec 22, 2021 at 9:20 PM Michael S. Tsirkin
> > > > wrote:
> > > > > On Wed, Dec 22, 2021 at 08:19:41PM +0100, Philippe Mathieu-Daudé
> > > > > wrote:
> > > > > > +Mauro & Alex
> > > > > >
> > > > > > On 12/21/21 15:48, Michael S. Tsirkin wrote:
> > > > > > > When bus is looked up on a pci write, we didn't
> > > > > > > validate that the lookup succeeded.
> > > > > > > Fuzzers thus can trigger QEMU crash by dereferencing the NULL
> > > > > > > bus pointer.
> > > > > > >
> > > > > > > Fixes: b32bd763a1 ("pci: introduce acpi-index property for PCI
> > > > > > > device")
> > > > > > > Cc: "Igor Mammedov"
> > > > > > > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/770
> > > > > > > Signed-off-by: Michael S. Tsirkin
> > > > > >
> > > > > > It seems this problem is important enough to get a CVE assigned.
> > > > >
> > > > > Guest root can crash guest.
> > > > > I don't see why we would assign a CVE.
> > > >
> > > > Well thinking about downstream distributions, if there is a CVE
> > > > assigned,
> > > > it helps them to have it written in the commit. Maybe I am mistaken.
> > > >
> > > > Unrelated but it seems there is a coordination problem with the
> > > > qemu-security@ list,
> > > > if this isn't a security issue, why was a CVE requested?
> > >
> > > Right. I don't think a priveleged user crashing VM warrants a CVE,
> > > it can just halt a CPU or whatever. Just cancel the CVE request pls.
> >
> > While I agree with you that this is kind of borderline and I expressed
> > similar concerns in the past, I was told that:
> >
> > 1) root guest users are not necessarily trustworthy (from the host
> > perspective).
> > 2) NULL pointer deref and similar issues caused by an
> > ill-handled/error condition are CVE worthy, even if triggered by root.
> > 3) In other cases, DoS triggered by root is not a security issue
> > because it's an expected behavior and not an ill-handled/error
> > condition (think of assert failures, for example).
> >
> > In other words, "ill-handled condition" is the crucial factor that
> > makes a bug CVE worthy or not.
>
> I guess the point is that a downstream might have a slightly different
> code path where it would be more serious ...
> OK then, not a big deal for me. So what's the CVE # then?
CVE-2021-4158 has been assigned for this issue.
> > +Prasad, can you shed some light on this? Is my understanding correct?
> >
> > Also, please note that we regularly get CVE requests for bugs like
> > this and many CVEs have been assigned in the past. Of course that
> > doesn't mean we can't change things going forward, but I think we
> > should make it clear (probably here:
> > https://www.qemu.org/docs/master/system/security.html) that these
> > kinds of bugs are not eligible for CVE assignment.
Thanks.
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] acpi: validate hotplug selector on access
Hi,
On Wed, Dec 22, 2021 at 9:52 PM Michael S. Tsirkin wrote:
>
> On Wed, Dec 22, 2021 at 09:27:51PM +0100, Philippe Mathieu-Daudé wrote:
> > On Wed, Dec 22, 2021 at 9:20 PM Michael S. Tsirkin wrote:
> > > On Wed, Dec 22, 2021 at 08:19:41PM +0100, Philippe Mathieu-Daudé wrote:
> > > > +Mauro & Alex
> > > >
> > > > On 12/21/21 15:48, Michael S. Tsirkin wrote:
> > > > > When bus is looked up on a pci write, we didn't
> > > > > validate that the lookup succeeded.
> > > > > Fuzzers thus can trigger QEMU crash by dereferencing the NULL
> > > > > bus pointer.
> > > > >
> > > > > Fixes: b32bd763a1 ("pci: introduce acpi-index property for PCI
> > > > > device")
> > > > > Cc: "Igor Mammedov"
> > > > > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/770
> > > > > Signed-off-by: Michael S. Tsirkin
> > > >
> > > > It seems this problem is important enough to get a CVE assigned.
> > >
> > > Guest root can crash guest.
> > > I don't see why we would assign a CVE.
> >
> > Well thinking about downstream distributions, if there is a CVE assigned,
> > it helps them to have it written in the commit. Maybe I am mistaken.
> >
> > Unrelated but it seems there is a coordination problem with the
> > qemu-security@ list,
> > if this isn't a security issue, why was a CVE requested?
>
> Right. I don't think a priveleged user crashing VM warrants a CVE,
> it can just halt a CPU or whatever. Just cancel the CVE request pls.
While I agree with you that this is kind of borderline and I expressed
similar concerns in the past, I was told that:
1) root guest users are not necessarily trustworthy (from the host perspective).
2) NULL pointer deref and similar issues caused by an
ill-handled/error condition are CVE worthy, even if triggered by root.
3) In other cases, DoS triggered by root is not a security issue
because it's an expected behavior and not an ill-handled/error
condition (think of assert failures, for example).
In other words, "ill-handled condition" is the crucial factor that
makes a bug CVE worthy or not.
+Prasad, can you shed some light on this? Is my understanding correct?
Also, please note that we regularly get CVE requests for bugs like
this and many CVEs have been assigned in the past. Of course that
doesn't mean we can't change things going forward, but I think we
should make it clear (probably here:
https://www.qemu.org/docs/master/system/security.html) that these
kinds of bugs are not eligible for CVE assignment.
> > > > Mauro, please update us when you get the CVE number.
> > > > Michael, please amend the CVE number before committing the fix.
> > > >
> > > > FWIW Paolo asked every fuzzed bug reproducer to be committed
> > > > as qtest, see tests/qtest/fuzz*c. Alex has a way to generate
> > > > reproducer in plain C.
> > > >
> > > > Regards,
> > > >
> > > > Phil.
> > >
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [RFC PATCH] memory: Fix dma-reentrancy issues at the MMIO level
On Fri, Dec 17, 2021 at 7:28 AM Qiuhao Li wrote: > > Thanks Alex. It seems this patch sets and checks if the destination device is > busy. But how about the data transfers not triggered directly by PMIO/MMIO > handlers? For example: > > 1. Device A Timer's callback -> Device A MMIO handler > 2. Device A BH's callback -> Device A MMIO handler > > In these situations, when A launches a DMA to itself, the > dev->engaged_in_direct_io is not set, so the operation is allowed. Maybe we > should log the source and check the destination when we launch data > transfers. Is there a way to do that? > > Below is a reproducer in NVMe which triggers DMA in a timer's callback > (nvme_process_sq). I can still trigger use-after-free exception with this > patch on qemu-6.1.0: > > cat << EOF | ./qemu-system-x86_64 -display none -machine accel=qtest \ > -machine q35 -nodefaults -drive file=null-co://,if=none,format=raw,id=disk0 \ > -device nvme,drive=disk0,serial=1 -qtest stdio \ > > outl 0xcf8 0x8810 /* MLBAR (BAR0) – Memory Register Base > Address, lower 32-bits */ > outl 0xcfc 0xe000 /* MMIO Base Address = 0xe000 */ > outl 0xcf8 0x8804 /* CMD - Command */ > outw 0xcfc 0x06 /* Bus Master Enable, Memory Space Enable > */ > write 0xe024 0x4 0x02000200 /* [3] 3.1.8, Admin Queue Attributes */ > write 0xe028 0x4 0x0010 /* asq = 0x1000 */ > write 0xe030 0x4 0x0020 /* acq = 0x2000 */ > write 0xe014 0x4 0x01004600 /* [3] 3.1.5, Controller Configuration, > start ctrl */ > write 0xe0001000 0x1 0x01 /* [3] 3.1.24, SQyTDBL – Submission Queue > y Tail Doorbell */ > write 0x1000 0x1 0x02 /* cmd->opcode, > NVME_ADM_CMD_GET_LOG_PAGE, nvme_get_log() */ > write 0x1018 0x4 0x14e0 /* prp1 = 0xe014, NVME_REG_CC, > nvme_ctrl_reset() */ > write 0x1028 0x4 0x0304 /* cmd->cdw10, lid = 3 > NVME_LOG_FW_SLOT_INFO, nvme_fw_log_info, buf_len = 4 */ > write 0x1030 0x4 0xfc010000 /* cmd->cdw12 = 0x1fc, Log Page Offset, > trans_len = sizeof(fw_log) - 0x1fc = 4 */ > clock_step > EOF > > CC: Mauro Matteo Cascella and Philippe Mathieu-Daudé. Should we put the > reproducer above to https://gitlab.com/qemu-project/qemu/-/issues/556?? Upstream issue: https://gitlab.com/qemu-project/qemu/-/issues/782 Thanks. -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH 2/2] hw/nvme/ctrl: Prohibit DMA accesses to devices (CVE-2021-3929)
On Thu, Dec 16, 2021 at 6:55 PM Philippe Mathieu-Daudé
wrote:
>
> Fixes: CVE-2021-3929
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=2020298
> Reported-by: Qiuhao Li
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/nvme/ctrl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 604ed0aea0d..2be2c340b34 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -1146,7 +1146,7 @@ static uint16_t nvme_tx(NvmeCtrl *n, NvmeSg *sg,
> uint8_t *ptr, uint32_t len,
> assert(sg->flags & NVME_SG_ALLOC);
>
> if (sg->flags & NVME_SG_DMA) {
> -const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> +const MemTxAttrs attrs = { .memory = true };
> MemTxResult res;
> uint64_t residual;
>
> --
> 2.33.1
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH] hw/scsi/scsi-disk: MODE_PAGE_ALLS not allowed in MODE SELECT commands
MODE_PAGE_ALLS causes an off-by-one error in mode_sense_page() when accessing
the stack-allocated mode_sense_valid buffer. MODE_PAGE_ALLS is only valid for
MODE SENSE commands. Do not process it in MODE SELECT commands.
Fixes: CVE-2021-3930
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=2020588
Signed-off-by: Mauro Matteo Cascella
Reported-by: Qiuhao Li
---
hw/scsi/scsi-disk.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e8a547dbb7..5852e8dcfd 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1087,6 +1087,7 @@ static int mode_sense_page(SCSIDiskState *s, int page,
uint8_t **p_outbuf,
uint8_t *p = *p_outbuf + 2;
int length;
+assert(page != MODE_PAGE_ALLS);
if ((mode_sense_valid[page] & (1 << s->qdev.type)) == 0) {
return -1;
}
@@ -1428,6 +1429,11 @@ static int scsi_disk_check_mode_select(SCSIDiskState *s,
int page,
return -1;
}
+/* MODE_PAGE_ALLS is only valid for MODE SENSE commands */
+if (page == MODE_PAGE_ALLS) {
+return -1;
+}
+
p = mode_current;
memset(mode_current, 0, inlen + 2);
len = mode_sense_page(s, page, , 0);
--
2.31.1
Re: [PATCH] hw/display/ati_2d: Fix buffer overflow in ati_2d_blt (CVE-2021-3638)
On Tue, Sep 7, 2021 at 8:22 AM Philippe Mathieu-Daudé wrote:
>
> On 9/7/21 7:38 AM, Philippe Mathieu-Daudé wrote:
> > On 9/6/21 9:52 PM, BALATON Zoltan wrote:
> >> I don't think assigning a CVE to a bug that is in an experimental and
> >> largely unused part and happens when one enables debug code really worth
> >> the hassle, this could be handled as a normal bug. As long as the
> >
> > CVE assignment can happens outside of QEMU community, we try to make it
> > clear what is the "security boundary" but researchers filling CVEs
> > might not understand it well.
>
> BTW see commit b317006a3f1 ("docs/secure-coding-practices: Describe how
> to use 'null-co' block driver") which is related to your suggestion.
I agree we can avoid assigning CVEs to ati-vga and similar
experimental devices that are not intended to be used in production,
even if they fall under the virtualization use case. Maybe we can
improve the documentation
(https://qemu-project.gitlab.io/qemu/system/security.html) to clearly
state that some devices are not security supported? Would it be
possible/feasible to get a list of such devices? Or maybe the other
way around, document the list of devices that are undeniably security
supported (e.g., virtio*, *hci, e1000, etc.)?
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] hw/display/ati_2d: Fix buffer overflow in ati_2d_blt (CVE-2021-3638)
On Mon, Sep 6, 2021 at 5:31 PM Philippe Mathieu-Daudé wrote:
>
> When building QEMU with DEBUG_ATI defined then running with
> '-device ati-vga,romfile="" -d unimp,guest_errors -trace ati\*'
> we get:
>
> ati_mm_write 4 0x16c0 DP_CNTL <- 0x1
> ati_mm_write 4 0x146c DP_GUI_MASTER_CNTL <- 0x2
> ati_mm_write 4 0x16c8 DP_MIX <- 0xff
> ati_mm_write 4 0x16c4 DP_DATATYPE <- 0x2
> ati_mm_write 4 0x224 CRTC_OFFSET <- 0x0
> ati_mm_write 4 0x142c DST_PITCH_OFFSET <- 0xfe0
> ati_mm_write 4 0x1420 DST_Y <- 0x3fff
> ati_mm_write 4 0x1410 DST_HEIGHT <- 0x3fff
> ati_mm_write 4 0x1588 DST_WIDTH_X <- 0x3fff3fff
> ati_2d_blt: vram:0x7fff5fa0 addr:0 ds:0x7fff61273800 stride:2560 bpp:32
> rop:0xff
> ati_2d_blt: 0 0 0, 0 127 0, (0,0) -> (16383,16383) 16383x16383 > ^
> ati_2d_blt: pixman_fill(dst:0x7fff5fa0, stride:254, bpp:8, x:16383,
> y:16383, w:16383, h:16383, xor:0xff00)
> Thread 3 "qemu-system-i38" received signal SIGSEGV, Segmentation fault.
> (gdb) bt
> #0 0x77f62ce0 in sse2_fill.lto_priv () at /lib64/libpixman-1.so.0
> #1 0x77f09278 in pixman_fill () at /lib64/libpixman-1.so.0
> #2 0x57b5a9af in ati_2d_blt (s=0x63128800) at
> hw/display/ati_2d.c:196
> #3 0x57b4b5a2 in ati_mm_write (opaque=0x63128800, addr=5512,
> data=1073692671, size=4) at hw/display/ati.c:843
> #4 0x58b90ec4 in memory_region_write_accessor (mr=0x63139cc0,
> addr=5512, ..., size=4, ...) at softmmu/memory.c:492
>
> Commit 584acf34cb0 ("ati-vga: Fix reverse bit blts") introduced
> the local dst_x and dst_y which adjust the (x, y) coordinates
> depending on the direction in the SRCCOPY ROP3 operation, but
> forgot to address the same issue for the PATCOPY, BLACKNESS and
> WHITENESS operations, which also call pixman_fill().
>
> Fix that now by using the adjusted coordinates in the pixman_fill
> call, and update the related debug printf().
>
> Reported-by: Qiang Liu
> Fixes: 584acf34cb0 ("ati-vga: Fix reverse bit blts")
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/display/ati_2d.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
> index 4dc10ea7952..692bec91de4 100644
> --- a/hw/display/ati_2d.c
> +++ b/hw/display/ati_2d.c
> @@ -84,7 +84,7 @@ void ati_2d_blt(ATIVGAState *s)
> DPRINTF("%d %d %d, %d %d %d, (%d,%d) -> (%d,%d) %dx%d %c %c\n",
> s->regs.src_offset, s->regs.dst_offset, s->regs.default_offset,
> s->regs.src_pitch, s->regs.dst_pitch, s->regs.default_pitch,
> -s->regs.src_x, s->regs.src_y, s->regs.dst_x, s->regs.dst_y,
> +s->regs.src_x, s->regs.src_y, dst_x, dst_y,
> s->regs.dst_width, s->regs.dst_height,
> (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ? '>' : '<'),
> (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ? 'v' : '^'));
> @@ -180,11 +180,11 @@ void ati_2d_blt(ATIVGAState *s)
> dst_stride /= sizeof(uint32_t);
> DPRINTF("pixman_fill(%p, %d, %d, %d, %d, %d, %d, %x)\n",
> dst_bits, dst_stride, bpp,
> -s->regs.dst_x, s->regs.dst_y,
> +dst_x, dst_y,
> s->regs.dst_width, s->regs.dst_height,
> filler);
> pixman_fill((uint32_t *)dst_bits, dst_stride, bpp,
> -s->regs.dst_x, s->regs.dst_y,
> +dst_x, dst_y,
> s->regs.dst_width, s->regs.dst_height,
> filler);
> if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr &&
> --
> 2.31.1
>
Tested-by: Mauro Matteo Cascella
Thanks.
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH 0/1] uas: add stream number sanity checks (maybe 6.1)
Hi, On Fri, Aug 20, 2021 at 3:07 PM Philippe Mathieu-Daudé wrote: > > Cc'ing Mauro to double-check. > > On 8/20/21 2:12 PM, Peter Maydell wrote: > > On Wed, 18 Aug 2021 at 13:10, Gerd Hoffmann wrote: > >> > >> Security fix. Sorry for the last-minute patch, I had completely > >> forgotten this one until the CVE number for it arrived today. > >> > >> Given that the classic usb storage device is way more popular than > >> the uas (usb attached scsi) device the impact should be pretty low > >> and we might consider to not screw up our release schedule for this. > > > > What's the impact if the bug is exploited ? > > Bug class: "guest-triggered user-after-free". > > Being privileged (root) in the guest, you can leak some data from > the host process then DoS the host or potentially exploit the > use-after-free to execute code on the host. > This is actually an out-of-bounds access issue (not UAF). It's still potentially bad, but I agree with Gerd the impact is low. Plus there's an assert right before [1] that makes it a DoS if the accessed memory is not NULL. [1] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/usb/dev-uas.c#L850 Regards. -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] hw/net: Discard overly fragmented packets
ot;\x01", 0x1);
> +qtest_bufwrite(s, 0x2c9, "\x40", 0x1);
> +qtest_bufwrite(s, 0x2d8, "\x01", 0x1);
> +qtest_bufwrite(s, 0x2d9, "\x40", 0x1);
> +qtest_bufwrite(s, 0x2e8, "\x01", 0x1);
> +qtest_bufwrite(s, 0x2e9, "\x40", 0x1);
> +qtest_bufwrite(s, 0x2f8, "\x01", 0x1);
> +qtest_bufwrite(s, 0x2f9, "\x40", 0x1);
> +qtest_bufwrite(s, 0x308, "\x01", 0x1);
> +qtest_bufwrite(s, 0x309, "\x40", 0x1);
> +qtest_bufwrite(s, 0x318, "\x01", 0x1);
> +qtest_bufwrite(s, 0x319, "\x40", 0x1);
> +qtest_bufwrite(s, 0x328, "\x01", 0x1);
> +qtest_bufwrite(s, 0x329, "\x40", 0x1);
> +qtest_bufwrite(s, 0x338, "\x01", 0x1);
> +qtest_bufwrite(s, 0x339, "\x40", 0x1);
> +qtest_bufwrite(s, 0x348, "\x01", 0x1);
> +qtest_bufwrite(s, 0x349, "\x40", 0x1);
> +qtest_bufwrite(s, 0x358, "\x01", 0x1);
> +qtest_bufwrite(s, 0x359, "\x40", 0x1);
> +qtest_bufwrite(s, 0x368, "\x01", 0x1);
> +qtest_bufwrite(s, 0x369, "\x40", 0x1);
> +qtest_bufwrite(s, 0x378, "\x01", 0x1);
> +qtest_bufwrite(s, 0x379, "\x40", 0x1);
> +qtest_bufwrite(s, 0x388, "\x01", 0x1);
> +qtest_bufwrite(s, 0x389, "\x40", 0x1);
> +qtest_bufwrite(s, 0x398, "\x01", 0x1);
> +qtest_bufwrite(s, 0x399, "\x40", 0x1);
> +qtest_bufwrite(s, 0x3a8, "\x01", 0x1);
> +qtest_bufwrite(s, 0x3a9, "\x40", 0x1);
> +qtest_bufwrite(s, 0x3b8, "\x01", 0x1);
> +qtest_bufwrite(s, 0x3b9, "\x40", 0x1);
> +qtest_bufwrite(s, 0x3c8, "\x01", 0x1);
> +qtest_bufwrite(s, 0x3c9, "\x40", 0x1);
> +qtest_bufwrite(s, 0x3d8, "\x01", 0x1);
> +qtest_bufwrite(s, 0x3d9, "\x40", 0x1);
> +qtest_bufwrite(s, 0x3e8, "\x01", 0x1);
> +qtest_bufwrite(s, 0x3e9, "\x40", 0x1);
> +qtest_bufwrite(s, 0x3f8, "\x01", 0x1);
> +qtest_bufwrite(s, 0x3f9, "\x40", 0x1);
> +qtest_bufwrite(s, 0xd, "\x10", 0x1);
> +qtest_bufwrite(s, 0x2600, "\x00", 0x1);
> +qtest_quit(s);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +const char *arch = qtest_get_arch();
> +
> +g_test_init(, , NULL);
> +
> +if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +qtest_add_func("fuzz/test_oss_35799_eth_setup_ip4_fragmentation",
> + test_oss_35799_eth_setup_ip4_fragmentation);
> +}
> +
> +return g_test_run();
> +}
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cb8f3ea2c2e..43e5050ad96 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2001,6 +2001,7 @@ S: Maintained
> F: hw/net/vmxnet*
> F: hw/scsi/vmw_pvscsi*
> F: tests/qtest/vmxnet3-test.c
> +F: tests/qtest/fuzz-vmxnet3-test.c
>
> Rocker
> M: Jiri Pirko
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index b03e8541700..42add92e9d4 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -66,6 +66,7 @@
>(config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-swtpm-test']
> : []) +\
>(config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : [])
> + \
>(config_all_devices.has_key('CONFIG_E1000E_PCI_EXPRESS') ?
> ['fuzz-e1000e-test'] : []) + \
> + (config_all_devices.has_key('CONFIG_VMXNET3_PCI') ? ['fuzz-vmxnet3-test']
> : []) + \
>(config_all_devices.has_key('CONFIG_ESP_PCI') ? ['am53c974-test'] : []) +
>\
>qtests_pci +
>\
>['fdc-test',
> --
> 2.31.1
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[Bug 1907497] Re: [OSS-Fuzz] Issue 28435 qemu:qemu-fuzz-i386-target-generic-fuzz-intel-hda: Stack-overflow in ldl_le_dma
Just FYI, this issue was assigned CVE-2021-3611 by Red Hat. ** CVE added: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2021-3611 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1907497 Title: [OSS-Fuzz] Issue 28435 qemu:qemu-fuzz-i386-target-generic-fuzz-intel- hda: Stack-overflow in ldl_le_dma Status in QEMU: Confirmed Bug description: affects qemu === Reproducer (build with --enable-sanitizers) === cat << EOF | ./qemu-system-i386 -machine q35 -nodefaults \ -device intel-hda,id=hda0 -device hda-output,bus=hda0.0 \ -device hda-micro,bus=hda0.0 -device hda-duplex,bus=hda0.0 \ -qtest stdio outl 0xcf8 0x8804 outw 0xcfc 0x write 0x0 0x1 0x12 write 0x2 0x1 0x2f outl 0xcf8 0x8811 outl 0xcfc 0x5a6a4406 write 0x6a44005a 0x1 0x11 write 0x6a44005c 0x1 0x3f write 0x6a442050 0x4 0x446a write 0x6a44204a 0x1 0xf3 write 0x6a44204c 0x1 0xff writeq 0x6a44005a 0x17b3f0011 write 0x6a442050 0x4 0x446a write 0x6a44204a 0x1 0xf3 write 0x6a44204c 0x1 0xff EOF === Stack Trace === ==411958==ERROR: AddressSanitizer: stack-overflow on address 0x7ffcaeb8bc88 (pc 0x55c7c9dc1159 bp 0x7ffcaeb8c4d0 sp 0x7ffcaeb8bc90 T0) #0 0x55c7c9dc1159 in __asan_memcpy (u-system-i386+0x2a13159) #1 0x55c7cb2a457e in flatview_do_translate softmmu/physmem.c:513:12 #2 0x55c7cb2bdab0 in flatview_translate softmmu/physmem.c:563:15 #3 0x55c7cb2bdab0 in flatview_read softmmu/physmem.c:2861:10 #4 0x55c7cb2bdab0 in address_space_read_full softmmu/physmem.c:2875:18 #5 0x55c7caaec937 in dma_memory_rw_relaxed include/sysemu/dma.h:87:18 #6 0x55c7caaec937 in dma_memory_rw include/sysemu/dma.h:110:12 #7 0x55c7caaec937 in dma_memory_read include/sysemu/dma.h:116:12 #8 0x55c7caaec937 in ldl_le_dma include/sysemu/dma.h:179:1 #9 0x55c7caaec937 in ldl_le_pci_dma include/hw/pci/pci.h:816:1 #10 0x55c7caaec937 in intel_hda_corb_run hw/audio/intel-hda.c:338:16 #11 0x55c7cb2e7198 in memory_region_write_accessor softmmu/memory.c:491:5 #12 0x55c7cb2e6bd3 in access_with_adjusted_size softmmu/memory.c:552:18 #13 0x55c7cb2e646c in memory_region_dispatch_write softmmu/memory.c #14 0x55c7cb2c8445 in flatview_write_continue softmmu/physmem.c:2759:23 #15 0x55c7cb2bdfb8 in flatview_write softmmu/physmem.c:2799:14 #16 0x55c7cb2bdfb8 in address_space_write softmmu/physmem.c:2891:18 #17 0x55c7caae2c54 in dma_memory_rw_relaxed include/sysemu/dma.h:87:18 #18 0x55c7caae2c54 in dma_memory_rw include/sysemu/dma.h:110:12 #19 0x55c7caae2c54 in dma_memory_write include/sysemu/dma.h:122:12 #20 0x55c7caae2c54 in stl_le_dma include/sysemu/dma.h:179:1 #21 0x55c7caae2c54 in stl_le_pci_dma include/hw/pci/pci.h:816:1 #22 0x55c7caae2c54 in intel_hda_response hw/audio/intel-hda.c:370:5 #23 0x55c7caaeca00 in intel_hda_corb_run hw/audio/intel-hda.c:342:9 #24 0x55c7cb2e7198 in memory_region_write_accessor softmmu/memory.c:491:5 ... OSS-Fuzz Report: https://bugs.chromium.org/p/oss- fuzz/issues/detail?id=28435 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1907497/+subscriptions
Re: [PATCH v2] Test comment for git-publish
On Fri, Jun 11, 2021 at 6:43 PM Mauro Matteo Cascella
wrote:
>
> ---
> hw/rdma/vmw/pvrdma_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index 84ae8024fc..e229c19564 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -427,7 +427,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr,
> uint64_t val,
> case PVRDMA_REG_REQUEST:
> if (val == 0) {
> trace_pvrdma_regs_write(addr, val, "REQUEST", "");
> -pvrdma_exec_cmd(dev);
> +pvrdma_exec_cmd(dev); // this is a test comment
> }
> break;
> default:
> --
> 2.31.1
>
Again, sorry for the spam. Can someone please explain how to *not* use
the profiles defined in .gitpublish? I used the following command,
evidently with no success:
$ git publish --override-cc --override-to --to=mcasc...@redhat.com
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH v2] Test comment for git-publish
---
hw/rdma/vmw/pvrdma_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 84ae8024fc..e229c19564 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -427,7 +427,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr,
uint64_t val,
case PVRDMA_REG_REQUEST:
if (val == 0) {
trace_pvrdma_regs_write(addr, val, "REQUEST", "");
-pvrdma_exec_cmd(dev);
+pvrdma_exec_cmd(dev); // this is a test comment
}
break;
default:
--
2.31.1
Re: [PATCH] Test comment for git-publish
On Fri, Jun 11, 2021 at 6:20 PM Mauro Matteo Cascella
wrote:
>
> ---
> hw/rdma/vmw/pvrdma_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index 84ae8024fc..e229c19564 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -427,7 +427,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr,
> uint64_t val,
> case PVRDMA_REG_REQUEST:
> if (val == 0) {
> trace_pvrdma_regs_write(addr, val, "REQUEST", "");
> -pvrdma_exec_cmd(dev);
> +pvrdma_exec_cmd(dev); // this is a test comment
> }
> break;
> default:
> --
> 2.31.1
>
Sorry, please disregard this "patch" =)
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[PATCH] Test comment for git-publish
---
hw/rdma/vmw/pvrdma_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 84ae8024fc..e229c19564 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -427,7 +427,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr,
uint64_t val,
case PVRDMA_REG_REQUEST:
if (val == 0) {
trace_pvrdma_regs_write(addr, val, "REQUEST", "");
-pvrdma_exec_cmd(dev);
+pvrdma_exec_cmd(dev); // this is a test comment
}
break;
default:
--
2.31.1
[PATCH] ui/spice-display: check NULL pointer in interface_release_resource()
Check rext.info to avoid potential NULL pointer dereference. A similar check
exists in interface_release_resource() in hw/display/qxl.c.
Reported-by: Yu Lu
Signed-off-by: Mauro Matteo Cascella
---
ui/spice-display.c | 4
1 file changed, 4 insertions(+)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index d22781a23d..f59c69882d 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -561,6 +561,10 @@ static void interface_release_resource(QXLInstance *sin,
SimpleSpiceCursor *cursor;
QXLCommandExt *ext;
+if (!rext.info) {
+return;
+}
+
ext = (void *)(intptr_t)(rext.info->id);
switch (ext->cmd.type) {
case QXL_CMD_DRAW:
--
2.31.1
Re: [PATCH v2 2/5] usb/redir: avoid dynamic stack allocation (CVE-2021-3527)
On Mon, May 3, 2021 at 3:29 PM Gerd Hoffmann wrote:
>
> Use autofree heap allocation instead.
>
> Fixes: 4f4321c11ff ("usb: use iovecs in USBPacket")
> Reviewed-by: Philippe Mathieu-Daudé
> Signed-off-by: Gerd Hoffmann
> ---
> hw/usb/redirect.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 17f06f34179a..6a75b0dc4ab2 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -620,7 +620,7 @@ static void usbredir_handle_iso_data(USBRedirDevice *dev,
> USBPacket *p,
> .endpoint = ep,
> .length = p->iov.size
> };
> -uint8_t buf[p->iov.size];
> +g_autofree uint8_t *buf = g_malloc(p->iov.size);
> /* No id, we look at the ep when receiving a status back */
> usb_packet_copy(p, buf, p->iov.size);
> usbredirparser_send_iso_packet(dev->parser, 0, _packet,
> @@ -818,7 +818,7 @@ static void usbredir_handle_bulk_data(USBRedirDevice
> *dev, USBPacket *p,
> usbredirparser_send_bulk_packet(dev->parser, p->id,
> _packet, NULL, 0);
> } else {
> -uint8_t buf[size];
> +g_autofree uint8_t *buf = g_malloc(size);
> usb_packet_copy(p, buf, size);
> usbredir_log_data(dev, "bulk data out:", buf, size);
> usbredirparser_send_bulk_packet(dev->parser, p->id,
> @@ -923,7 +923,7 @@ static void
> usbredir_handle_interrupt_out_data(USBRedirDevice *dev,
> USBPacket *p, uint8_t ep)
> {
> struct usb_redir_interrupt_packet_header interrupt_packet;
> -uint8_t buf[p->iov.size];
> +g_autofree uint8_t *buf = g_malloc(p->iov.size);
>
> DPRINTF("interrupt-out ep %02X len %zd id %"PRIu64"\n", ep,
> p->iov.size, p->id);
> --
> 2.30.2
>
Nitpick: I would probably reference CVE-2021-3527 in patch 4/5 and 5/5
as well. Just to avoid someone from cherry-picking this patch only,
not actually fixing the root cause of the CVE.
Regards.
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[Bug 1914236] Re: QEMU: scsi: use-after-free in mptsas_process_scsi_io_request() of mptsas1068 emulator
Upstream commit: https://git.qemu.org/?p=qemu.git;a=commit;h=3791642c8d60029adf9b00bcb4e34d7d8a1aea4d ** Changed in: qemu Status: New => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1914236 Title: QEMU: scsi: use-after-free in mptsas_process_scsi_io_request() of mptsas1068 emulator Status in QEMU: Fix Committed Bug description: * Cheolwoo Myung of Seoul National University reported a use-after-free issue in the SCSI Megaraid emulator of the QEMU. * It occurs while handling mptsas_process_scsi_io_request(), as it does not check a list in s->pending. * This was found in version 5.2.0 (master) ==31872==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000107568 at pc 0x564514950c7c bp 0x7fff524ef4b0 sp 0x7fff524ef4a0 WRITE of size 8 at 0x60c000107568 thread T0 #0 0x564514950c7b in mptsas_process_scsi_io_request ../hw/scsi/mptsas.c:306 #1 0x564514950c7b in mptsas_fetch_request ../hw/scsi/mptsas.c:775 #2 0x564514950c7b in mptsas_fetch_requests ../hw/scsi/mptsas.c:790 #3 0x56451585c25d in aio_bh_poll ../util/async.c:164 #4 0x5645158d7e7d in aio_dispatch ../util/aio-posix.c:381 #5 0x56451585be2d in aio_ctx_dispatch ../util/async.c:306 #6 0x7f1cc8af4416 in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c416) #7 0x56451583f059 in glib_pollfds_poll ../util/main-loop.c:221 #8 0x56451583f059 in os_host_main_loop_wait ../util/main-loop.c:244 #9 0x56451583f059 in main_loop_wait ../util/main-loop.c:520 #10 0x56451536b181 in qemu_main_loop ../softmmu/vl.c:1537 #11 0x5645143ddd3d in main ../softmmu/main.c:50 #12 0x7f1cc2650b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96) #13 0x5645143eece9 in _start (/home/cwmyung/prj/hyfuzz/src/qemu-repro/build/qemu-system-i386+0x1d55ce9) 0x60c000107568 is located 104 bytes inside of 120-byte region [0x60c000107500,0x60c000107578) freed by thread T0 here: #0 0x7f1cca9777a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8) #1 0x56451495008b in mptsas_process_scsi_io_request ../hw/scsi/mptsas.c:358 #2 0x56451495008b in mptsas_fetch_request ../hw/scsi/mptsas.c:775 #3 0x56451495008b in mptsas_fetch_requests ../hw/scsi/mptsas.c:790 #4 0x7fff524ef8bf () previously allocated by thread T0 here: #0 0x7f1cca977d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28) #1 0x7f1cc8af9b10 in g_malloc0 (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51b10) #2 0x7fff524ef8bf () SUMMARY: AddressSanitizer: heap-use-after-free ../hw/scsi/mptsas.c:306 in mptsas_process_scsi_io_request Shadow bytes around the buggy address: 0x0c1880018e50: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x0c1880018e60: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa 0x0c1880018e70: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c1880018e80: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x0c1880018e90: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa =>0x0c1880018ea0: fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fa 0x0c1880018eb0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 0x0c1880018ec0: 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa 0x0c1880018ed0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1880018ee0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1880018ef0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==31872==ABORTING To reproduce this issue, please run the QEMU with the following command line. # To enable ASan option, please set configuration with the following command $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers $ make # To reproduce this issue, please run the QEMU process with the following command line. $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw -device mptsas1068,id=scsi -device scsi-hd,drive=SysDisk -drive id=SysDisk,if=none,file=./disk.img To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1914236/+subscriptions
[Bug 1919036] Re: Assertion failure in fifo8_push_all() through am53c974
I'm not able to change the status of this bug anymore. It should have been closed as "Fix committed" - QEMU 6.0.0 is not yet released. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1919036 Title: Assertion failure in fifo8_push_all() through am53c974 Status in QEMU: Fix Released Bug description: Hello, Using hypervisor fuzzer, hyfuzz, I found an assertion failure through am53c974 emulator. A malicious guest user/process could use this flaw to abort the QEMU process on the host, resulting in a denial of service. This was found in version 5.2.0 (master, 3f8d1885e4) ``` qemu-system-i386: ../util/fifo8.c:43: fifo8_push_all: Assertion `fifo->num + num <= fifo->capacity' failed. #0 0x70218fb7 in __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x7021a921 in __GI_abort () at abort.c:79 #2 0x7020a48a in __assert_fail_base (fmt=0x70391750 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x58ed2400 "fifo->num + num <= fifo->capacity", file=file@entry=0x58ed2380 "../util/fifo8.c", line=line@entry=0x2b, function=function@entry=0x58ed2560 <__PRETTY_FUNCTION__.16583> "fifo8_push_all") at assert.c:92 #3 0x7020a502 in __GI___assert_fail (assertion=assertion@entry=0x58ed2400 "fifo->num + num <= fifo->capacity", file=file@entry=0x58ed2380 "../util/fifo8.c", line=line@entry=0x2b, function=function@entry=0x58ed2560 <__PRETTY_FUNCTION__.16583> "fifo8_push_all") at assert.c:101 #4 0x587749c4 in fifo8_push_all (fifo=fifo@entry=0x61f05200, data=data@entry=0x7fff72bfa640 "", num=num@entry=0x24) at ../util/fifo8.c:43 #5 0x572bd13e in esp_do_dma (s=s@entry=0x61f05088) at ../hw/scsi/esp.c:577 #6 0x572bfc8f in handle_ti (s=0x61f05088) at ../hw/scsi/esp.c:845 #7 0x572c419c in esp_reg_write (s=0x61f05088, saddr=saddr@entry=0x3, val=) at ../hw/scsi/esp.c:987 #8 0x57bb916a in esp_pci_io_write (opaque=0x61f04680, addr=, val=, size=) at ../hw/scsi/esp-pci.c:214 #9 0x5817ea28 in memory_region_write_accessor (mr=0x61f04f70, addr=, value=, size=, shift=, mask=, attrs=...) at ../softmmu/memory.c:491 #10 0x58176671 in access_with_adjusted_size (addr=addr@entry=0xc, value=value@entry=0x7fff72bfb2a8, size=size@entry=0x1, access_size_min=, access_size_max=, access_fn= 0x5817e7c0 , mr=0x61f04f70, attrs=...) at ../softmmu/memory.c:552 #11 0x581892aa in memory_region_dispatch_write (mr=mr@entry=0x61f04f70, addr=, data=, data@entry=0xff90, op=op@entry=MO_8, attrs=..., attrs@entry=...) at ../softmmu/memory.c:1508 #12 0x58024b66 in address_space_stb (as=, addr=, val=, attrs=..., result=0x0) at /home/cwmyung/prj/hyfuzz/src/qemu-master/memory_ldst.c.inc:382 #13 0x7fff9323641c in code_gen_buffer () #14 0x57e793bb in cpu_tb_exec (tb_exit=, itb=, cpu=0x62e004b4) at ../accel/tcg/cpu-exec.c:190 #15 0x57e793bb in cpu_loop_exec_tb (tb_exit=, last_tb=, tb=, cpu=0x62e004b4) at ../accel/tcg/cpu-exec.c:673 #16 0x57e793bb in cpu_exec (cpu=cpu@entry=0x62e00400) at ../accel/tcg/cpu-exec.c:798 #17 0x57f5fc5a in tcg_cpus_exec (cpu=cpu@entry=0x62e00400) at ../accel/tcg/tcg-accel-ops.c:68 #18 0x582260af in mttcg_cpu_thread_fn (arg=arg@entry=0x62e00400) at ../accel/tcg/tcg-accel-ops-mttcg.c:70 #19 0x58777b05 in qemu_thread_start (args=) at ../util/qemu-thread-posix.c:521 #20 0x705d26db in start_thread (arg=0x7fff72bff700) at pthread_create.c:463 #21 0x702fb71f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 ``` To reproduce the assertion failure, please run the QEMU with the following command line. ``` $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw -device am53c974,id=scsi -device scsi-hd,drive=SysDisk -drive id=SysDisk,if=none,file=./disk.img ``` Please let me know if I can provide any further info. Thank you. - Cheolwoo, Myung (Seoul National University) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1919036/+subscriptions
[Bug 1919035] Re: Assertion failure in fifo8_pop_buf() through am53c974
I'm not able to change the status of this bug anymore. It should have been closed as "Fix committed" - QEMU 6.0.0 is not yet released. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1919035 Title: Assertion failure in fifo8_pop_buf() through am53c974 Status in QEMU: Fix Released Bug description: Hello, Using hypervisor fuzzer, hyfuzz, I found an assertion failure through am53c974 emulator. A malicious guest user/process could use this flaw to abort the QEMU process on the host, resulting in a denial of service. This was found in version 5.2.0 (master, 3f8d1885e4) ``` qemu-system-i386: ../util/fifo8.c:73: fifo8_pop_buf: Assertion `max > 0 && max <= fifo->num' failed. #0 0x70218fb7 in __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x7021a921 in __GI_abort () at abort.c:79 #2 0x7020a48a in __assert_fail_base (fmt=0x70391750 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x58ed24a0 "max > 0 && max <= fifo->num", file=file@entry=0x58ed2380 "../util/fifo8.c", line=line@entry=0x49, function=function@entry=0x58ed24e0 <__PRETTY_FUNCTION__.16603> "fifo8_pop_buf") at assert.c:92 #3 0x7020a502 in __GI___assert_fail (assertion=assertion@entry=0x58ed24a0 "max > 0 && max <= fifo->num", file=file@entry=0x58ed2380 "../util/fifo8.c", line=line@entry=0x49, function=function@entry=0x58ed24e0 <__PRETTY_FUNCTION__.16603> "fifo8_pop_buf") at assert.c:101 #4 0x5877519a in fifo8_pop_buf (fifo=fifo@entry=0x61f05200, max=max@entry=0xff, num=num@entry=0x7fff72bfa550) at ../util/fifo8.c:73 #5 0x572b7d9a in do_cmd (s=s@entry=0x61f05088) at ../hw/scsi/esp.c:328 #6 0x572b879a in esp_do_nodma (s=s@entry=0x61f05088) at ../hw/scsi/esp.c:701 #7 0x572bfd79 in handle_ti (s=0x61f05088) at ../hw/scsi/esp.c:848 #8 0x572c419c in esp_reg_write (s=0x61f05088, saddr=saddr@entry=0x3, val=) at ../hw/scsi/esp.c:987 #9 0x57bb916a in esp_pci_io_write (opaque=0x61f04680, addr=, val=, size=) at ../hw/scsi/esp-pci.c:214 #10 0x5817ea28 in memory_region_write_accessor (mr=0x61f04f70, addr=, value=, size=, shift=, mask=, attrs=...) at ../softmmu/memory.c:491 #11 0x58176671 in access_with_adjusted_size (addr=addr@entry=0xc, value=value@entry=0x7fff72bfb2a8, size=size@entry=0x1, access_size_min=, access_size_max=, access_fn= 0x5817e7c0 , mr=0x61f04f70, attrs=...) at ../softmmu/memory.c:552 #12 0x581892aa in memory_region_dispatch_write (mr=mr@entry=0x61f04f70, addr=, data=, data@entry=0x10, op=op@entry=MO_8, attrs=..., attrs@entry=...) at ../softmmu/memory.c:1508 #13 0x58024b66 in address_space_stb (as=, addr=, val=, attrs=..., result=0x0) at /home/cwmyung/prj/hyfuzz/src/qemu-master/memory_ldst.c.inc:382 #14 0x7fff93236d3c in code_gen_buffer () #15 0x57e793bb in cpu_tb_exec (tb_exit=, itb=, cpu=0x62e004b4) at ../accel/tcg/cpu-exec.c:190 #16 0x57e793bb in cpu_loop_exec_tb (tb_exit=, last_tb=, tb=, cpu=0x62e004b4) at ../accel/tcg/cpu-exec.c:673 #17 0x57e793bb in cpu_exec (cpu=cpu@entry=0x62e00400) at ../accel/tcg/cpu-exec.c:798 #18 0x57f5fc5a in tcg_cpus_exec (cpu=cpu@entry=0x62e00400) at ../accel/tcg/tcg-accel-ops.c:68 #19 0x582260af in mttcg_cpu_thread_fn (arg=arg@entry=0x62e00400) at ../accel/tcg/tcg-accel-ops-mttcg.c:70 #20 0x58777b05 in qemu_thread_start (args=) at ../util/qemu-thread-posix.c:521 #21 0x705d26db in start_thread (arg=0x7fff72bff700) at pthread_create.c:463 #22 0x702fb71f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 ``` To reproduce the assertion failure, please run the QEMU with the following command line. ``` $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw -device am53c974,id=scsi -device scsi-hd,drive=SysDisk -drive id=SysDisk,if=none,file=./disk.img ``` Please let me know if I can provide any further info. Thank you. - Cheolwoo, Myung (Seoul National University) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1919035/+subscriptions
[Bug 1910723] Re: NULL pointer dereference issues in am53c974 SCSI host bus adapter
** Changed in: qemu Status: Fix Released => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1910723 Title: NULL pointer dereference issues in am53c974 SCSI host bus adapter Status in QEMU: Fix Committed Bug description: Two NULL pointer dereference issues were found in the am53c974 SCSI host bus adapter emulation of QEMU. They could occur while handling the 'Information Transfer' command (CMD_TI) in function handle_ti() in hw/scsi/esp.c, and could be abused by a malicious guest to crash the QEMU process on the host resulting in a denial of service. Both issues were reported by Cheolwoo Myung (Seoul National University). To reproduce them, configure and run QEMU as follows. Please find attached the required disk images. $ ./configure --target-list=x86_64-softmmu --enable-kvm --enable-sanitizers $ make $ ./qemu-system-x86_64 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Additional info: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909766 RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909769 ASAN logs: ==672133== hw/scsi/scsi-bus.c:1385:12: runtime error: member access within null pointer of type 'struct SCSIRequest' AddressSanitizer:DEADLYSIGNAL = ==672133==ERROR: AddressSanitizer: SEGV on unknown address 0x0171 (pc 0x55bd63e20b85 bp 0x7f4b6fffdfa0 sp 0x7f4b6fffdf70 T7) ==672133==The signal is caused by a READ memory access. ==672133==Hint: address points to the zero page. #0 0x55bd63e20b85 in scsi_req_continue hw/scsi/scsi-bus.c:1385 #1 0x55bd63ab34fb in esp_do_dma hw/scsi/esp.c:453 #2 0x55bd63ab4b3c in handle_ti hw/scsi/esp.c:549 #3 0x55bd63ab72a9 in esp_reg_write hw/scsi/esp.c:691 #4 0x55bd63d7b5dd in esp_pci_io_write hw/scsi/esp-pci.c:206 #5 0x55bd645d55a3 in memory_region_write_accessor softmmu/memory.c:491 #6 0x55bd645d5a24 in access_with_adjusted_size softmmu/memory.c:552 #7 0x55bd645e2baa in memory_region_dispatch_write softmmu/memory.c:1501 #8 0x55bd646b75ff in flatview_write_continue softmmu/physmem.c:2759 #9 0x55bd646b79d1 in flatview_write softmmu/physmem.c:2799 #10 0x55bd646b8341 in address_space_write softmmu/physmem.c:2891 #11 0x55bd646b83f9 in address_space_rw softmmu/physmem.c:2901 #12 0x55bd648c4736 in kvm_handle_io accel/kvm/kvm-all.c:2285 #13 0x55bd648c69c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531 #14 0x55bd647b2413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49 #15 0x55bd64f560de in qemu_thread_start util/qemu-thread-posix.c:521 #16 0x7f4b981763f8 in start_thread (/lib64/libpthread.so.0+0x93f8) #17 0x7f4b980a3902 in __GI___clone (/lib64/libc.so.6+0x101902) --- ==672020== hw/scsi/esp.c:196:62: runtime error: member access within null pointer of type 'struct SCSIDevice' AddressSanitizer:DEADLYSIGNAL = ==672020==ERROR: AddressSanitizer: SEGV on unknown address 0x0098 (pc 0x559bc99946fd bp 0x7f08bd737fb0 sp 0x7f08bd737f70 T7) ==672020==The signal is caused by a READ memory access. ==672020==Hint: address points to the zero page. #0 0x559bc99946fd in do_busid_cmd hw/scsi/esp.c:196 #1 0x559bc9994e71 in do_cmd hw/scsi/esp.c:220 #2 0x559bc999ae81 in handle_ti hw/scsi/esp.c:555 #3 0x559bc999d2a9 in esp_reg_write hw/scsi/esp.c:691 #4 0x559bc9c615dd in esp_pci_io_write hw/scsi/esp-pci.c:206 #5 0x559bca4bb5a3 in memory_region_write_accessor softmmu/memory.c:491 #6 0x559bca4bba24 in access_with_adjusted_size softmmu/memory.c:552 #7 0x559bca4c8baa in memory_region_dispatch_write softmmu/memory.c:1501 #8 0x559bca59d5ff in flatview_write_continue softmmu/physmem.c:2759 #9 0x559bca59d9d1 in flatview_write softmmu/physmem.c:2799 #10 0x559bca59e341 in address_space_write softmmu/physmem.c:2891 #11 0x559bca59e3f9 in address_space_rw softmmu/physmem.c:2901 #12 0x559bca7aa736 in kvm_handle_io accel/kvm/kvm-all.c:2285 #13 0x559bca7ac9c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531 #14 0x559bca698413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49 #15 0x559bcae3c0de in qemu_thread_start util/qemu-thread-posix.c:521 #16 0x7f08e57ba3f8 in
[Bug 1909247] Re: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c
** Changed in: qemu Status: Fix Released => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1909247 Title: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c Status in QEMU: Fix Committed Bug description: A use-after-free vulnerability was found in the am53c974 SCSI host bus adapter emulation of QEMU. It could occur in the esp_do_dma() function in hw/scsi/esp.c while handling the 'Information Transfer' command (CMD_TI). A privileged guest user may abuse this flaw to crash the QEMU process on the host, resulting in a denial of service or potential code execution with the privileges of the QEMU process. This issue was reported by Cheolwoo Myung (Seoul National University). Original report: Using hypervisor fuzzer, hyfuzz, I found a use-after-free issue in am53c974 emulator of QEMU enabled ASan. It occurs while transferring information, as it does not check the buffer to be transferred. A malicious guest user/process could use this flaw to crash the QEMU process resulting in DoS scenario. To reproduce this issue, please run the QEMU with the following command line. # To enable ASan option, please set configuration with the following $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers $ make # To reproduce this issue, please run the QEMU process with the following command line $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Please find attached the disk images to reproduce this issue. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1909247/+subscriptions
[Bug 1909247] Re: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c
This is fixed now, thank you Mark. Patchset v4: https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01000.html Upstream commits: https://git.qemu.org/?p=qemu.git;a=commit;h=0db895361b8a82e1114372ff9f48 https://git.qemu.org/?p=qemu.git;a=commit;h=e392255766071c8cac480da3a9ae https://git.qemu.org/?p=qemu.git;a=commit;h=e5455b8c1c6170c788f3c0fd577c https://git.qemu.org/?p=qemu.git;a=commit;h=c5fef9112b15c4b5494791cdf8bb https://git.qemu.org/?p=qemu.git;a=commit;h=7b320a8e67a534925048cbabfa51 https://git.qemu.org/?p=qemu.git;a=commit;h=99545751734035b76bd372c4e721 https://git.qemu.org/?p=qemu.git;a=commit;h=fa7505c154d4d00ad89a747be2ed https://git.qemu.org/?p=qemu.git;a=commit;h=fbc6510e3379fa8f8370bf71198f https://git.qemu.org/?p=qemu.git;a=commit;h=0ebb5fd80589835153a0c2baa1b8 https://git.qemu.org/?p=qemu.git;a=commit;h=324c8809897c8c53ad05c3a7147d https://git.qemu.org/?p=qemu.git;a=commit;h=607206948cacda4a80be5b976dba ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1909247 Title: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c Status in QEMU: Fix Released Bug description: A use-after-free vulnerability was found in the am53c974 SCSI host bus adapter emulation of QEMU. It could occur in the esp_do_dma() function in hw/scsi/esp.c while handling the 'Information Transfer' command (CMD_TI). A privileged guest user may abuse this flaw to crash the QEMU process on the host, resulting in a denial of service or potential code execution with the privileges of the QEMU process. This issue was reported by Cheolwoo Myung (Seoul National University). Original report: Using hypervisor fuzzer, hyfuzz, I found a use-after-free issue in am53c974 emulator of QEMU enabled ASan. It occurs while transferring information, as it does not check the buffer to be transferred. A malicious guest user/process could use this flaw to crash the QEMU process resulting in DoS scenario. To reproduce this issue, please run the QEMU with the following command line. # To enable ASan option, please set configuration with the following $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers $ make # To reproduce this issue, please run the QEMU process with the following command line $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Please find attached the disk images to reproduce this issue. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1909247/+subscriptions
[Bug 1919035] Re: Assertion failure in fifo8_pop_buf() through am53c974
This is fixed now, thank you Mark. Patchset v4: https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01000.html Upstream commits: https://git.qemu.org/?p=qemu.git;a=commit;h=0db895361b8a82e1114372ff9f48 https://git.qemu.org/?p=qemu.git;a=commit;h=e392255766071c8cac480da3a9ae https://git.qemu.org/?p=qemu.git;a=commit;h=e5455b8c1c6170c788f3c0fd577c https://git.qemu.org/?p=qemu.git;a=commit;h=c5fef9112b15c4b5494791cdf8bb https://git.qemu.org/?p=qemu.git;a=commit;h=7b320a8e67a534925048cbabfa51 https://git.qemu.org/?p=qemu.git;a=commit;h=99545751734035b76bd372c4e721 https://git.qemu.org/?p=qemu.git;a=commit;h=fa7505c154d4d00ad89a747be2ed https://git.qemu.org/?p=qemu.git;a=commit;h=fbc6510e3379fa8f8370bf71198f https://git.qemu.org/?p=qemu.git;a=commit;h=0ebb5fd80589835153a0c2baa1b8 https://git.qemu.org/?p=qemu.git;a=commit;h=324c8809897c8c53ad05c3a7147d https://git.qemu.org/?p=qemu.git;a=commit;h=607206948cacda4a80be5b976dba ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1919035 Title: Assertion failure in fifo8_pop_buf() through am53c974 Status in QEMU: Fix Released Bug description: Hello, Using hypervisor fuzzer, hyfuzz, I found an assertion failure through am53c974 emulator. A malicious guest user/process could use this flaw to abort the QEMU process on the host, resulting in a denial of service. This was found in version 5.2.0 (master, 3f8d1885e4) ``` qemu-system-i386: ../util/fifo8.c:73: fifo8_pop_buf: Assertion `max > 0 && max <= fifo->num' failed. #0 0x70218fb7 in __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x7021a921 in __GI_abort () at abort.c:79 #2 0x7020a48a in __assert_fail_base (fmt=0x70391750 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x58ed24a0 "max > 0 && max <= fifo->num", file=file@entry=0x58ed2380 "../util/fifo8.c", line=line@entry=0x49, function=function@entry=0x58ed24e0 <__PRETTY_FUNCTION__.16603> "fifo8_pop_buf") at assert.c:92 #3 0x7020a502 in __GI___assert_fail (assertion=assertion@entry=0x58ed24a0 "max > 0 && max <= fifo->num", file=file@entry=0x58ed2380 "../util/fifo8.c", line=line@entry=0x49, function=function@entry=0x58ed24e0 <__PRETTY_FUNCTION__.16603> "fifo8_pop_buf") at assert.c:101 #4 0x5877519a in fifo8_pop_buf (fifo=fifo@entry=0x61f05200, max=max@entry=0xff, num=num@entry=0x7fff72bfa550) at ../util/fifo8.c:73 #5 0x572b7d9a in do_cmd (s=s@entry=0x61f05088) at ../hw/scsi/esp.c:328 #6 0x572b879a in esp_do_nodma (s=s@entry=0x61f05088) at ../hw/scsi/esp.c:701 #7 0x572bfd79 in handle_ti (s=0x61f05088) at ../hw/scsi/esp.c:848 #8 0x572c419c in esp_reg_write (s=0x61f05088, saddr=saddr@entry=0x3, val=) at ../hw/scsi/esp.c:987 #9 0x57bb916a in esp_pci_io_write (opaque=0x61f04680, addr=, val=, size=) at ../hw/scsi/esp-pci.c:214 #10 0x5817ea28 in memory_region_write_accessor (mr=0x61f04f70, addr=, value=, size=, shift=, mask=, attrs=...) at ../softmmu/memory.c:491 #11 0x58176671 in access_with_adjusted_size (addr=addr@entry=0xc, value=value@entry=0x7fff72bfb2a8, size=size@entry=0x1, access_size_min=, access_size_max=, access_fn= 0x5817e7c0 , mr=0x61f04f70, attrs=...) at ../softmmu/memory.c:552 #12 0x581892aa in memory_region_dispatch_write (mr=mr@entry=0x61f04f70, addr=, data=, data@entry=0x10, op=op@entry=MO_8, attrs=..., attrs@entry=...) at ../softmmu/memory.c:1508 #13 0x58024b66 in address_space_stb (as=, addr=, val=, attrs=..., result=0x0) at /home/cwmyung/prj/hyfuzz/src/qemu-master/memory_ldst.c.inc:382 #14 0x7fff93236d3c in code_gen_buffer () #15 0x57e793bb in cpu_tb_exec (tb_exit=, itb=, cpu=0x62e004b4) at ../accel/tcg/cpu-exec.c:190 #16 0x57e793bb in cpu_loop_exec_tb (tb_exit=, last_tb=, tb=, cpu=0x62e004b4) at ../accel/tcg/cpu-exec.c:673 #17 0x57e793bb in cpu_exec (cpu=cpu@entry=0x62e00400) at ../accel/tcg/cpu-exec.c:798 #18 0x57f5fc5a in tcg_cpus_exec (cpu=cpu@entry=0x62e00400) at ../accel/tcg/tcg-accel-ops.c:68 #19 0x582260af in mttcg_cpu_thread_fn (arg=arg@entry=0x62e00400) at ../accel/tcg/tcg-accel-ops-mttcg.c:70 #20 0x58777b05 in qemu_thread_start (args=) at ../util/qemu-thread-posix.c:521 #21 0x705d26db in start_thread (arg=0x7fff72bff700) at pthread_create.c:463 #22 0x702fb71f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 ``` To reproduce the assertion failure, please run the QEMU with the following command line. ``` $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw -device am53c974,id=scsi -device
[Bug 1919036] Re: Assertion failure in fifo8_push_all() through am53c974
This is fixed now, thank you Mark. Patchset v4: https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01000.html Upstream commits: https://git.qemu.org/?p=qemu.git;a=commit;h=0db895361b8a82e1114372ff9f48 https://git.qemu.org/?p=qemu.git;a=commit;h=e392255766071c8cac480da3a9ae https://git.qemu.org/?p=qemu.git;a=commit;h=e5455b8c1c6170c788f3c0fd577c https://git.qemu.org/?p=qemu.git;a=commit;h=c5fef9112b15c4b5494791cdf8bb https://git.qemu.org/?p=qemu.git;a=commit;h=7b320a8e67a534925048cbabfa51 https://git.qemu.org/?p=qemu.git;a=commit;h=99545751734035b76bd372c4e721 https://git.qemu.org/?p=qemu.git;a=commit;h=fa7505c154d4d00ad89a747be2ed https://git.qemu.org/?p=qemu.git;a=commit;h=fbc6510e3379fa8f8370bf71198f https://git.qemu.org/?p=qemu.git;a=commit;h=0ebb5fd80589835153a0c2baa1b8 https://git.qemu.org/?p=qemu.git;a=commit;h=324c8809897c8c53ad05c3a7147d https://git.qemu.org/?p=qemu.git;a=commit;h=607206948cacda4a80be5b976dba ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1919036 Title: Assertion failure in fifo8_push_all() through am53c974 Status in QEMU: Fix Released Bug description: Hello, Using hypervisor fuzzer, hyfuzz, I found an assertion failure through am53c974 emulator. A malicious guest user/process could use this flaw to abort the QEMU process on the host, resulting in a denial of service. This was found in version 5.2.0 (master, 3f8d1885e4) ``` qemu-system-i386: ../util/fifo8.c:43: fifo8_push_all: Assertion `fifo->num + num <= fifo->capacity' failed. #0 0x70218fb7 in __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x7021a921 in __GI_abort () at abort.c:79 #2 0x7020a48a in __assert_fail_base (fmt=0x70391750 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x58ed2400 "fifo->num + num <= fifo->capacity", file=file@entry=0x58ed2380 "../util/fifo8.c", line=line@entry=0x2b, function=function@entry=0x58ed2560 <__PRETTY_FUNCTION__.16583> "fifo8_push_all") at assert.c:92 #3 0x7020a502 in __GI___assert_fail (assertion=assertion@entry=0x58ed2400 "fifo->num + num <= fifo->capacity", file=file@entry=0x58ed2380 "../util/fifo8.c", line=line@entry=0x2b, function=function@entry=0x58ed2560 <__PRETTY_FUNCTION__.16583> "fifo8_push_all") at assert.c:101 #4 0x587749c4 in fifo8_push_all (fifo=fifo@entry=0x61f05200, data=data@entry=0x7fff72bfa640 "", num=num@entry=0x24) at ../util/fifo8.c:43 #5 0x572bd13e in esp_do_dma (s=s@entry=0x61f05088) at ../hw/scsi/esp.c:577 #6 0x572bfc8f in handle_ti (s=0x61f05088) at ../hw/scsi/esp.c:845 #7 0x572c419c in esp_reg_write (s=0x61f05088, saddr=saddr@entry=0x3, val=) at ../hw/scsi/esp.c:987 #8 0x57bb916a in esp_pci_io_write (opaque=0x61f04680, addr=, val=, size=) at ../hw/scsi/esp-pci.c:214 #9 0x5817ea28 in memory_region_write_accessor (mr=0x61f04f70, addr=, value=, size=, shift=, mask=, attrs=...) at ../softmmu/memory.c:491 #10 0x58176671 in access_with_adjusted_size (addr=addr@entry=0xc, value=value@entry=0x7fff72bfb2a8, size=size@entry=0x1, access_size_min=, access_size_max=, access_fn= 0x5817e7c0 , mr=0x61f04f70, attrs=...) at ../softmmu/memory.c:552 #11 0x581892aa in memory_region_dispatch_write (mr=mr@entry=0x61f04f70, addr=, data=, data@entry=0xff90, op=op@entry=MO_8, attrs=..., attrs@entry=...) at ../softmmu/memory.c:1508 #12 0x58024b66 in address_space_stb (as=, addr=, val=, attrs=..., result=0x0) at /home/cwmyung/prj/hyfuzz/src/qemu-master/memory_ldst.c.inc:382 #13 0x7fff9323641c in code_gen_buffer () #14 0x57e793bb in cpu_tb_exec (tb_exit=, itb=, cpu=0x62e004b4) at ../accel/tcg/cpu-exec.c:190 #15 0x57e793bb in cpu_loop_exec_tb (tb_exit=, last_tb=, tb=, cpu=0x62e004b4) at ../accel/tcg/cpu-exec.c:673 #16 0x57e793bb in cpu_exec (cpu=cpu@entry=0x62e00400) at ../accel/tcg/cpu-exec.c:798 #17 0x57f5fc5a in tcg_cpus_exec (cpu=cpu@entry=0x62e00400) at ../accel/tcg/tcg-accel-ops.c:68 #18 0x582260af in mttcg_cpu_thread_fn (arg=arg@entry=0x62e00400) at ../accel/tcg/tcg-accel-ops-mttcg.c:70 #19 0x58777b05 in qemu_thread_start (args=) at ../util/qemu-thread-posix.c:521 #20 0x705d26db in start_thread (arg=0x7fff72bff700) at pthread_create.c:463 #21 0x702fb71f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 ``` To reproduce the assertion failure, please run the QEMU with the following command line. ``` $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw -device am53c974,id=scsi -device scsi-hd,drive=SysDisk -drive
[Bug 1910723] Re: NULL pointer dereference issues in am53c974 SCSI host bus adapter
Patchset v4: https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01000.html Upstream commits: https://git.qemu.org/?p=qemu.git;a=commit;h=0db895361b8a82e1114372ff9f4857abea605701 https://git.qemu.org/?p=qemu.git;a=commit;h=e392255766071c8cac480da3a9ae4f94e56d7cbc https://git.qemu.org/?p=qemu.git;a=commit;h=e5455b8c1c6170c788f3c0fd577cc3be53539a99 https://git.qemu.org/?p=qemu.git;a=commit;h=c5fef9112b15c4b5494791cdf8bbb40bc1938dd3 https://git.qemu.org/?p=qemu.git;a=commit;h=7b320a8e67a534925048cbabfa51431e0349dafd https://git.qemu.org/?p=qemu.git;a=commit;h=99545751734035b76bd372c4e7215bb337428d89 https://git.qemu.org/?p=qemu.git;a=commit;h=fa7505c154d4d00ad89a747be2eda556643ce00e https://git.qemu.org/?p=qemu.git;a=commit;h=fbc6510e3379fa8f8370bf71198f0ce733bf07f9 https://git.qemu.org/?p=qemu.git;a=commit;h=0ebb5fd80589835153a0c2baa1b8cc7a04e67a93 https://git.qemu.org/?p=qemu.git;a=commit;h=324c8809897c8c53ad05c3a7147d272f1711cd5e https://git.qemu.org/?p=qemu.git;a=commit;h=607206948cacda4a80be5b976dba490970a18a76 ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1910723 Title: NULL pointer dereference issues in am53c974 SCSI host bus adapter Status in QEMU: Fix Released Bug description: Two NULL pointer dereference issues were found in the am53c974 SCSI host bus adapter emulation of QEMU. They could occur while handling the 'Information Transfer' command (CMD_TI) in function handle_ti() in hw/scsi/esp.c, and could be abused by a malicious guest to crash the QEMU process on the host resulting in a denial of service. Both issues were reported by Cheolwoo Myung (Seoul National University). To reproduce them, configure and run QEMU as follows. Please find attached the required disk images. $ ./configure --target-list=x86_64-softmmu --enable-kvm --enable-sanitizers $ make $ ./qemu-system-x86_64 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Additional info: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909766 RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909769 ASAN logs: ==672133== hw/scsi/scsi-bus.c:1385:12: runtime error: member access within null pointer of type 'struct SCSIRequest' AddressSanitizer:DEADLYSIGNAL = ==672133==ERROR: AddressSanitizer: SEGV on unknown address 0x0171 (pc 0x55bd63e20b85 bp 0x7f4b6fffdfa0 sp 0x7f4b6fffdf70 T7) ==672133==The signal is caused by a READ memory access. ==672133==Hint: address points to the zero page. #0 0x55bd63e20b85 in scsi_req_continue hw/scsi/scsi-bus.c:1385 #1 0x55bd63ab34fb in esp_do_dma hw/scsi/esp.c:453 #2 0x55bd63ab4b3c in handle_ti hw/scsi/esp.c:549 #3 0x55bd63ab72a9 in esp_reg_write hw/scsi/esp.c:691 #4 0x55bd63d7b5dd in esp_pci_io_write hw/scsi/esp-pci.c:206 #5 0x55bd645d55a3 in memory_region_write_accessor softmmu/memory.c:491 #6 0x55bd645d5a24 in access_with_adjusted_size softmmu/memory.c:552 #7 0x55bd645e2baa in memory_region_dispatch_write softmmu/memory.c:1501 #8 0x55bd646b75ff in flatview_write_continue softmmu/physmem.c:2759 #9 0x55bd646b79d1 in flatview_write softmmu/physmem.c:2799 #10 0x55bd646b8341 in address_space_write softmmu/physmem.c:2891 #11 0x55bd646b83f9 in address_space_rw softmmu/physmem.c:2901 #12 0x55bd648c4736 in kvm_handle_io accel/kvm/kvm-all.c:2285 #13 0x55bd648c69c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531 #14 0x55bd647b2413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49 #15 0x55bd64f560de in qemu_thread_start util/qemu-thread-posix.c:521 #16 0x7f4b981763f8 in start_thread (/lib64/libpthread.so.0+0x93f8) #17 0x7f4b980a3902 in __GI___clone (/lib64/libc.so.6+0x101902) --- ==672020== hw/scsi/esp.c:196:62: runtime error: member access within null pointer of type 'struct SCSIDevice' AddressSanitizer:DEADLYSIGNAL = ==672020==ERROR: AddressSanitizer: SEGV on unknown address 0x0098 (pc 0x559bc99946fd bp 0x7f08bd737fb0 sp 0x7f08bd737f70 T7) ==672020==The signal is caused by a READ memory access. ==672020==Hint: address points to the zero page. #0 0x559bc99946fd in do_busid_cmd hw/scsi/esp.c:196 #1 0x559bc9994e71 in do_cmd hw/scsi/esp.c:220
[Bug 1910723] Re: NULL pointer dereference issues in am53c974 SCSI host bus adapter
I can confirm this is fixed now, thank you Mark. Patchset v2: https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg06550.html -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1910723 Title: NULL pointer dereference issues in am53c974 SCSI host bus adapter Status in QEMU: New Bug description: Two NULL pointer dereference issues were found in the am53c974 SCSI host bus adapter emulation of QEMU. They could occur while handling the 'Information Transfer' command (CMD_TI) in function handle_ti() in hw/scsi/esp.c, and could be abused by a malicious guest to crash the QEMU process on the host resulting in a denial of service. Both issues were reported by Cheolwoo Myung (Seoul National University). To reproduce them, configure and run QEMU as follows. Please find attached the required disk images. $ ./configure --target-list=x86_64-softmmu --enable-kvm --enable-sanitizers $ make $ ./qemu-system-x86_64 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Additional info: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909766 RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909769 ASAN logs: ==672133== hw/scsi/scsi-bus.c:1385:12: runtime error: member access within null pointer of type 'struct SCSIRequest' AddressSanitizer:DEADLYSIGNAL = ==672133==ERROR: AddressSanitizer: SEGV on unknown address 0x0171 (pc 0x55bd63e20b85 bp 0x7f4b6fffdfa0 sp 0x7f4b6fffdf70 T7) ==672133==The signal is caused by a READ memory access. ==672133==Hint: address points to the zero page. #0 0x55bd63e20b85 in scsi_req_continue hw/scsi/scsi-bus.c:1385 #1 0x55bd63ab34fb in esp_do_dma hw/scsi/esp.c:453 #2 0x55bd63ab4b3c in handle_ti hw/scsi/esp.c:549 #3 0x55bd63ab72a9 in esp_reg_write hw/scsi/esp.c:691 #4 0x55bd63d7b5dd in esp_pci_io_write hw/scsi/esp-pci.c:206 #5 0x55bd645d55a3 in memory_region_write_accessor softmmu/memory.c:491 #6 0x55bd645d5a24 in access_with_adjusted_size softmmu/memory.c:552 #7 0x55bd645e2baa in memory_region_dispatch_write softmmu/memory.c:1501 #8 0x55bd646b75ff in flatview_write_continue softmmu/physmem.c:2759 #9 0x55bd646b79d1 in flatview_write softmmu/physmem.c:2799 #10 0x55bd646b8341 in address_space_write softmmu/physmem.c:2891 #11 0x55bd646b83f9 in address_space_rw softmmu/physmem.c:2901 #12 0x55bd648c4736 in kvm_handle_io accel/kvm/kvm-all.c:2285 #13 0x55bd648c69c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531 #14 0x55bd647b2413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49 #15 0x55bd64f560de in qemu_thread_start util/qemu-thread-posix.c:521 #16 0x7f4b981763f8 in start_thread (/lib64/libpthread.so.0+0x93f8) #17 0x7f4b980a3902 in __GI___clone (/lib64/libc.so.6+0x101902) --- ==672020== hw/scsi/esp.c:196:62: runtime error: member access within null pointer of type 'struct SCSIDevice' AddressSanitizer:DEADLYSIGNAL = ==672020==ERROR: AddressSanitizer: SEGV on unknown address 0x0098 (pc 0x559bc99946fd bp 0x7f08bd737fb0 sp 0x7f08bd737f70 T7) ==672020==The signal is caused by a READ memory access. ==672020==Hint: address points to the zero page. #0 0x559bc99946fd in do_busid_cmd hw/scsi/esp.c:196 #1 0x559bc9994e71 in do_cmd hw/scsi/esp.c:220 #2 0x559bc999ae81 in handle_ti hw/scsi/esp.c:555 #3 0x559bc999d2a9 in esp_reg_write hw/scsi/esp.c:691 #4 0x559bc9c615dd in esp_pci_io_write hw/scsi/esp-pci.c:206 #5 0x559bca4bb5a3 in memory_region_write_accessor softmmu/memory.c:491 #6 0x559bca4bba24 in access_with_adjusted_size softmmu/memory.c:552 #7 0x559bca4c8baa in memory_region_dispatch_write softmmu/memory.c:1501 #8 0x559bca59d5ff in flatview_write_continue softmmu/physmem.c:2759 #9 0x559bca59d9d1 in flatview_write softmmu/physmem.c:2799 #10 0x559bca59e341 in address_space_write softmmu/physmem.c:2891 #11 0x559bca59e3f9 in address_space_rw softmmu/physmem.c:2901 #12 0x559bca7aa736 in kvm_handle_io accel/kvm/kvm-all.c:2285 #13 0x559bca7ac9c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531 #14 0x559bca698413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49 #15 0x559bcae3c0de in qemu_thread_start
[Bug 1909247] Re: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c
Hello, Thank you all for your comments. Both patches (PJP/comment#8 - Mark/comment#9) seem to properly fix the UAF reported by Alexander in comment #6. However, I'm still able to reproduce the heap-bof from the above hw-esp-oob-issues.zip: ./x86_64-softmmu/qemu-system-x86_64 -m 512 \ -drive file=./atch2/hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./atch2/disk.img -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1909247 Title: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c Status in QEMU: New Bug description: A use-after-free vulnerability was found in the am53c974 SCSI host bus adapter emulation of QEMU. It could occur in the esp_do_dma() function in hw/scsi/esp.c while handling the 'Information Transfer' command (CMD_TI). A privileged guest user may abuse this flaw to crash the QEMU process on the host, resulting in a denial of service or potential code execution with the privileges of the QEMU process. This issue was reported by Cheolwoo Myung (Seoul National University). Original report: Using hypervisor fuzzer, hyfuzz, I found a use-after-free issue in am53c974 emulator of QEMU enabled ASan. It occurs while transferring information, as it does not check the buffer to be transferred. A malicious guest user/process could use this flaw to crash the QEMU process resulting in DoS scenario. To reproduce this issue, please run the QEMU with the following command line. # To enable ASan option, please set configuration with the following $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers $ make # To reproduce this issue, please run the QEMU process with the following command line $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Please find attached the disk images to reproduce this issue. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1909247/+subscriptions
[Bug 1910723] Re: NULL pointer dereference issues in am53c974 SCSI host bus adapter
** CVE added: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2020-35504 ** CVE added: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2020-35505 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1910723 Title: NULL pointer dereference issues in am53c974 SCSI host bus adapter Status in QEMU: New Bug description: Two NULL pointer dereference issues were found in the am53c974 SCSI host bus adapter emulation of QEMU. They could occur while handling the 'Information Transfer' command (CMD_TI) in function handle_ti() in hw/scsi/esp.c, and could be abused by a malicious guest to crash the QEMU process on the host resulting in a denial of service. Both issues were reported by Cheolwoo Myung (Seoul National University). To reproduce them, configure and run QEMU as follows. Please find attached the required disk images. $ ./configure --target-list=x86_64-softmmu --enable-kvm --enable-sanitizers $ make $ ./qemu-system-x86_64 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Additional info: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909766 RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909769 ASAN logs: ==672133== hw/scsi/scsi-bus.c:1385:12: runtime error: member access within null pointer of type 'struct SCSIRequest' AddressSanitizer:DEADLYSIGNAL = ==672133==ERROR: AddressSanitizer: SEGV on unknown address 0x0171 (pc 0x55bd63e20b85 bp 0x7f4b6fffdfa0 sp 0x7f4b6fffdf70 T7) ==672133==The signal is caused by a READ memory access. ==672133==Hint: address points to the zero page. #0 0x55bd63e20b85 in scsi_req_continue hw/scsi/scsi-bus.c:1385 #1 0x55bd63ab34fb in esp_do_dma hw/scsi/esp.c:453 #2 0x55bd63ab4b3c in handle_ti hw/scsi/esp.c:549 #3 0x55bd63ab72a9 in esp_reg_write hw/scsi/esp.c:691 #4 0x55bd63d7b5dd in esp_pci_io_write hw/scsi/esp-pci.c:206 #5 0x55bd645d55a3 in memory_region_write_accessor softmmu/memory.c:491 #6 0x55bd645d5a24 in access_with_adjusted_size softmmu/memory.c:552 #7 0x55bd645e2baa in memory_region_dispatch_write softmmu/memory.c:1501 #8 0x55bd646b75ff in flatview_write_continue softmmu/physmem.c:2759 #9 0x55bd646b79d1 in flatview_write softmmu/physmem.c:2799 #10 0x55bd646b8341 in address_space_write softmmu/physmem.c:2891 #11 0x55bd646b83f9 in address_space_rw softmmu/physmem.c:2901 #12 0x55bd648c4736 in kvm_handle_io accel/kvm/kvm-all.c:2285 #13 0x55bd648c69c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531 #14 0x55bd647b2413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49 #15 0x55bd64f560de in qemu_thread_start util/qemu-thread-posix.c:521 #16 0x7f4b981763f8 in start_thread (/lib64/libpthread.so.0+0x93f8) #17 0x7f4b980a3902 in __GI___clone (/lib64/libc.so.6+0x101902) --- ==672020== hw/scsi/esp.c:196:62: runtime error: member access within null pointer of type 'struct SCSIDevice' AddressSanitizer:DEADLYSIGNAL = ==672020==ERROR: AddressSanitizer: SEGV on unknown address 0x0098 (pc 0x559bc99946fd bp 0x7f08bd737fb0 sp 0x7f08bd737f70 T7) ==672020==The signal is caused by a READ memory access. ==672020==Hint: address points to the zero page. #0 0x559bc99946fd in do_busid_cmd hw/scsi/esp.c:196 #1 0x559bc9994e71 in do_cmd hw/scsi/esp.c:220 #2 0x559bc999ae81 in handle_ti hw/scsi/esp.c:555 #3 0x559bc999d2a9 in esp_reg_write hw/scsi/esp.c:691 #4 0x559bc9c615dd in esp_pci_io_write hw/scsi/esp-pci.c:206 #5 0x559bca4bb5a3 in memory_region_write_accessor softmmu/memory.c:491 #6 0x559bca4bba24 in access_with_adjusted_size softmmu/memory.c:552 #7 0x559bca4c8baa in memory_region_dispatch_write softmmu/memory.c:1501 #8 0x559bca59d5ff in flatview_write_continue softmmu/physmem.c:2759 #9 0x559bca59d9d1 in flatview_write softmmu/physmem.c:2799 #10 0x559bca59e341 in address_space_write softmmu/physmem.c:2891 #11 0x559bca59e3f9 in address_space_rw softmmu/physmem.c:2901 #12 0x559bca7aa736 in kvm_handle_io accel/kvm/kvm-all.c:2285 #13 0x559bca7ac9c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531 #14 0x559bca698413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49 #15 0x559bcae3c0de in
[Bug 1909247] Re: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c
** CVE added: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2020-35506 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1909247 Title: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c Status in QEMU: New Bug description: A use-after-free vulnerability was found in the am53c974 SCSI host bus adapter emulation of QEMU. It could occur in the esp_do_dma() function in hw/scsi/esp.c while handling the 'Information Transfer' command (CMD_TI). A privileged guest user may abuse this flaw to crash the QEMU process on the host, resulting in a denial of service or potential code execution with the privileges of the QEMU process. This issue was reported by Cheolwoo Myung (Seoul National University). Original report: Using hypervisor fuzzer, hyfuzz, I found a use-after-free issue in am53c974 emulator of QEMU enabled ASan. It occurs while transferring information, as it does not check the buffer to be transferred. A malicious guest user/process could use this flaw to crash the QEMU process resulting in DoS scenario. To reproduce this issue, please run the QEMU with the following command line. # To enable ASan option, please set configuration with the following $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers $ make # To reproduce this issue, please run the QEMU process with the following command line $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Please find attached the disk images to reproduce this issue. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1909247/+subscriptions
[Bug 1909247] Re: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c
Note that the use-after-free was found in v5.2.0 and, as far as I can tell, is not reproducible anymore on master. The ESP/NCR53C9x emulator (hw/scsi/esp.c) underwent several changes since v5.2.0. By git- bisecting, it looks like the original reproducer is neutralized after commit [1]. However, the qtest reproducer (comment #3) seems to be working fine on master as of today. [1] https://git.qemu.org/?p=qemu.git;a=commit;h=bb0bc7bbc9764a5e9e81756819838c5db88652b8 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1909247 Title: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c Status in QEMU: New Bug description: A use-after-free vulnerability was found in the am53c974 SCSI host bus adapter emulation of QEMU. It could occur in the esp_do_dma() function in hw/scsi/esp.c while handling the 'Information Transfer' command (CMD_TI). A privileged guest user may abuse this flaw to crash the QEMU process on the host, resulting in a denial of service or potential code execution with the privileges of the QEMU process. This issue was reported by Cheolwoo Myung (Seoul National University). Original report: Using hypervisor fuzzer, hyfuzz, I found a use-after-free issue in am53c974 emulator of QEMU enabled ASan. It occurs while transferring information, as it does not check the buffer to be transferred. A malicious guest user/process could use this flaw to crash the QEMU process resulting in DoS scenario. To reproduce this issue, please run the QEMU with the following command line. # To enable ASan option, please set configuration with the following $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers $ make # To reproduce this issue, please run the QEMU process with the following command line $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Please find attached the disk images to reproduce this issue. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1909247/+subscriptions
[Bug 1909247] Re: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c
Technically, the first one is a heap use-after-free, while the second a stack buffer overflow. They could be two different manifestations of the same issue; they both originate from handle_ti() and the root cause may be the same. Heap uaf: = ==129653==ERROR: AddressSanitizer: heap-use-after-free on address 0x629b5000 at pc 0x7f0c3d947dd3 bp 0x7f0c13bfdac0 sp 0x7f0c13bfd270 READ of size 27 at 0x629b5000 thread T7 #0 0x7f0c3d947dd2 in __interceptor_memcpy (/lib64/libasan.so.6+0x39dd2) #1 0x562c1c7292b2 in flatview_write_continue softmmu/physmem.c:2781 #2 0x562c1c729589 in flatview_write softmmu/physmem.c:2816 #3 0x562c1c729ef7 in address_space_write softmmu/physmem.c:2908 #4 0x562c1c729faf in address_space_rw softmmu/physmem.c:2918 #5 0x562c1c217754 in dma_memory_rw_relaxed include/sysemu/dma.h:8 #6 0x562c1c2177a1 in dma_memory_rw include/sysemu/dma.h:127 #7 0x562c1c21791b in pci_dma_rw include/hw/pci/pci.h:803 #8 0x562c1c21b6e3 in esp_pci_dma_memory_rw hw/scsi/esp-pci.c:283 #9 0x562c1c21ba6e in esp_pci_dma_memory_write hw/scsi/esp-pci.c:302 #10 0x562c1c428685 in esp_do_dma hw/scsi/esp.c:526 #11 0x562c1c429cb5 in handle_ti hw/scsi/esp.c:629 ... Stack bof: = ==138588==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc8a90c300 at pc 0x559b1de0780e bp 0x7ffc8a90bd10 sp 0x7ffc8a90bd08 WRITE of size 4 at 0x7ffc8a90c300 thread T0 #0 0x559b1de0780d in stl_he_p include/qemu/bswap.h:353 #1 0x559b1de07dec in stn_he_p include/qemu/bswap.h:486 #2 0x559b1de23e47 in flatview_read_continue softmmu/physmem.c:2841 #3 0x559b1de24215 in flatview_read softmmu/physmem.c:2879 #4 0x559b1de243b5 in address_space_read_full softmmu/physmem.c:2892 #5 0x559b1de2462c in address_space_rw softmmu/physmem.c:2920 #6 0x559b1d1ec514 in dma_memory_rw_relaxed include/sysemu/dma.h:88 #7 0x559b1d1ec561 in dma_memory_rw include/sysemu/dma.h:127 #8 0x559b1d1ec6db in pci_dma_rw include/hw/pci/pci.h:803 #9 0x559b1d1f04a3 in esp_pci_dma_memory_rw hw/scsi/esp-pci.c:283 #10 0x559b1d1f07f8 in esp_pci_dma_memory_read hw/scsi/esp-pci.c:296 #11 0x559b1d66fab1 in esp_do_dma hw/scsi/esp.c:576 #12 0x559b1d6746e1 in handle_ti hw/scsi/esp.c:845 ... -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1909247 Title: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c Status in QEMU: New Bug description: A use-after-free vulnerability was found in the am53c974 SCSI host bus adapter emulation of QEMU. It could occur in the esp_do_dma() function in hw/scsi/esp.c while handling the 'Information Transfer' command (CMD_TI). A privileged guest user may abuse this flaw to crash the QEMU process on the host, resulting in a denial of service or potential code execution with the privileges of the QEMU process. This issue was reported by Cheolwoo Myung (Seoul National University). Original report: Using hypervisor fuzzer, hyfuzz, I found a use-after-free issue in am53c974 emulator of QEMU enabled ASan. It occurs while transferring information, as it does not check the buffer to be transferred. A malicious guest user/process could use this flaw to crash the QEMU process resulting in DoS scenario. To reproduce this issue, please run the QEMU with the following command line. # To enable ASan option, please set configuration with the following $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers $ make # To reproduce this issue, please run the QEMU process with the following command line $ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Please find attached the disk images to reproduce this issue. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1909247/+subscriptions
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
On Thu, Feb 11, 2021 at 8:48 PM Philippe Mathieu-Daudé wrote: > > On 2/11/21 9:52 AM, Mauro Matteo Cascella wrote: > > Hello, > > > > On Wed, Feb 10, 2021 at 11:27 PM Alistair Francis > > wrote: > >> > >> On Tue, Feb 9, 2021 at 2:55 AM Bin Meng wrote: > >>> > >>> At the end of sdhci_send_command(), it starts a data transfer if > >>> the command register indicates a data is associated. However the > >>> data transfer should only be initiated when the command execution > >>> has succeeded. > >> > >> Isn't this already fixed? > > The previous patch was enough to catch the previous reproducer, > but something changed elsewhere making the same reproducer crash > QEMU again... > > > It turned out the bug was still reproducible on master. I'm actually > > thinking of assigning a new CVE for this, to make it possible for > > distros to apply this fix. > > It sounds fair. Do you have an ETA for the new CVE? This is now CVE-2021-3409. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1928146 -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
Hello, On Wed, Feb 10, 2021 at 11:27 PM Alistair Francis wrote: > > On Tue, Feb 9, 2021 at 2:55 AM Bin Meng wrote: > > > > At the end of sdhci_send_command(), it starts a data transfer if > > the command register indicates a data is associated. However the > > data transfer should only be initiated when the command execution > > has succeeded. > > > > Cc: qemu-sta...@nongnu.org > > Fixes: CVE-2020-17380 > > Fixes: CVE-2020-25085 > > Reported-by: Alexander Bulekov > > Reported-by: Sergej Schumilo (Ruhr-University Bochum) > > Reported-by: Cornelius Aschermann (Ruhr-University Bochum) > > Reported-by: Simon Wrner (Ruhr-University Bochum) > > Buglink: https://bugs.launchpad.net/qemu/+bug/1892960 > > Isn't this already fixed? > It turned out the bug was still reproducible on master. I'm actually thinking of assigning a new CVE for this, to make it possible for distros to apply this fix. -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] hw/sd/sdhci: Do not modify BlockSizeRegister if transaction in progress
On Mon, Feb 8, 2021 at 9:26 PM Philippe Mathieu-Daudé wrote: > > On Mon, Feb 8, 2021 at 8:59 PM Mauro Matteo Cascella > wrote: > > On Mon, Feb 8, 2021 at 8:35 PM Philippe Mathieu-Daudé > > wrote: > > > > > > Per the "SD Host Controller Simplified Specification Version 2.00" > > > spec. 'Table 2-4 : Block Size Register': > > > > > > Transfer Block Size [...] can be accessed only if no > > > transaction is executing (i.e., after a transaction has stopped). > > > Read operations during transfers may return an invalid value, > > > and write operations shall be ignored. > > > > ... > > > > > > Fixes: CVE-2020-17380 > > > Fixes: CVE-2020-25085 > > > Signed-off-by: Philippe Mathieu-Daudé > > > --- > > > Cc: Mauro Matteo Cascella > > > Cc: Alexander Bulekov > > > Cc: Alistair Francis > > > Cc: Prasad J Pandit > > > Cc: Bandan Das > > > > > > RFC because missing Reported-by tags, launchpad/bugzilla links and > > > qtest reproducer. Sending for review meanwhile. > ... > > For the above CVEs: > > Tested-by: Mauro Matteo Cascella > > Thanks Mauro for testing. Do you know what tags I should add for the credits? > > Phil. > I think the credit should go to Alexander for reporting [1] as well as people from Ruhr-University Bochum for CVE-2020-25085 (I don't know about their emails, though): Reported-by: Alexander Bulekov Reported-by: Sergej Schumilo (Ruhr-University Bochum) Reported-by: Cornelius Aschermann (Ruhr-University Bochum) Reported-by: Simon Wrner (Ruhr-University Bochum) [1] https://bugs.launchpad.net/qemu/+bug/1892960 -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] hw/sd/sdhci: Do not modify BlockSizeRegister if transaction in progress
On Mon, Feb 8, 2021 at 8:35 PM Philippe Mathieu-Daudé wrote: > > Per the "SD Host Controller Simplified Specification Version 2.00" > spec. 'Table 2-4 : Block Size Register': > > Transfer Block Size [...] can be accessed only if no > transaction is executing (i.e., after a transaction has stopped). > Read operations during transfers may return an invalid value, > and write operations shall be ignored. > > Transactions will update 'data_count', so do not modify 'blksize' > and 'blkcnt' when 'data_count' is used. This fixes: > > $ cat << EOF | qemu-system-x86_64 -qtest stdio -monitor none \ >-nographic -serial none -M pc-q35-5.0 \ >-device sdhci-pci,sd-spec-version=3 \ >-device sd-card,drive=mydrive \ >-drive if=sd,index=0,file=null-co://,format=raw,id=mydrive > outl 0xcf8 0x80001810 > outl 0xcfc 0xe1068000 > outl 0xcf8 0x80001814 > outl 0xcf8 0x80001804 > outw 0xcfc 0x7 > outl 0xcf8 0x8000fa20 > write 0xe106802c 0x1 0x0f > write 0xe1068004 0xc 0x2801d10101fbff28a384 > write 0xe106800c 0x1f > 0x9dacbbcad9e8f7061524334251606f7e8d9cabbac9d8e7f60514233241505f > write 0xe1068003 0x28 > 0x80d000251480d000252280d000253080d000253e80d000254c80d000255a80d000256880d0002576 > write 0xe1068003 0x1 0xfe > EOF > = > ==2686219==ERROR: AddressSanitizer: heap-buffer-overflow on address > 0x6153bb00 at pc 0x55ab469f456c bp 0x7ffee71be330 sp 0x7ffee71bdae0 > WRITE of size 4 at 0x6153bb00 thread T0 > #0 0x55ab469f456b in __asan_memcpy (qemu-system-i386+0x1cea56b) > #1 0x55ab483dc396 in stl_he_p include/qemu/bswap.h:353:5 > #2 0x55ab483af5e4 in stn_he_p include/qemu/bswap.h:546:1 > #3 0x55ab483aeb4b in flatview_read_continue softmmu/physmem.c:2839:13 > #4 0x55ab483b0705 in flatview_read softmmu/physmem.c:2877:12 > #5 0x55ab483b028e in address_space_read_full softmmu/physmem.c:2890:18 > #6 0x55ab483b1294 in address_space_rw softmmu/physmem.c:2918:16 > #7 0x55ab479374a2 in dma_memory_rw_relaxed include/sysemu/dma.h:88:12 > #8 0x55ab47936f50 in dma_memory_rw include/sysemu/dma.h:127:12 > #9 0x55ab4793665f in dma_memory_read include/sysemu/dma.h:145:12 > #10 0x55ab4792f176 in sdhci_sdma_transfer_multi_blocks > hw/sd/sdhci.c:639:13 > #11 0x55ab4793dc9d in sdhci_write hw/sd/sdhci.c:1129:17 > #12 0x55ab483f8db8 in memory_region_write_accessor > softmmu/memory.c:491:5 > #13 0x55ab483f868a in access_with_adjusted_size softmmu/memory.c:552:18 > #14 0x55ab483f6da5 in memory_region_dispatch_write > softmmu/memory.c:1501:16 > #15 0x55ab483c3b11 in flatview_write_continue softmmu/physmem.c:2774:23 > #16 0x55ab483b0eb6 in flatview_write softmmu/physmem.c:2814:14 > #17 0x55ab483b0a3e in address_space_write softmmu/physmem.c:2906:18 > #18 0x55ab48465c56 in qtest_process_command softmmu/qtest.c:654:9 > > 0x6153bb00 is located 0 bytes to the right of 512-byte region > [0x6153b900,0x6153bb00) > allocated by thread T0 here: > #0 0x55ab469f58a7 in calloc (qemu-system-i386+0x1ceb8a7) > #1 0x7f21d678f9b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x589b0) > #2 0x55ab479530ed in sdhci_pci_realize hw/sd/sdhci-pci.c:36:5 > #3 0x55ab476f102a in pci_qdev_realize hw/pci/pci.c:2108:9 > #4 0x55ab48baaad2 in device_set_realized hw/core/qdev.c:761:13 > > SUMMARY: AddressSanitizer: heap-buffer-overflow > (qemu-system-i386+0x1cea56b) in __asan_memcpy > Shadow bytes around the buggy address: > 0x0c2a7710: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > 0x0c2a7720: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c2a7730: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c2a7740: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c2a7750: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > =>0x0c2a7760:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > 0x0c2a7770: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd > 0x0c2a7780: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd > 0x0c2a7790: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd > 0x0c2a77a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd > 0x0c2a77b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > Shadow byte legend (one shadow byte represents 8 application bytes): > Addressable: 00 > Heap left redzone: fa > Freed heap region: fd > ==2686219==ABORTING > > Fixes: CVE-2020-17380 > Fixes: CVE-2020-25085 > Signed-off-by: Philippe Mathieu-Daudé > --- &g
Re: [PATCH] hw/scsi/megasas: check for NULL frame in megasas_command_cancelled()
On Mon, Jan 25, 2021 at 3:52 PM Philippe Mathieu-Daudé wrote: > > You forgot to Cc the subsystem maintainers... > > ./scripts/get_maintainer.pl -f hw/scsi/megasas.c > Hannes Reinecke (supporter:megasas) > Paolo Bonzini (supporter:SCSI) > Fam Zheng (reviewer:SCSI) I used to only check the MAINTAINERS file, which only mentions Hannes in connection with megasas. Good to know for the nex time. > As Paolo usually asks for reproducer to be integrated with the fix, > it might save him/you time if you respin with the reproducer. You > can have a look at > https://www.mail-archive.com/qemu-block@nongnu.org/msg78982.html > for example. Thank you for the heads up, Philippe. I'll try to incorporate the reproducer. > That said, unrelated to your patch but I'm not sure how useful it > is to test for bugs found by fuzzer each time in our CI. There are > borderline cases not representing proper use. Maybe we could run > them weekly instead... -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] hw/scsi/megasas: check for NULL frame in megasas_command_cancelled()
Hello,
Any updates on this little patch? Please find below a reproducer for
this bug (thanks Alexander):
https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg02567.html
Thank you,
On Thu, Dec 24, 2020 at 6:55 PM Mauro Matteo Cascella
wrote:
>
> Ensure that 'cmd->frame' is not NULL before accessing the 'header' field.
> This check prevents a potential NULL pointer dereference issue.
>
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1910346
> Signed-off-by: Mauro Matteo Cascella
> Reported-by: Cheolwoo Myung
> ---
> hw/scsi/megasas.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index 1a5fc5857d..77510e120c 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -1893,7 +1893,7 @@ static void megasas_command_cancelled(SCSIRequest *req)
> {
> MegasasCmd *cmd = req->hba_private;
>
> -if (!cmd) {
> +if (!cmd || !cmd->frame) {
> return;
> }
> cmd->frame->header.cmd_status = MFI_STAT_SCSI_IO_FAILED;
> --
> 2.29.2
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
[Bug 1910723] [NEW] NULL pointer dereference issues in am53c974 SCSI host bus adapter
Public bug reported: Two NULL pointer dereference issues were found in the am53c974 SCSI host bus adapter emulation of QEMU. They could occur while handling the 'Information Transfer' command (CMD_TI) in function handle_ti() in hw/scsi/esp.c, and could be abused by a malicious guest to crash the QEMU process on the host resulting in a denial of service. Both issues were reported by Cheolwoo Myung (Seoul National University). To reproduce them, configure and run QEMU as follows. Please find attached the required disk images. $ ./configure --target-list=x86_64-softmmu --enable-kvm --enable-sanitizers $ make $ ./qemu-system-x86_64 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \ -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \ -drive id=SysDisk,if=none,file=./disk.img Additional info: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909766 RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909769 ASAN logs: ==672133== hw/scsi/scsi-bus.c:1385:12: runtime error: member access within null pointer of type 'struct SCSIRequest' AddressSanitizer:DEADLYSIGNAL = ==672133==ERROR: AddressSanitizer: SEGV on unknown address 0x0171 (pc 0x55bd63e20b85 bp 0x7f4b6fffdfa0 sp 0x7f4b6fffdf70 T7) ==672133==The signal is caused by a READ memory access. ==672133==Hint: address points to the zero page. #0 0x55bd63e20b85 in scsi_req_continue hw/scsi/scsi-bus.c:1385 #1 0x55bd63ab34fb in esp_do_dma hw/scsi/esp.c:453 #2 0x55bd63ab4b3c in handle_ti hw/scsi/esp.c:549 #3 0x55bd63ab72a9 in esp_reg_write hw/scsi/esp.c:691 #4 0x55bd63d7b5dd in esp_pci_io_write hw/scsi/esp-pci.c:206 #5 0x55bd645d55a3 in memory_region_write_accessor softmmu/memory.c:491 #6 0x55bd645d5a24 in access_with_adjusted_size softmmu/memory.c:552 #7 0x55bd645e2baa in memory_region_dispatch_write softmmu/memory.c:1501 #8 0x55bd646b75ff in flatview_write_continue softmmu/physmem.c:2759 #9 0x55bd646b79d1 in flatview_write softmmu/physmem.c:2799 #10 0x55bd646b8341 in address_space_write softmmu/physmem.c:2891 #11 0x55bd646b83f9 in address_space_rw softmmu/physmem.c:2901 #12 0x55bd648c4736 in kvm_handle_io accel/kvm/kvm-all.c:2285 #13 0x55bd648c69c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531 #14 0x55bd647b2413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49 #15 0x55bd64f560de in qemu_thread_start util/qemu-thread-posix.c:521 #16 0x7f4b981763f8 in start_thread (/lib64/libpthread.so.0+0x93f8) #17 0x7f4b980a3902 in __GI___clone (/lib64/libc.so.6+0x101902) --- ==672020== hw/scsi/esp.c:196:62: runtime error: member access within null pointer of type 'struct SCSIDevice' AddressSanitizer:DEADLYSIGNAL = ==672020==ERROR: AddressSanitizer: SEGV on unknown address 0x0098 (pc 0x559bc99946fd bp 0x7f08bd737fb0 sp 0x7f08bd737f70 T7) ==672020==The signal is caused by a READ memory access. ==672020==Hint: address points to the zero page. #0 0x559bc99946fd in do_busid_cmd hw/scsi/esp.c:196 #1 0x559bc9994e71 in do_cmd hw/scsi/esp.c:220 #2 0x559bc999ae81 in handle_ti hw/scsi/esp.c:555 #3 0x559bc999d2a9 in esp_reg_write hw/scsi/esp.c:691 #4 0x559bc9c615dd in esp_pci_io_write hw/scsi/esp-pci.c:206 #5 0x559bca4bb5a3 in memory_region_write_accessor softmmu/memory.c:491 #6 0x559bca4bba24 in access_with_adjusted_size softmmu/memory.c:552 #7 0x559bca4c8baa in memory_region_dispatch_write softmmu/memory.c:1501 #8 0x559bca59d5ff in flatview_write_continue softmmu/physmem.c:2759 #9 0x559bca59d9d1 in flatview_write softmmu/physmem.c:2799 #10 0x559bca59e341 in address_space_write softmmu/physmem.c:2891 #11 0x559bca59e3f9 in address_space_rw softmmu/physmem.c:2901 #12 0x559bca7aa736 in kvm_handle_io accel/kvm/kvm-all.c:2285 #13 0x559bca7ac9c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531 #14 0x559bca698413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49 #15 0x559bcae3c0de in qemu_thread_start util/qemu-thread-posix.c:521 #16 0x7f08e57ba3f8 in start_thread (/lib64/libpthread.so.0+0x93f8) #17 0x7f08e56e7902 in __GI___clone (/lib64/libc.so.6+0x101902) ** Affects: qemu Importance: Undecided Status: New ** Tags: cve qemu security ** Attachment added: "null-derefs-am53c974.tar.gz" https://bugs.launchpad.net/bugs/1910723/+attachment/5450693/+files/null-derefs-am53c974.tar.gz -- You received this bug notification because you are a member of qemu- devel-ml, which is
