Re: [PATCH] hw/audio/virtio-snd: fix invalid param check

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Fri, 02 Aug 2024 10:18, Volker Rümelin <vr_qemu@t-online.de> wrote:
>Commit 9b6083465f ("virtio-snd: check for invalid param shift
>operands") tries to prevent invalid parameters specified by the
>guest. However, the code is not correct.
>
>Change the code so that the parameters format and rate, which are
>a bit numbers, are compared with the bit size of the data type.
>
>Fixes: 9b6083465f ("virtio-snd: check for invalid param shift operands")
>Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
>---
> hw/audio/virtio-snd.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
>index e5196aa4bb..d1cf5eb445 100644
>--- a/hw/audio/virtio-snd.c
>+++ b/hw/audio/virtio-snd.c
>@@ -282,12 +282,12 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
>         error_report("Number of channels is not supported.");
>         return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
>     }
>-    if (BIT(params->format) > sizeof(supported_formats) ||
>+    if (params->format >= sizeof(supported_formats) * BITS_PER_BYTE ||
>         !(supported_formats & BIT(params->format))) {
>         error_report("Stream format is not supported.");
>         return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
>     }
>-    if (BIT(params->rate) > sizeof(supported_rates) ||
>+    if (params->rate >= sizeof(supported_rates) * BITS_PER_BYTE ||
>         !(supported_rates & BIT(params->rate))) {
>         error_report("Stream rate is not supported.");
>         return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
>-- 
>2.35.3
>

Thanks for the fix Volker :)

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>