Re: [PATCH v2] virtio-snd: Enhance error handling for invalid transfers

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

ping

On Fri, 22 Mar 2024 13:08, Zheyu Ma <zheyuma97@gmail.com> wrote:
>This patch improves error handling in virtio_snd_handle_tx_xfer()
>and virtio_snd_handle_rx_xfer() in the VirtIO sound driver. Previously,
>'goto' statements were used for error paths, leading to unnecessary
>processing and potential null pointer dereferences. Now, 'continue' is
>used to skip the rest of the current loop iteration for errors such as
>message size discrepancies or null streams, reducing crash risks.
>
>ASAN log illustrating the issue addressed:
>
>ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000b4
>    #0 0x57cea39967b8 in qemu_mutex_lock_impl qemu/util/qemu-thread-posix.c:92:5
>    #1 0x57cea128c462 in qemu_mutex_lock qemu/include/qemu/thread.h:122:5
>    #2 0x57cea128d72f in qemu_lockable_lock qemu/include/qemu/lockable.h:95:5
>    #3 0x57cea128c294 in qemu_lockable_auto_lock qemu/include/qemu/lockable.h:105:5
>    #4 0x57cea1285eb2 in virtio_snd_handle_rx_xfer qemu/hw/audio/virtio-snd.c:1026:9
>    #5 0x57cea2caebbc in virtio_queue_notify_vq qemu/hw/virtio/virtio.c:2268:9
>    #6 0x57cea2cae412 in virtio_queue_host_notifier_read qemu/hw/virtio/virtio.c:3671:9
>    #7 0x57cea39822f1 in aio_dispatch_handler qemu/util/aio-posix.c:372:9
>    #8 0x57cea3979385 in aio_dispatch_handlers qemu/util/aio-posix.c:414:20
>    #9 0x57cea3978eb1 in aio_dispatch qemu/util/aio-posix.c:424:5
>    #10 0x57cea3a1eede in aio_ctx_dispatch qemu/util/async.c:360:5
>
>Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
>Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>---
>Changes in v2:
>    - Applied similar error handling logic to virtio_snd_handle_rx_xfer()
>for consistency.
>---
> hw/audio/virtio-snd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
>index e604d8f30c..30493f06a8 100644
>--- a/hw/audio/virtio-snd.c
>+++ b/hw/audio/virtio-snd.c
>@@ -913,13 +913,13 @@ static void virtio_snd_handle_tx_xfer(VirtIODevice *vdev, VirtQueue *vq)
>                             &hdr,
>                             sizeof(virtio_snd_pcm_xfer));
>         if (msg_sz != sizeof(virtio_snd_pcm_xfer)) {
>-            goto tx_err;
>+            continue;
>         }
>         stream_id = le32_to_cpu(hdr.stream_id);
> 
>         if (stream_id >= s->snd_conf.streams
>             || s->pcm->streams[stream_id] == NULL) {
>-            goto tx_err;
>+            continue;
>         }
> 
>         stream = s->pcm->streams[stream_id];
>@@ -995,13 +995,13 @@ static void virtio_snd_handle_rx_xfer(VirtIODevice *vdev, VirtQueue *vq)
>                             &hdr,
>                             sizeof(virtio_snd_pcm_xfer));
>         if (msg_sz != sizeof(virtio_snd_pcm_xfer)) {
>-            goto rx_err;
>+            continue;
>         }
>         stream_id = le32_to_cpu(hdr.stream_id);
> 
>         if (stream_id >= s->snd_conf.streams
>             || !s->pcm->streams[stream_id]) {
>-            goto rx_err;
>+            continue;
>         }
> 
>         stream = s->pcm->streams[stream_id];
>-- 
>2.34.1
>