From: Dan Carpenter <dan.carpenter@linaro.org>
To: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>, Mark Brown <broonie@kernel.org>,
Takashi Sakamoto <o-takashi@sakamocchi.jp>,
linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org
Subject: [PATCH] ALSA: silence integer wrapping warning
Date: Mon, 30 Sep 2024 10:19:58 +0300 [thread overview]
Message-ID: <5457e8c1-01ff-4dd9-b49c-15b817f65ee7@stanley.mountain> (raw)
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@linaro.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
next reply other threads:[~2024-09-30 7:20 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-30 7:19 Dan Carpenter [this message]
2024-09-30 7:35 ` Jaroslav Kysela
2024-10-01 12:58 ` Takashi Iwai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5457e8c1-01ff-4dd9-b49c-15b817f65ee7@stanley.mountain \
--to=dan.carpenter@linaro.org \
--cc=broonie@kernel.org \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=o-takashi@sakamocchi.jp \
--cc=perex@perex.cz \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®