[PATCH] block: Fix crash after setting latency historygram with single bin

Kevin Wolf posted 1 patch 1 day, 11 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260331102608.60882-1-kwolf@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
block/accounting.c | 9 +++++++++
1 file changed, 9 insertions(+)
[PATCH] block: Fix crash after setting latency historygram with single bin
Posted by Kevin Wolf 1 day, 11 hours ago
Passing an empty list of boundaries to block-latency-histogram-set sets
up a state that leads to a NULL pointer dereference when the next
request should be accounted for. This is not a useful configuration, so
just error out if the user tries to set it.

The crash can easily be reproduced with the following script:

    qmp() {
    cat <<EOF
    {'execute':'qmp_capabilities'}
    {'execute':'block-latency-histogram-set',
     'arguments': {'id':'ide0','boundaries':[]}}
    {'execute':'cont'}
    EOF
    }

    qmp | ./qemu-system-x86_64 -S -qmp stdio \
        -drive if=none,format=raw,file=null-co:// \
        -device ide-hd,drive=none0,id=ide0

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/accounting.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/block/accounting.c b/block/accounting.c
index 5cf51f029b1..f00fe997403 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -185,6 +185,15 @@ int block_latency_histogram_set(BlockAcctStats *stats, enum BlockAcctType type,
         prev = entry->value;
     }
 
+    /*
+     * block_latency_histogram_account() assumes that it can always access
+     * hist->boundaries[0], so require at least one boundary. A histogram with
+     * a single bin is useless anyway.
+     */
+    if (new_nbins <= 1) {
+        return -EINVAL;
+    }
+
     hist->nbins = new_nbins;
     g_free(hist->boundaries);
     hist->boundaries = g_new(uint64_t, hist->nbins - 1);
-- 
2.53.0
Re: [PATCH] block: Fix crash after setting latency historygram with single bin
Posted by Philippe Mathieu-Daudé 1 day, 4 hours ago
On 31/3/26 12:26, Kevin Wolf wrote:
> Passing an empty list of boundaries to block-latency-histogram-set sets
> up a state that leads to a NULL pointer dereference when the next
> request should be accounted for. This is not a useful configuration, so
> just error out if the user tries to set it.
> 
> The crash can easily be reproduced with the following script:
> 
>      qmp() {
>      cat <<EOF
>      {'execute':'qmp_capabilities'}
>      {'execute':'block-latency-histogram-set',
>       'arguments': {'id':'ide0','boundaries':[]}}
>      {'execute':'cont'}
>      EOF
>      }
> 
>      qmp | ./qemu-system-x86_64 -S -qmp stdio \
>          -drive if=none,format=raw,file=null-co:// \
>          -device ide-hd,drive=none0,id=ide0
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block/accounting.c | 9 +++++++++
>   1 file changed, 9 insertions(+)

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

Re: [PATCH] block: Fix crash after setting latency historygram with single bin
Posted by Hanna Czenczek 1 day, 6 hours ago
On 31.03.26 12:26, Kevin Wolf wrote:
> Passing an empty list of boundaries to block-latency-histogram-set sets
> up a state that leads to a NULL pointer dereference when the next
> request should be accounted for. This is not a useful configuration, so
> just error out if the user tries to set it.
>
> The crash can easily be reproduced with the following script:
>
>      qmp() {
>      cat <<EOF
>      {'execute':'qmp_capabilities'}
>      {'execute':'block-latency-histogram-set',
>       'arguments': {'id':'ide0','boundaries':[]}}
>      {'execute':'cont'}
>      EOF
>      }
>
>      qmp | ./qemu-system-x86_64 -S -qmp stdio \
>          -drive if=none,format=raw,file=null-co:// \
>          -device ide-hd,drive=none0,id=ide0
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block/accounting.c | 9 +++++++++
>   1 file changed, 9 insertions(+)

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

(Though the subject should probably say "histogram" instead of 
"historygram")