Adding Michael. The thread started here:

https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg00199.html

More below:

On 1/4/23 07:06, Laszlo Ersek wrote:
> On 1/3/23 18:42, Laszlo Ersek wrote:
>> (resending with qemu-devel on CC; sorry!)
>>
>> Hi,
>>
>> this is with QEMU-7.2.
>
> This is a regression. It works fine with QEMU-5.0. The regression has
> not been fixed since QEMU-7.2, as of master @ 222059a0fccf ("Merge tag
> 'pull-ppc-20221221' of https://gitlab.com/danielhb/qemu into staging",
> 2022-12-21).
>
> I'm bisecting.
>
> (It's a relief that this is a regression. I felt pretty certain that I
> had tested CPU hotplug with TCG as well!
>
> I've picked QEMU-5.0 as the start candidate for my bisection for the
> following reason: per git-blame, Igor described the modern interface
> detection steps in commit ae340aa3d2567 (which I reviewed), and the
> first tag/release to contain that commit was QEMU-5.0. The first QEMU
> release after Igor and I had worked on this in QEMU and OVMF
> definitely worked with TCG too, by my account.)

See the bisection log attached.

The first bad commit is:

> commit 5d971f9e672507210e77d020d89e0e89165c8fc9
> Author: Michael S. Tsirkin <m...@redhat.com>
> Date:   Wed Jun 10 09:47:49 2020 -0400
>
>     memory: Revert "memory: accept mismatching sizes in 
> memory_region_access_valid"
>
>     Memory API documentation documents valid .min_access_size and 
> .max_access_size
>     fields and explains that any access outside these boundaries is blocked.
>
>     This is what devices seem to assume.
>
>     However this is not what the implementation does: it simply
>     ignores the boundaries unless there's an "accepts" callback.
>
>     Naturally, this breaks a bunch of devices.
>
>     Revert to the documented behaviour.
>
>     Devices that want to allow any access can just drop the valid field,
>     or add the impl field to have accesses converted to appropriate
>     length.
>
>     Cc: qemu-sta...@nongnu.org
>     Reviewed-by: Richard Henderson <r...@twiddle.net>
>     Fixes: CVE-2020-13754
>     Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1842363
>     Fixes: a014ed07bd5a ("memory: accept mismatching sizes in 
> memory_region_access_valid")
>     Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
>     Message-Id: <20200610134731.1514409-1-...@redhat.com>
>     Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>

First released in QEMU v5.1.0 and v6.0.0 (exact same commit hash in
both, as v6.0.0 descends of v5.1.0).

Because this is a CVE fix, and also because of the suggestions made in
the commit message, I think the commit is actually right, and what we
need to fix is the hotplug register block -- namely the
"AcpiCpuHotplug_ops" structure in "hw/acpi/cpu_hotplug.c".

However, what I *really* don't understand is why this commit
(5d971f9e672507210e77d020d89e0e89165c8fc9) makes a difference *only*
under TCG.

Laszlo
git bisect start
# bad: [222059a0fccf4af3be776fe35a5ea2d6a68f9a0b] Merge tag 'pull-ppc-20221221' of https://gitlab.com/danielhb/qemu into staging
git bisect bad 222059a0fccf4af3be776fe35a5ea2d6a68f9a0b
# good: [fdd76fecdde1ad444ff4deb7f1c4f7e4a1ef97d6] Update version for v5.0.0 release
git bisect good fdd76fecdde1ad444ff4deb7f1c4f7e4a1ef97d6
# bad: [609d7596524ab204ccd71ef42c9eee4c7c338ea4] Update version for v6.0.0 release
git bisect bad 609d7596524ab204ccd71ef42c9eee4c7c338ea4
# bad: [a0bdf866873467271eff9a92f179ab0f77d735cb] Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20201012a' into staging
git bisect bad a0bdf866873467271eff9a92f179ab0f77d735cb
# bad: [3a9163af4e3dd61795a35d47b702e302f98f81d6] Merge remote-tracking branch 'remotes/philmd-gitlab/tags/sdcard-CVE-2020-13253-pull-request' into staging
git bisect bad 3a9163af4e3dd61795a35d47b702e302f98f81d6
# good: [7d3660e79830a069f1848bb4fa1cdf8f666424fb] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
git bisect good 7d3660e79830a069f1848bb4fa1cdf8f666424fb
# bad: [92fbc3e07eb924bd5e03d22764e637b2e02e46bd] vhost_net: use the function qemu_get_peer
git bisect bad 92fbc3e07eb924bd5e03d22764e637b2e02e46bd
# good: [495134b75cca3e6a34d4233113c5143439061771] hw/riscv: sifive: Change SiFive E/U CPU reset vector to 0x1004
git bisect good 495134b75cca3e6a34d4233113c5143439061771
# good: [4e5df0369fb5a49ed26518dccffeb03a252352db] adb: add autopoll_blocked variable to block autopoll
git bisect good 4e5df0369fb5a49ed26518dccffeb03a252352db
# good: [c7459633baa71d1781fde4a245d6ec9ce2f008cf] target/arm: Enable MTE
git bisect good c7459633baa71d1781fde4a245d6ec9ce2f008cf
# bad: [3591ddd39987cbdaa0cfa344a262f315abd97582] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
git bisect bad 3591ddd39987cbdaa0cfa344a262f315abd97582
# bad: [1f18a1e6ab8368a4eab2d22894d3b2ae75250cd3] target/i386: reimplement fyl2x using floatx80 operations
git bisect bad 1f18a1e6ab8368a4eab2d22894d3b2ae75250cd3
# bad: [ee760ac80ac1f130138e8eb4eba263a4d48ace51] hw/scsi/megasas: Fix possible out-of-bounds array access in tracepoints
git bisect bad ee760ac80ac1f130138e8eb4eba263a4d48ace51
# bad: [5d971f9e672507210e77d020d89e0e89165c8fc9] memory: Revert "memory: accept mismatching sizes in memory_region_access_valid"
git bisect bad 5d971f9e672507210e77d020d89e0e89165c8fc9
# good: [ae2b72072bc401114ebb9f46abd67d07d4cabf10] util/getauxval: Porting to FreeBSD getauxval feature
git bisect good ae2b72072bc401114ebb9f46abd67d07d4cabf10
# good: [4b7c06837ae0b1ff56473202a42e7e386f53d6db] libqos: pci-pc: use 32-bit write for EJ register
git bisect good 4b7c06837ae0b1ff56473202a42e7e386f53d6db
# first bad commit: [5d971f9e672507210e77d020d89e0e89165c8fc9] memory: Revert "memory: accept mismatching sizes in memory_region_access_valid"

Reply via email to