linux-kernel - Re: WC vs UC mappings in snd_dma_sg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <s5h5ytrzlit.wl-tiwai@suse.de>
Date:   Wed, 20 Oct 2021 18:52:26 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     sanekf@....ru
Cc:     Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: WC vs UC mappings in snd_dma_sg_alloc()

On Tue, 19 Oct 2021 17:03:08 +0200,
sanekf@....ru wrote:
> 
> Hi
> 
> I've stumbled across this code in sound/core/sgbuf.c:
> 
> 66 static void *snd_dma_sg_alloc(struct snd_dma_buffer *dmab, size_t size)
> 67 {
> < ... >
> 80 	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG) {
> 81 		type = SNDRV_DMA_TYPE_DEV_WC;
> 82 #ifdef pgprot_noncached
> 83 		prot = pgprot_noncached(PAGE_KERNEL);
> 84 #endif
> 85 	}
> < ... >
> 131 	area = vmap(sgbuf->page_table, sgbuf->pages, VM_MAP, prot);
> 
> Does not this violate x86 rules about using the same memory mapping type for all mappings? It seems that the following patch should fix it (only compile tested - my x86 PCs are either without Linux or without sound, and probably in practice this might not trigger any problems since both WC and UC-minus are incoherent types):

Actually this has been already fixed in the latest code for 5.16
together with the rewriting of the SG buffer code.  There, sgbuf.c is
gone completely, and this inconsistency was resolved, too.


thanks,

Takashi

> 
> -----------------------------
> 
> ALSA: memalloc: duly use pgprot_writecombine() for WC mapping
> 
> x86 has strict rules about not having memory type aliasing (Documentation/x86/pat.rst). snd_dma_sg_alloc() violates them by mapping first as WC (with set_memory_wc()) and then as UC- (with pgprot_noncached() + vmap()). Switching to pgprot_writecombine() should fix this.
> 
> Signed-off-by: Aleksandr Fedorov <halcien@...il.com>
> diff --git a/sound/core/sgbuf.c b/sound/core/sgbuf.c
> index 8352a5cdb19f..670b30c3b6e5 100644
> --- a/sound/core/sgbuf.c
> +++ b/sound/core/sgbuf.c
> @@ -79,9 +79,7 @@ static void *snd_dma_sg_alloc(struct snd_dma_buffer *dmab, size_t size)
>  		return NULL;
>  	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG) {
>  		type = SNDRV_DMA_TYPE_DEV_WC;
> -#ifdef pgprot_noncached
> -		prot = pgprot_noncached(PAGE_KERNEL);
> -#endif
> +		prot = pgprot_writecombine(PAGE_KERNEL);
>  	}
>  	sgbuf->dev = dmab->dev.dev;
>  	pages = snd_sgbuf_aligned_pages(size);
>