linux-kernel - Re: [PATCH] ALSA: silence integer wrapping warning

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: <87ed50nfns.wl-tiwai@suse.de>
Date: Tue, 01 Oct 2024 14:58:31 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>,
	Mark Brown <broonie@...nel.org>,
	Takashi Sakamoto <o-takashi@...amocchi.jp>,
	linux-sound@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] ALSA: silence integer wrapping warning

On Mon, 30 Sep 2024 09:19:58 +0200,
Dan Carpenter wrote:
> 
> This patch doesn't change runtime at all, it's just for kernel hardening.
> 
> The "count" here comes from the user and on 32bit systems, it leads to
> integer wrapping when we pass it to compute_user_elem_size():
> 
> 	alloc_size = compute_user_elem_size(private_size, count);
> 
> However, the integer over is harmless because later "count" is checked
> when we pass it to snd_ctl_new():
> 
> 	err = snd_ctl_new(&kctl, count, access, file);
> 
> These days as part of kernel hardening we're trying to avoid integer
> overflows when they affect size_t type.  So to avoid the integer overflow
> copy the check from snd_ctl_new() and do it at the start of the
> snd_ctl_elem_add() function as well.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
> ---
> I'm going to write a blog about this which explains the kernel hardening
> proposal in more detail.
> 
> The problem is that integer overflows are really hard to analyze
> because the integer overflow itself is harmless.  The harmful thing comes
> later.  Not only are integer overflows harmless, but many of them are
> done deliberately.
> 
> So what we're doing is we're saying that size_t types should not overflow.
> This eliminates many deliberate integer overflows handling time values for
> example.  We're also ignoring deliberate idiomatic integer overflows such
> as if (a + b < a) {.
> 
> We're going to detect these integer overflows using static analysis and at
> runtime using UBSan and Syzbot.
> 
> The other thing, actually, is the we're planning to only work on 64bit
> systems for now so if you want to ignore this patch then that's fine.  There
> are a lot more (like 10x more) integer overflows on 32bit systems but most
> people are on 64bit.  So it's less work and more impact to focus on 64bit
> at first.

The fix is straightforward and still better to have even for 64bit, so
let's take it.  Now merged to for-linus branch.


thanks,

Takashi