Re: [PATCH v2 5/5] ALSA: virtio: Keep control payloads alive through DMA completion

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: Unaligned memory access due to +1 offset with small DMA cacheline alignment
--

commit d7c4b679538e28af5e6c452343c3bd0ca83a7fd1
Author: Joonho Lee <mynameisjune111@gmail.com>

ALSA: virtio: Keep control payloads alive through DMA completion

Control requests can expose caller-owned payloads to the device after a
synchronous send times out. Allocate request and response headers and
payloads with the reference-counted message, and place each scatterlist
region on separate DMA cacheline boundaries.

> diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
> --- a/sound/virtio/virtio_ctl_msg.c
> +++ b/sound/virtio/virtio_ctl_msg.c

[ ... ]

> @@ -85,20 +161,83 @@ struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,

[ ... ]

>  	msg = kzalloc(sizeof(*msg), gfp);
>  	if (!msg)
>  		return NULL;
>  
> -	sg_init_one(&msg->sg_request, (u8 *)msg + sizeof(*msg), request_size);
> -	sg_init_one(&msg->sg_response, (u8 *)msg + sizeof(*msg) + request_size,
> -		    response_size);
> +	msg->dma_buffer = kzalloc(dma_buffer_size, gfp);
> +	if (!msg->dma_buffer) {
> +		kfree(msg);
> +		return NULL;
> +	}
> +	data = (u8 *)msg->dma_buffer + 1;

[Severity: High]
When dma_get_cache_alignment() returns 1 (as it typically does on
cache-coherent architectures like x86), does this +1 offset force all
subsequent scatterlist entries to be at an odd, unaligned memory address?

Since the alignment is 1, padding = address % alignment in
virtsnd_ctl_msg_sg_init() will evaluate to 0, leaving the data address
unaligned without any added padding.

Later, callers like virtsnd_kctl_get() cast this response data to
struct virtio_snd_ctl_value and directly load 64-bit integers:

sound/virtio/virtio_kctl.c:virtsnd_kctl_get() {
    ...
    case VIRTIO_SND_CTL_TYPE_INTEGER64:
        for (i = 0; i < count; ++i)
            uvalue->value.integer64.value[i] =
                le64_to_cpu(kvalue->value.integer64[i]);
    ...
}

Can this lead to an unaligned memory access exception or a kernel panic
when UBSAN is enabled on strict architectures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791877396.git.mynameisjune111@gmail.com?part=5