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 19CBEC47258 for ; Thu, 18 Jan 2024 00:13:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9C3B26B0085; Wed, 17 Jan 2024 19:13:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 974426B0087; Wed, 17 Jan 2024 19:13:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 815036B0098; Wed, 17 Jan 2024 19:13:58 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 6E64C6B0085 for ; Wed, 17 Jan 2024 19:13:58 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 28FDB140158 for ; Thu, 18 Jan 2024 00:13:58 +0000 (UTC) X-FDA: 81690508956.04.51F7178 Received: from mail-pg1-f177.google.com (mail-pg1-f177.google.com [209.85.215.177]) by imf28.hostedemail.com (Postfix) with ESMTP id 4C4DDC0002 for ; Thu, 18 Jan 2024 00:13:56 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=OebLN86G; spf=pass (imf28.hostedemail.com: domain of shy828301@gmail.com designates 209.85.215.177 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705536836; a=rsa-sha256; cv=none; b=jgor7z9cBxFjd57OhQvOOixFb8vF6E/Rr9NXd5lVOnjF+geXB/iHRWiQ8QYXFMlIRYtGbB VXeP8cfocL6AbPCkH9Mf51Pnk5CCgqjtWatx0nYC3my6plAFzTIgariuz9TKHeb86eXc48 PC1B8zsoJzqGfdYr/LSEEGHt09hhbr8= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=OebLN86G; spf=pass (imf28.hostedemail.com: domain of shy828301@gmail.com designates 209.85.215.177 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705536836; 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=eOITPKr6gj8Tz8POR15BR9uZhh9uFiEf+DCyXQVNgyE=; b=P5twMx+GlJs5arGaDxF21aoH/S9YWVs00QPCs2E9E3L41sE0HRiX4h68ICCiZQMZIVzuDN pjqlCzy/y+TzqZUARcw3/U5vlM5xycBHJU+Lk1JbAAtjtFMlJZ/lgFqDfI8lORwLGBuoV4 yakRk+vio6KOl91/AfkVrJ7+WlWa46E= Received: by mail-pg1-f177.google.com with SMTP id 41be03b00d2f7-5cec32dedf3so5653367a12.0 for ; Wed, 17 Jan 2024 16:13:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705536835; x=1706141635; darn=kvack.org; 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=eOITPKr6gj8Tz8POR15BR9uZhh9uFiEf+DCyXQVNgyE=; b=OebLN86Gg4IozA+w/KqkgAL2jsRVdX64hYWTDa+MmR9ZuS+kvvIC5I6hsIkC68nK+Q E2XDvd5QBBEcFOZSrHvKu8dryGcIPEjRaSkSQ/pV+ESNf1EJAlEniuNC2LVxMVRQeS8f GAOM80mzH3uB7eq6XpQOwz19wy/NER16PBgPRB9S2wPmK1kzdBVFW8li6ZUNTaHAUNKi D9BZsGTLYPVrR16we2OAzM6r1F+k8lSZJX2koUn9ggeMO/9bvEtJ691B6aYkdrKPt76O x0+aLOWzY4cBNysNw9d8nNobREsHIN4DUZJKFbx9g5gTBLiDhuHY5IwiI3LVI8uYiWew mGkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705536835; x=1706141635; 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=eOITPKr6gj8Tz8POR15BR9uZhh9uFiEf+DCyXQVNgyE=; b=iU84jBZZWHcbWMYonT0oNe3gSOw2/HDhjSDiRrMwbXd8DlJdJEi81xctz0xzjQOVL8 /NM3iSLI0a9RA2EnaE9x78DtoCAwpDYogvxhis8JR0WrWesjoD9JdDu+QeZ5Z4v4nQ0s Bn/uUpVKagTxJvAlTceNhVyi/cGzLXjpEHECiOYeHC/m6puNSmRu28siRq09EXquzUy0 GL6dR/KSkFva67sM5z+BkEfUwOmduXC+OJsdDNw3WObiIcEcsGsirYbz6kW4ec+0hv9J cXE+xW5f3WmQwfis9PTHoBKQO8t+g3a/Rn0UoysM4yVsBGdzuMeVjLhMslriJAUGUC+6 Uhfg== X-Gm-Message-State: AOJu0YxlUnmRbXoGEh1Sd23sUx9hQKigXKUg610qyX3ZEu2DnNmy0ll7 yHlMx9YElAFDRwIjv7IyIbBhRVj84qo2aWxvLvdPfRF8UDAESyhZt+bJtlnnFclsbbjgEQPZMc/ 1GlzRSluwCkC4lKFdepcKnzKc9o4= X-Google-Smtp-Source: AGHT+IHgfLolLNjIuaIitzPZ8ouXggbxq8Lw5f9f3iTIl4A3dlgnM9Y6qWnZatGpO4aFKg+s6c80owQ2E69sT/GDGk4= X-Received: by 2002:a05:6a20:1614:b0:19a:999c:665d with SMTP id l20-20020a056a20161400b0019a999c665dmr52685pzj.5.1705536835091; Wed, 17 Jan 2024 16:13:55 -0800 (PST) MIME-Version: 1.0 References: <20220809142457.4751229f@imladris.surriel.com> <3193bf5b-4e22-412f-8c5b-68574942d9bc@kernel.org> <202401170925.015D300A@keescook> In-Reply-To: From: Yang Shi Date: Wed, 17 Jan 2024 16:13:42 -0800 Message-ID: Subject: Re: [PATCH v2] mm: align larger anonymous mappings on THP boundaries To: Suren Baghdasaryan Cc: Kees Cook , Jeffrey Vander Stoep , Jiri Slaby , Rik van Riel , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, Matthew Wilcox , Christoph Lameter , linux-hardening@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 4C4DDC0002 X-Stat-Signature: bbhc85surb39xyftppp6by3gdm5fgjp7 X-Rspam-User: X-HE-Tag: 1705536836-809650 X-HE-Meta: U2FsdGVkX19RU2tuO+R2jpRC860HPwctoRoEdFzwuJMfuzGmmNCUCLcqXBHP2WgG9flD9p0jaKYqHGWUFGfKBMWrpp8aTpf/0H+mIoRqJcdpuehJ9HIjKPPV8phrNFuruD5MkoScyy/6mDoytHqLDKrPcLGdSYFn2pW3Wjd4d/igAVkTtw69zNmxuCncYH/jYS0CScw4KVYs7Kq4CBzsLms+Lcil8I+SV+SiVgN46nikCz4qMW3YD4SA3h2MmgsS/CmUPvEQWvGsTrd+tRwYoXIhY5/yTmpdaw7TfPewAFQ9MNSkPwKY7IdcPwEYv48xpf9IrglAHh2zyp2Na/bgR9DAcvYHXr7vDIP8TE0HMzw0IX6Km6bfAiak1C3Ksg8qJdzryjVTF00fdjmjkuClTSVANBcW9KhXBZ+p/gZGFyJJDfY9k3VMfc9rms6/Yoa2fe1zhdg2mKAtAiVDCFo/omnrCTDN80LbNDzIibDihXteo9M66777WENTMols5CGKvmD8SPhyizeyFtgZuaEPB/JEZIKuzFjuGDPsdhHpHfjRLiXdVNnPFYeYGD/nM3j8AoO1AC06LPJ17T4RHmbQZCpJ3C3Da/OmqGOeeFoWi/ozro2tl4QkdG4gkMiiFWGST8D3U3Wm0VMom2nvN2WVhDD5P/qpOhx/SHe3vW5DnVBDlkpo0eKg7LS1+iE3Q3A6eN1Ix+D9JSV9AU1aALtUn2fEOQaDnHGqHcxm7F8jHka0YEwUveycq7xRK0vNALr08RygP8Xzuw5djkGG656vjk/dxjNUUA4Nuu9bLT344dqboxpqCZqCsO28PYNTtkFpcyUJEtaWdLJwukoX2teygsbpVJYYzWYY7RE13aMA0bWx9+jFsEMiioY4WIQcbkWk1XtgNZVsxvVxKL7d+DWwHPJrpr2DC4K7K6KZCARSrnkuqw7NtMABZ/XS5gRCfNvxtkFEhUvIvJ+On1kUp5p Krj0TO+H CRQcYHyVPgZT/2h0ZrK8cHh7CiwqWEFnF/txQv2ILUWMeLNdvhHx+HWwffr5r13Ab6NrbQ+jC0IqyZUh8pnvFZsLQDoLvCAfsR9/D0f7bVHFeJ+DCx2U+RqN9wWVtdAayhmog7dIAxmTviaDZCmKpF9fkEa4tgCE3wUfJrZaH+5vkS8o/THN6QFQffxr0u4QvcUp4m0KmxIPlHXHUQklDwKKhxRxYFzsKyVZbhEO1Rso+mtToH9KO+sScCUXWh3+mgy1GXOXCZr48RaE+ijgceMACe3tzvWfV+kmsFFyY3aKtwvHL02yysPt+CuHxQQyUJy4Uc3JElV8YPdOLNR6GW8GvsE+68OnnJnWyaoBLm7kFdm3zhS7jF31LNQ3XbKTo+m99PHYa+rSPKZF5Xhz1pfqBo6wJAJqHtsuA4ISsOxEjlxX1j661Kj/L9n/vKRRetz7UQYL8O9GqRZw8OI0AT4yzlxpFZsZ6WvB4GMaRGaQ7feeGV6JmW8N4/ri5gkPPqVo7tNfQguRfvr2NZlB3eaRU8oI73b67bexpVkhzUbPApCHlnKumo+7vLIDAOxS6pY2sNS4sBO7kW49wCFCchYmnSrk0q0m+i7P8rfE2JT06X1fuDrzTrzgjgI/oQLJ2xgXvWLdyfB7PPG+eEXZeK2KiuhsqvhiTRGRm 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: List-Subscribe: List-Unsubscribe: On Wed, Jan 17, 2024 at 4:02=E2=80=AFPM Suren Baghdasaryan wrote: > > On Wed, Jan 17, 2024 at 3:32=E2=80=AFPM Yang Shi wr= ote: > > > > On Wed, Jan 17, 2024 at 9:40=E2=80=AFAM Kees Cook wrote: > > > > > > On Tue, Jan 16, 2024 at 02:30:36PM -0800, Suren Baghdasaryan wrote: > > > > On Tue, Jan 16, 2024 at 2:25=E2=80=AFPM Yang Shi wrote: > > > > > > > > > > On Tue, Jan 16, 2024 at 1:58=E2=80=AFPM Suren Baghdasaryan wrote: > > > > > > > > > > > > On Tue, Jan 16, 2024 at 12:56=E2=80=AFPM Yang Shi wrote: > > > > > > > > > > > > > > On Tue, Jan 16, 2024 at 11:16=E2=80=AFAM Suren Baghdasaryan <= surenb@google.com> wrote: > > > > > > > > > > > > > > > > On Tue, Jan 16, 2024 at 4:09=E2=80=AFAM Jiri Slaby wrote: > > > > > > > > > > > > > > > > > > On 16. 01. 24, 12:53, Jiri Slaby wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > On 09. 08. 22, 20:24, Rik van Riel wrote: > > > > > > > > > >> Align larger anonymous memory mappings on THP boundari= es by > > > > > > > > > >> going through thp_get_unmapped_area if THPs are enable= d for > > > > > > > > > >> the current process. > > > > > > > > > >> > > > > > > > > > >> With this patch, larger anonymous mappings are now THP= aligned. > > > > > > > > > >> When a malloc library allocates a 2MB or larger arena,= that > > > > > > > > > >> arena can now be mapped with THPs right from the start= , which > > > > > > > > > >> can result in better TLB hit rates and execution time. > > > > > > > > > > > > > > > > > > > > This appears to break 32bit processes on x86_64 (at lea= st). In > > > > > > > > > > particular, 32bit kernel or firefox builds in our build= system. > > > > > > > > > > > > > > > > > > > > Reverting this on top of 6.7 makes it work again. > > > > > > > > > > > > > > > > > > > > Downstream report: > > > > > > > > > > https://bugzilla.suse.com/show_bug.cgi?id=3D1218841 > > > > > > > > > > > > > > > > > > > > So running: > > > > > > > > > > pahole -J --btf_gen_floats -j --lang_exclude=3Drust > > > > > > > > > > --skip_encoding_btf_inconsistent_proto --btf_gen_optimi= zed .tmp_vmlinux.btf > > > > > > > > > > > > > > > > > > > > crashes or errors out with some random errors: > > > > > > > > > > [182671] STRUCT idr's field 'idr_next' offset=3D128 bit= _size=3D0 type=3D181346 > > > > > > > > > > Error emitting field > > > > > > > > > > > > > > > > > > > > strace shows mmap() fails with ENOMEM right before the = errors: > > > > > > > > > > 1223 mmap2(NULL, 5783552, PROT_READ|PROT_WRITE, > > > > > > > > > > MAP_PRIVATE|MAP_ANONYMOUS, -1, 0 > > > > > > > > > > ... > > > > > > > > > > 1223 <... mmap2 resumed>) =3D -1 ENOMEM (= Cannot allocate > > > > > > > > > > memory) > > > > > > > > > > > > > > > > > > > > Note the .tmp_vmlinux.btf above can be arbitrary, but l= ikely large > > > > > > > > > > enough. For reference, one is available at: > > > > > > > > > > https://decibel.fi.muni.cz/~xslaby/n/btf > > > > > > > > > > > > > > > > > > > > Any ideas? > > > > > > > > > > > > > > > > > > This works around the problem, of course (but is a band-a= id, not a fix): > > > > > > > > > > > > > > > > > > --- a/mm/mmap.c > > > > > > > > > +++ b/mm/mmap.c > > > > > > > > > @@ -1829,7 +1829,7 @@ get_unmapped_area(struct file *file= , unsigned long > > > > > > > > > addr, unsigned long len, > > > > > > > > > */ > > > > > > > > > pgoff =3D 0; > > > > > > > > > get_area =3D shmem_get_unmapped_area; > > > > > > > > > - } else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE= )) { > > > > > > > > > + } else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE= ) && > > > > > > > > > !in_32bit_syscall()) { > > > > > > > > > /* Ensures that larger anonymous mapping= s are THP > > > > > > > > > aligned. */ > > > > > > > > > get_area =3D thp_get_unmapped_area; > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > thp_get_unmapped_area() does not take care of the legacy = stuff... > > > > > > > > > > > > > > > > This change also affects the entropy of allocations. With t= his patch > > > > > > > > Android test [1] started failing and it requires only 8 bit= s of > > > > > > > > entropy. The feedback from our security team: > > > > > > > > > > > > > > > > 8 bits of entropy is already embarrassingly low, but was ne= cessary for > > > > > > > > 32 bit ARM targets with low RAM at the time. It's definitel= y not > > > > > > > > acceptable for 64 bit targets. > > > > > > > > > > > > > > Thanks for the report. Is it 32 bit only or 64 bit is also im= pacted? > > > > > > > If I understand the code correctly, it expects the address al= located > > > > > > > by malloc() is kind of randomized, right? > > > > > > > > > > > > Yes, correct, the test expects a certain level of address rando= mization. > > > > > > The test failure was reported while running kernel_virt_x86_64 = target > > > > > > (Android emulator on x86), so it does impact 64bit targets. > > > > > > > > > > IIUC this breaks the "expectation" for randomized addresses retur= ned > > > > > by malloc(), but it doesn't break any real Android application, r= ight? > > > > > So this is a security concern instead of a real regression. > > > > > > > > How is making a system move vulnerabile not a real regression? > > > > > > > > > > > > > > I think we can make this opt-in with a knob. Anyone who outweighs > > > > > security could opt this feature out. However I'm wondering whethe= r > > > > > Android should implement a general address randomization mechanis= m > > > > > instead of depending on "luck" if you do care about it. > > > > > > > > This is not depending on luck. This is checking for possible > > > > vulnerabilities in the system. > > > > I admit I'm not a security expert, so I'm looping in Jeff and Kees = to chime in. > > > > > > Hi! > > > > > > Just to chime in, but reduction in ASLR entropy is absolutely a > > > regression. This is userspace visible (via /proc/sys/kernel/randomize= _va_space, > > > /proc/sys/vm/mmap_rnd*_bits) with expectations that it work as > > > advertised. So, while 32-bit might be already ASLR-weak, we don't wan= t > > > to make things super bad nor break ASLR in compat mode under 64-bit > > > systems. > > > > > > Having an opt-in sounds reasonable, but we need to wire it to the ASL= R > > > sysctls in some way so nothing lying about the ASLR entropy. > > > > Thanks for the explanation. IIUC the randomiza_va_space and > > mmap_rnd_bits randomize the mmap_base and start_brk for each exec() > > call. So the heap allocation is randomized. But it seems the formula > > doesn't take into account huge page. ARM64 adjusts the mmap_rnd_bits > > based on page size. > > > > I did a simple test, which conceptually does: > > 1. call mmap to allocate 8M heap > > 2. print out the allocated address > > 3. run the program 1000 times (launch/exit/re-launch) > > 4. check how many unique addresses > > > > With the default config on my arm64 VM (mmap_rnd_bits is 18), I saw > > 134 unique addresses. Without the patch, I saw 945 unique addresses. > > So I think the test could replicate what your test does. > > > > When I increased the mmap_rnd_bits to 24, I saw 988 unique addresses > > with the patch. x86_64 should have 28 bits by default, it should > > randomize the address quite well. I don't know why you still saw this, > > or you have a different setting for mmap_rnd_bits? > > I checked the configuration on our test harness where the test failed: Thanks, Suren. > > /proc/sys/vm/mmap_rnd_bits =3D 32 It is surprising 32 bits still fail. 24 bits on arm64 works for me. Or the compat bits is used? > /proc/sys/vm/mmap_rnd_compat_bits =3D 16 > > The failure logs are: > > 10-20 14:37:52.123 7029 7029 V AslrMallocTest: 7 bits of entropy for > allocation size 8388608 (minimum 8) > 10-20 14:37:52.123 7029 7029 E AslrMallocTest: insufficient entropy > for malloc(8388608) > > which come from here: > https://cs.android.com/android/platform/superproject/main/+/main:cts/test= s/aslr/src/AslrMallocTest.cpp;l=3D127 > So, the allocation size for which this test failed was 2^23. The patch just tries to align >=3D 2M allocations. It looks like your test allocates 256 bytes, 64K and 8M. So just 8M is impacted. > > > > I'm wondering whether we should take into account huge page alignment > > for mmap_rnd_bits. And I think this is a huge page common problem, we > > have file mapping huge page aligned as well. > > > > 32 bit is easy, I think I can just make thp_get_unmapped_area() a > > no-op on 32 bit system. > > > > > > > > -Kees > > > > > > -- > > > Kees Cook