Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 26 Jun 2023 09:08:54 +0200
Message-ID: <877crqwvi1.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Tuo Li <islituo@gmail.com>
Cc:     perex@perex.cz, tiwai@suse.com, alsa-devel@alsa-project.org,
        Linux Kernel <linux-kernel@vger.kernel.org>,
        baijiaju1990@outlook.com
Subject: Re: [BUG] ALSA: core: pcm_memory: a possible data race in do_alloc_pages()
In-Reply-To: <CADm8Tek6t0WedK+3Y6rbE5YEt19tML8BUL45N2ji4ZAz1KcN_A@mail.gmail.com>
References: <CADm8Tek6t0WedK+3Y6rbE5YEt19tML8BUL45N2ji4ZAz1KcN_A@mail.gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Mon, 26 Jun 2023 05:42:29 +0200,
Tuo Li wrote:
> 
> 
> Hello,
> 
> Our static analysis tool finds a possible data race in ALSA in Linux 6.4.0.
> 
> In some functions, the field snd_card.total_pcm_alloc_bytes is accessed
> with holding the lock snd_card.memory_mutex. Here is an example:
> 
> ? do_free_pages() --> Line 57
> ? ? mutex_lock(&card->memory_mutex); --> Line 61 (Lock card->memory_mutex)
> ? ? card->total_pcm_alloc_bytes -= dmab->bytes; ?--> Line 63 (Access??card->
> total_pcm_alloc_bytes)
> 
> However, in the function do_alloc_pages():
> 
> ? if (max_alloc_per_card &&
> ? ? card->total_pcm_alloc_bytes + size > max_alloc_per_card) --> Line 41
> 
> the variable card->total_pcm_alloc_bytes is accessed without holding
> the lock card->memory_mutex, and thus a data race can occur.
> 
> In my opinion, this data race may be harmful, because the value of
> card->total_pcm_alloc_bytes may be changed by another thread after
> the if check. Therefore, its value may be too large after Line 51 and can
> cause memory bugs such as buffer overflow:
> 
> ? card->total_pcm_alloc_bytes += dmab->bytes; ?--> Line 51
> 
> I am not quite sure whether this possible data race is real and how to
> fix it if it is real.
> 
> Any feedback would be appreciated, thanks!
> 
> Reported-by: BassCheck <bass@buaa.edu.cn>

It's a bit racy indeed, but the effect is almost negligible.  The size
check there is merely a sanity check, and allocating more bytes
doesn't mean to conflict against anything practically.

That said, it's a better-to-be-addressed bug, but nothing too
serious.


thanks,

Takashi