linux-kernel - [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-next>] [day] [month] [year] [list]

Message-ID: <5457e8c1-01ff-4dd9-b49c-15b817f65ee7@stanley.mountain>
Date: Mon, 30 Sep 2024 10:19:58 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Jaroslav Kysela <perex@...ex.cz>
Cc: 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: [PATCH] ALSA: silence integer wrapping warning

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.

 sound/core/control.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/core/control.c b/sound/core/control.c
index 4f55f64c42e1..82b9d14f4ee3 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -1641,6 +1641,8 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
 	count = info->owner;
 	if (count == 0)
 		count = 1;
+	if (count > MAX_CONTROL_COUNT)
+		return -EINVAL;

 	/* Arrange access permissions if needed. */
 	access = info->access;
-- 
2.45.2