From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6BA3AC433F5 for ; Wed, 16 Mar 2022 19:29:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 79D306B0071; Wed, 16 Mar 2022 15:29:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 74DB28D0002; Wed, 16 Mar 2022 15:29:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 615508D0001; Wed, 16 Mar 2022 15:29:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.25]) by kanga.kvack.org (Postfix) with ESMTP id 542596B0071 for ; Wed, 16 Mar 2022 15:29:08 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay11.hostedemail.com (Postfix) with ESMTP id 30288814D2 for ; Wed, 16 Mar 2022 19:29:08 +0000 (UTC) X-FDA: 79251237576.12.FAC8846 Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com [209.85.167.46]) by imf30.hostedemail.com (Postfix) with ESMTP id B6B318000C for ; Wed, 16 Mar 2022 19:29:07 +0000 (UTC) Received: by mail-lf1-f46.google.com with SMTP id n19so5430379lfh.8 for ; Wed, 16 Mar 2022 12:29:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=kXm3aPGa6kumOLc0BpQgk8B33A75Ce44sP5rN5yEc48=; b=UM0MNayjkbqgfu1XWtyQYDhdQX7mfqFrOqJ5oTFXnVWReMsB2OW0NJS9z0mNjhhG7T N2kjjSIYqHXhSCpdeX1kH9/ZaHtGOMLhLlpsRan74z8rXHQzlg/+zNFykJGOa5jH7x5Y O3ezJcxIGCKF6PpACvz1gCAsDUwBPb8o3fyzQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=kXm3aPGa6kumOLc0BpQgk8B33A75Ce44sP5rN5yEc48=; b=KkxEb3j3goL0wJ+w+yg/djcvAlFvlCVx6xGGFb1Pq+gbOtiWY+0/l/4rKbCDfF0iWV PAP3R8/h4z4i7gl3ss0n/POTl138klbOW67NJbfkIxb+uIoyUP2f+oLesrWYjZgzNyt0 nM/3oXZHOMw8l/C1zjT5atRl37GQbCshgQbPrOW/eZLwpMXWuOo+YAeGh2N5MKSVVh5g vkngPLnCLrDAChHVlclHXRM2Cm3d9qLclvtORbZsN2uXrftJNczLl4cGJfbaLoeozGpG 8yDKDbfY3zqwMuEj7wjygKGv2/cpGb2ho1lNqDzCIG/liN8A5FKuKSK1gf9Q8eRBwh9k rNRA== X-Gm-Message-State: AOAM532oz9XaM4xisj2orBDuTtiuyZvGivWB8Mi7bJvzt0NbNVpAg/N7 wkKOQkJINielymGxa2oDw0cNOD8SwJbQHmfOrDc= X-Google-Smtp-Source: ABdhPJy/hZYFU69boXqm6LVzCW9JQch/rMRGxs7b7Z+ZeP6qvARTUbNnuGZd/Qm/4odQ9ycge0U0VA== X-Received: by 2002:a05:6512:400d:b0:443:dcdb:bdff with SMTP id br13-20020a056512400d00b00443dcdbbdffmr680701lfb.287.1647458945228; Wed, 16 Mar 2022 12:29:05 -0700 (PDT) Received: from mail-lj1-f181.google.com (mail-lj1-f181.google.com. [209.85.208.181]) by smtp.gmail.com with ESMTPSA id bu1-20020a056512168100b004437db5e773sm250804lfb.94.2022.03.16.12.29.03 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 16 Mar 2022 12:29:03 -0700 (PDT) Received: by mail-lj1-f181.google.com with SMTP id z26so4449772lji.8 for ; Wed, 16 Mar 2022 12:29:03 -0700 (PDT) X-Received: by 2002:a2e:6f17:0:b0:248:124:9c08 with SMTP id k23-20020a2e6f17000000b0024801249c08mr671918ljc.506.1647458942587; Wed, 16 Mar 2022 12:29:02 -0700 (PDT) MIME-Version: 1.0 References: <00000000000085b1b305da5a66f3@google.com> In-Reply-To: <00000000000085b1b305da5a66f3@google.com> From: Linus Torvalds Date: Wed, 16 Mar 2022 12:28:46 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [syzbot] WARNING: kmalloc bug in snd_pcm_plugin_alloc (2) To: syzbot , Jaroslav Kysela , Takashi Iwai Cc: Andrew Morton , alsa-devel@alsa-project.org, Linux Kernel Mailing List , Linux-MM , syzkaller-bugs , Willy Tarreau Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: B6B318000C X-Stat-Signature: fu8px6bto9jt6jprua4p59pcm3fcbd57 Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=UM0MNayj; dmarc=none; spf=pass (imf30.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.167.46 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org X-Rspam-User: X-HE-Tag: 1647458947-342404 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Mar 16, 2022 at 11:51 AM syzbot wrote: > > WARNING: CPU: 1 PID: 3761 at mm/util.c:591 kvmalloc_node+0x121/0x130 mm/util.c:591 > snd_pcm_plugin_alloc+0x570/0x770 sound/core/oss/pcm_plugin.c:71 > snd_pcm_plug_alloc+0x20d/0x310 sound/core/oss/pcm_plugin.c:118 > snd_pcm_oss_change_params_locked+0x19db/0x3bf0 sound/core/oss/pcm_oss.c:1041 > snd_pcm_oss_change_params sound/core/oss/pcm_oss.c:1104 [inline] > snd_pcm_oss_get_active_substream+0x164/0x1c0 sound/core/oss/pcm_oss.c:1121 > snd_pcm_oss_get_rate sound/core/oss/pcm_oss.c:1778 [inline] > snd_pcm_oss_set_rate sound/core/oss/pcm_oss.c:1770 [inline] > snd_pcm_oss_ioctl+0x144f/0x3430 sound/core/oss/pcm_oss.c:2632 Well, that looks like a real bug in the sound subsystem, and the warning is appropriate. It looks like size = frames * format->channels * width; can overflow 32 bits, and this is presumably user-triggerable with snd_pcm_oss_ioctl(). Maybe there's some range check at an upper layer that is supposed to catch this, but I'm not seeing it. I think the simple fix is to do size = array3_size(frames, format->channels, width); instead, which clamps the values at the maximum size_t. Then you can trivially check for that overflow value (SIZE_MAX), but you can - and probably should - just check for some sane value. INT_MAX comes to mind, since that's what the allocation routine will warn about. But you can also say "Ok, I have now used the 'array_size()' function to make sure any overflow will clamp to a very high value, so I know I'll get an allocation failure, and I'd rather just make the allocator do the size checking, so I'll add __GFP_NOWARN at allocation time and just return -ENOMEM when that fails". But that __GFP_NOWARN is *ONLY* acceptable if you have actually made sure that "yes, all my size calculations have checked for overflow and/or done that SIZE_MAX clamping". Alternatively, you can just do each multiplication carefully, and use "check_mul_overflow()" by hand, but it's a lot more inconvenient and the end result tends to look horrible. There's a reason we have those "array_size()" and "array3_size()" helpers. There is also some very odd and suspicious-looking code in snd_pcm_oss_change_params_locked(): oss_period_size *= oss_frame_size; oss_buffer_size = oss_period_size * runtime->oss.periods; if (oss_buffer_size < 0) { err = -EINVAL; goto failure; } which seems to think that checking the end result for being negative is how you check for overflow. But that's actually after the snd_pcm_plug_alloc() call. It looks like all of this should use "check_mul_overflow()", but it presumably also wants fixing (and also would like to use the 'array_size()' helpers, but note that those take a 'size_t', so you do want to check for negative values *before* if you allow zeroes anywhere else) If you don't mind "multiplying by zero will hide a negative intermediate value", you can pass in 'ssize_t' arguments, do the multiplication as unsigned, put the result in a 'ssize_t' value, and just check for a negative result. That would seem to be acceptable here, and that snd_pcm_oss_change_params_locked() code could also just be oss_period_size = array_size(oss_period_size, oss_frame_size); oss_buffer_size = array_size(oss_period_size, runtime->oss.periods); if (oss_buffer_size < 0) { ... but I would suggest checking for a zero result too, because that can hide the sub-parts having been some invalid crazy values that can also cause problems later. Takashi? Linus