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 25DAEC76196 for ; Thu, 6 Apr 2023 23:27:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 47B7E6B0071; Thu, 6 Apr 2023 19:27:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 42BEB6B0074; Thu, 6 Apr 2023 19:27:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 31BC06B0075; Thu, 6 Apr 2023 19:27:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 23C086B0071 for ; Thu, 6 Apr 2023 19:27:42 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id CC192121001 for ; Thu, 6 Apr 2023 23:27:41 +0000 (UTC) X-FDA: 80652555522.19.4A02AF7 Received: from mail-yb1-f169.google.com (mail-yb1-f169.google.com [209.85.219.169]) by imf03.hostedemail.com (Postfix) with ESMTP id 1E5112000D for ; Thu, 6 Apr 2023 23:27:39 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=Et1SgasC; spf=pass (imf03.hostedemail.com: domain of tjmercier@google.com designates 209.85.219.169 as permitted sender) smtp.mailfrom=tjmercier@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680823660; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=5TDmHE+6Z/IxOxGPOrmPdh/+ahrGLDRiBjXuUSgqT7s=; b=ey2r6G+GGOm/fvgElauJM6onfLMqnkU76MlhTC2mlDktRnlaOgrBLWBlMRcxu5FVZhelue I0SWQ6rzjImQhdf8b2Z3yb6A8yuh15cLLm0SATNI6+1p7JOcU2u/p8k+btJmDudFVMa2HK d607/TI4/dF7qufP3wI0YUt9Yil0U+c= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=Et1SgasC; spf=pass (imf03.hostedemail.com: domain of tjmercier@google.com designates 209.85.219.169 as permitted sender) smtp.mailfrom=tjmercier@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680823660; a=rsa-sha256; cv=none; b=3ciTX620wh2IHTSXo1YWEGxmKkHOu0oW19LmMpj8I1CGWwYpXOCDFKyMOBlehlDX5IMmy9 NkE9SvlAmuS/7u7xxNwcyJqlYjE7xyYr7rxRAy0kKOwngGxOUoXugn5UnXigP6qrZ5asTr X8jHMvIh0M0/6V1dyrhAzXZhhjIVw3M= Received: by mail-yb1-f169.google.com with SMTP id r187so47733686ybr.6 for ; Thu, 06 Apr 2023 16:27:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680823659; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=5TDmHE+6Z/IxOxGPOrmPdh/+ahrGLDRiBjXuUSgqT7s=; b=Et1SgasCmflDYlpPApKkbbGVFvsipra17Uu+GGK8mwwGzKrBhJUqOtt6zoFvNNtnrD HCnxfqLYwANaJifhCfrKLsE/RK5YrWpR/QjvVXP8N9WVNeoif9xBHnvYc7pc8jIckrUl gWXLJH8tYr+EUgloRfIJdwaJyvZl29idB5QIGB6goRd0Mt3j4Gurr1rMRkVCRa/2VIzy MQE6i3DKQuq1Yn6ZnftsZML08s+JoYUBuJRuPHWrr/gbnSI5yJruqx9N1to+LZO/vs6o jln/xz3/nnJBRzapGM5o4tVzD41eS5fvLRGTALzcgRzwYf5Ba+lPdcZK8HAoZShLQA/n WJeg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680823659; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5TDmHE+6Z/IxOxGPOrmPdh/+ahrGLDRiBjXuUSgqT7s=; b=YWpY/KnS67G8+m4Q2KOCcmFtL0UbiV1J9Z2hZpICby+pgasORiSki9sAYs5zEpN+gX uYS9a1+HyY4HRDaKl7L+SGb8PEat07rbepE2g+C1+S8SHcugKvp48Urf+DukG6g5t8oP X0iWlSjrt4kCkgELizUMUYEy8uZKS7ZRvLO+u9Q+fZN+OBMPXqFcy5bm1dfOo7Oj0VSf 1TrwpfzWYLph5VyEtBDs4o7fNg9Je2c/BMlbRRh9U3ThqA8+TU5DLf9KdzOY1CEdUfPW s6WRvOUIJkhsj598jH/CjYuyA7ofZlV9OfR90firjHa6AH9s99LuNYaKRPNKTwEhrXTG uqaQ== X-Gm-Message-State: AAQBX9f1gRs3uNuf9nu7zi8ciW/9KGGXGTBK0pYhYmBEkbxeZNwUF9Da Z025mLNWm73/9MXz4pf1xikglOS6nnLutrv2ajbPDQ== X-Google-Smtp-Source: AKy350bseRgo/6enzMBu28AACsjF+fsREfCPn4L0Y43rnpJBnC3NdkEGtmH9IqryvaFSDw2ENLZzTwYNTkLRxTQsr7E= X-Received: by 2002:a25:dad3:0:b0:b77:e465:cb16 with SMTP id n202-20020a25dad3000000b00b77e465cb16mr681943ybf.11.1680823659067; Thu, 06 Apr 2023 16:27:39 -0700 (PDT) MIME-Version: 1.0 References: <20230405185650.239f9721f066aa480e83d543@linux-foundation.org> <20230405172524.e25b62e1c548a95564b1d324@linux-foundation.org> <20230406000854.25764-1-jaewon31.kim@samsung.com> <20230406014419epcms1p3f285b6e3fdbb1457db1bcbaab9e863be@epcms1p3> <20230406021712epcms1p216f274040d25d18380668ffbfa809c48@epcms1p2> <20230405200923.9b0dca2165ef3335a0f6b112@linux-foundation.org> In-Reply-To: From: "T.J. Mercier" Date: Thu, 6 Apr 2023 16:27:28 -0700 Message-ID: Subject: Re: [PATCH v2] dma-buf/heaps: system_heap: Avoid DoS by limiting single allocations to half of all memory To: John Stultz Cc: Andrew Morton , jaewon31.kim@samsung.com, "sumit.semwal@linaro.org" , "daniel.vetter@ffwll.ch" , "hannes@cmpxchg.org" , "mhocko@kernel.org" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "jaewon31.kim@gmail.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 1E5112000D X-Stat-Signature: ysyktqdnwkuyku7d7t86pu9yhsknofji X-HE-Tag: 1680823659-801390 X-HE-Meta: U2FsdGVkX19GkjLoM1y3wAx+S7OwmcTXJi+cyiotYAOsBNSi/wG+9beYk1hyW3NKgk6s0+9S/d4qLxT5YfUYcvZaNQr897X0MDb0phtEDMqe0uYEWm3TiHq6lfc3V3ml3npWSkkQ9rCX/2Da6+9I+6iDwy6Uw02kotrhOCVoY8tWLUvl+DMR7IgWS6WiZmPtXYKTlF3yajjzDvRtEugpeWsCIzOzAyt8OkzX1E+ukGuuWrJGbKSzUDJVvqzdeKUfGOZ8EBFMVT8/YD+YOGV+PUQPgrtBOUIq5MpopWJOU4kJ9u7tWanAm87XCj+ec6JEcaFNtyUBi4ouaEg8Cd0HY/Jg946XNtzz6HFcliexiwp82z93bTI1Ig3LA7CTeQqDBJva+ru++6cTYVkhPLfIfl7PjlZH7ELWUTwreLdvj+qpXBFrfS8YHN5wqadCXsbooej5aUzLwEm6pD1ZGX6/CVp8bO6u61gC5kOjhV47JIkYRwgshjS+tUqfnN12PwHtA5oVtz3nYuemM8BqaUZHMG2ZNsdPSjZq/cz7k8amOIZT8Prq/1ZAc2D/buu3YadfTwW9MrITe1cO9wuOmDZNLnAatwcjYEQDMafBozsKAx1HTQ962KNlDZwd7EGppN8BZpb9LbzZ++Brag43731xVlzf12xz9lmPYDIdaTgea/fSxMLIiqvOGpUJshFEBbkZI+MGV+E7/fngACDJ5seb9TpTVfsMvjEeo3JmAEgkcxi/0bvgoT+9080HJ9/Jh62IUAySYnDFw9nzUaw/ufngXwT9bHuVrf3jaWbi3Lv8u3ssOCv/Ht0DGQ73LeTv5FDPNX0lFK8y6WgXmGItOVcOmswvbxMwqb/priIXpvom2bSqhbW3Pvb0TiIlPfSMgg81iar5r3Bn6XpNLY8EVGumsvcAVaF1RPQMDcC6Uzstj9oVBG2ENIwlV30gebPuKbe1yo0Fsy7CoEP3Y7sg2yQ RJYbTQDp TpD/3CbwnjAJOChNivgLweCNinNO+KEZgnuPfAS1Al5MbQeBISFNVRTIgKRwqokd/tCs6zWREhHReC+vvE7yqjcOWj0cNzajbU4GZEDTNjCmNY3nxFpOO61asP9p529EJUntAkNG/LrxrEYmMSOSumUOXyhBLaeVLxKcudSI2/tcH2/CYNodJACB+vRPU8hE5dOro297yjDE5asI7jH7YonN0Dso5x6o86JpjNvO9F4oK6utgBL2A0F9gl4rx7qIni6XB1GydPVo4B7RTwQxBAWmxDTnyj3+9wUinNLVzIU3Bpf8x963bu9sgHcylOidPjpFTLhhpTJOlH8rsOvPOROdnr+y7ENbPRvXAwrz7wjWhEt1SJgSpVkaFFTD39Z49HnWRcppl5x4EkCUnp3at3WDa1uT5fq91bzZ9sdFGO6aM6/FuEaGy2jrM0c7EJVMBmU3kzQybYvt5j+uVWqcgxzSn4xApIUdWMTGVPZxdm3sOrYbeJhLR7e4FAYZKjrvCuXH9E2T0fsSLtExN5tJadniFfdeGIiRWnBYyBwkZDnbh4mcYEZIozunWKDCe9wfjYDCj 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, Apr 5, 2023 at 9:24=E2=80=AFPM John Stultz wro= te: > > On Wed, Apr 5, 2023 at 8:09=E2=80=AFPM Andrew Morton wrote: > > On Thu, 06 Apr 2023 11:17:12 +0900 Jaewon Kim wrote: > > > > > >> + if (len / PAGE_SIZE > totalram_pages()) > > > >> + return ERR_PTR(-ENOMEM); > > > > > > > >We're catering for a buggy caller here, aren't we? Are such large > > > >requests ever reasonable? > > > > > > > >How about we decide what's the largest reasonable size and do a > > > >WARN_ON(larger-than-that), so the buggy caller gets fixed? > > > > > > Yes we're considering a buggy caller. I thought even totalram_pages()= / 2 in > > > the old ion system is also unreasonable. To avoid the /2, I changed i= t to > > > totalram_pages() though. > > > > > > Because userspace can request that size repeately, I think WARN_ON() = may be > > > called to too often, so that it would fill the kernel log buffer. > > > > Oh geeze. I trust that userspace needs elevated privileges of some for= m? > > > > If so, then spamming dmesg isn't an issue - root can do much worse than > > that. > > > > > Even we think WARN_ON_ONCE rather than WARN_ON, the buggy point is no= t kernel > > > layer. Unlike page fault mechanism, this dma-buf system heap gets the= size from > > > userspace, and it is allowing unlimited size. I think we can't fix th= e buggy > > > user space with the kernel warning log. So I think warning is not eno= ugh, > > > and we need a safeguard in kernel layer. > > > > I really dislike that ram/2 thing - it's so arbitrary, hence is surely > > wrong for all cases. Is there something more thoughtful we can do? > > Just for context, here's the old commit that added this to ION: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/comm= it/?id=3Dc9e8440eca61298ecccbb27f53036124a7a3c6c8 > > I think the consideration was that allocations larger than half of > memory are likely due to erroneously "negative" size values. > > My memory is foggy on any discussions from that time, but I imagine > the thinking was treating the value as if it were signed and error out > immediately on negative values, but rather than just capping at 2gb on > 32bit systems, one could scale it to half of the system memory size, > as that seemed an appropriate border of "obviously wrong". > > And the reason why I think folks wanted to avoid just warning and > continuing with the allocation, is that these large allocations would > bog the system down badly before it failed, so failing quickly was a > benefit as the system was still responsive and able to be used to > collect logs and debug the issue. > > When you say "decide what's the largest reasonable size", I think it > is difficult as with the variety of RAM sizes and buffer sizes I don't > think there's a fixed limit. Systems with more ram will use larger > buffers for image/video capture buffers. And yes, you're right that > ram/2-1 in a single allocation is just as broken, but I'm not sure how > to establish a better guard rail. > > thanks > -john I like ENOMEM with the len / PAGE_SIZE > totalram_pages() check and WARN_ON. We know for sure that's an invalid request, and it's pretty cheap to check as opposed to trying a bunch of reclaim before failing. For buffers smaller than that I agree with John in that I'm not sure there's a definitive threshold.