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 ACE06C25B75 for ; Thu, 16 May 2024 00:59:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EA2126B02BA; Wed, 15 May 2024 20:59:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E29DD6B02BE; Wed, 15 May 2024 20:59:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CA9256B02BB; Wed, 15 May 2024 20:59:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id A2C116B0151 for ; Wed, 15 May 2024 20:59:43 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 14CC2140635 for ; Thu, 16 May 2024 00:59:43 +0000 (UTC) X-FDA: 82122451446.26.DC16361 Received: from mail-oa1-f54.google.com (mail-oa1-f54.google.com [209.85.160.54]) by imf26.hostedemail.com (Postfix) with ESMTP id E021B140015 for ; Thu, 16 May 2024 00:59:39 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=Exp+3nwn; spf=pass (imf26.hostedemail.com: domain of jeffxu@chromium.org designates 209.85.160.54 as permitted sender) smtp.mailfrom=jeffxu@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1715821180; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to: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=FKByB1x2Ra+G7EvspZbKzTea5I7AEV+P8bKknO4K/Pw=; b=ea6FJgih+1YdKO4P8Gg9CI2jhwym+ZubAuVIRhJZEORbxFCFttrtVmZXJqSNZTR1E6MNQY SvHnAXBH4X3hG4wXpNAMW9R0b1k4pZAmDzHap+4Yk4EhfuDiRZ/xhkTCs0VZgXkv+8ntfK fSNG9spKTCu6EzW0//QVGFKOUD9Uy04= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=Exp+3nwn; spf=pass (imf26.hostedemail.com: domain of jeffxu@chromium.org designates 209.85.160.54 as permitted sender) smtp.mailfrom=jeffxu@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1715821180; a=rsa-sha256; cv=none; b=XQb1nIrKNMLzepBBu5ZnGjvVQ6YxjwGf+9uv4HIYaEKJJNubdfryfT7wJvqru1w529m9Aq Hgg6lkW0AgCegxEXU7g73VdZ0szozpVw1EL1BBS726ww6q6yTWI5QLCUTm5wqPRei6Yo3y 2X3Sj0RpeVdOIX9Hm2uQRoXIczrHarM= Received: by mail-oa1-f54.google.com with SMTP id 586e51a60fabf-23f9c2f1df6so70353fac.2 for ; Wed, 15 May 2024 17:59:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1715821179; x=1716425979; darn=kvack.org; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=FKByB1x2Ra+G7EvspZbKzTea5I7AEV+P8bKknO4K/Pw=; b=Exp+3nwnNHOjLFmyRKQdZWPiVmW7PFG014CkXc3Zw6kkwvVvL1ifoJot9UVtMSIuaz YkmY1pj4YmHyymBzljBZd85WmwbO6Wm0Qe46OgoAwN1ZGv4a+9sL0lQxWIOX+tLZesSO WJhZ7hH1hl6U9ZzFQfWhY59v8mxA+NmyeSEb4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715821179; x=1716425979; h=content-transfer-encoding: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=FKByB1x2Ra+G7EvspZbKzTea5I7AEV+P8bKknO4K/Pw=; b=p67jrpZEtlmgOFde4ns7O9bCgvdSAzlNd8Q9zxuY1j7ByEMOnM0BsVk0d7XaqMlK41 XWrBxoXBuMsbHyg6sSNBxTyLh7Vcq5+7Mt5XF9BbKwccUzU83Q8f7BfvWfJfDKNBcFAz W94ph/B/PQKui1+vVx+pLsjNy1O7UYHTeZKvOfeaSoTqntYWNeIM66Ge6tqGqgjzQNvi RtvNdrDol8QZaVSb/dICFQeSG2aUW9nKueZOH9cTSM0royqf22EqRh/ujx89t3Z7OkVf V+ucaFGKZdCbO9lrN2ElygOgogxHevbtac8ZkrC4+wUWc0uNtDR88bkWV0sZd7ibXoqd Qd2g== X-Forwarded-Encrypted: i=1; AJvYcCX2CoTwqRZsUsDkRZsZOmykN2mONJEVafkEgmXhcXUohdF1fXa1eV+gf6Sio2Y3sDuG+tVaHEvL7rjsWK8gyV3X1gQ= X-Gm-Message-State: AOJu0YxmFkiJfLCxHmP/L2BZZVltdJmQso5SzIos5NT1CgjiUhfgysWE H9fc1aoLaoJ1XeeLSp8hxHG+qDyGTYV+wlr5mvxaJ/f2yVMSnLUsYhUFuP9qy5ETBHiL0PLcOBM poiCegbsxEHsd/7UTrsWye8b8xixqHU4IMbf6 X-Google-Smtp-Source: AGHT+IFvkJEPxJ+yngArtr7AMa30DFdKcbfitHwSdnpWJQg42FEsfMf1OWA+u9hWOLZfCwrPgfJkJXFvANLzlkBxt8o= X-Received: by 2002:a05:6871:215:b0:23f:59fc:e509 with SMTP id 586e51a60fabf-24172a3d9edmr20301293fac.4.1715821178794; Wed, 15 May 2024 17:59:38 -0700 (PDT) MIME-Version: 1.0 References: <20240415163527.626541-1-jeffxu@chromium.org> <20240514104646.e6af4292f19b834777ec1e32@linux-foundation.org> <3rpmzsxiwo5t2uq7xy5inizbtaasotjtzocxbayw5ntgk5a2rx@jkccjg5mbqqh> In-Reply-To: <3rpmzsxiwo5t2uq7xy5inizbtaasotjtzocxbayw5ntgk5a2rx@jkccjg5mbqqh> From: Jeff Xu Date: Wed, 15 May 2024 17:59:27 -0700 Message-ID: Subject: Re: [PATCH v10 0/5] Introduce mseal To: "Liam R. Howlett" , Jeff Xu , Andrew Morton , keescook@chromium.org, jannh@google.com, sroettger@google.com, willy@infradead.org, gregkh@linuxfoundation.org, torvalds@linux-foundation.org, usama.anjum@collabora.com, corbet@lwn.net, surenb@google.com, merimus@google.com, rdunlap@infradead.org, jeffxu@google.com, jorgelo@chromium.org, groeck@chromium.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, pedro.falcato@gmail.com, dave.hansen@intel.com, linux-hardening@vger.kernel.org, deraadt@openbsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: E021B140015 X-Stat-Signature: rtqa7rnsqupsuqo6o1a33hwus54q93d6 X-HE-Tag: 1715821179-275199 X-HE-Meta: U2FsdGVkX1+2OZHhwU7Bv61yaTIjxrCbQgXwFrbFKak3MxafnAF+2riG8QcsHL7jiE2h0USxnF6gYNbVDdfj6a0V022RJ6ofGOcan7J6VBr+o5eYjtc46fuSSWOx33DOu92JWdqRAbmyzSmBo2PoK57NkQcNVqUZLLr0nWrKAwbRUWzWvEKo4HwKHe3MnXrx2+mZpD6ljXiRjaZXv4aI7pZQNOd1dKIreII4LbGjDJ6CSyjFtPSBGthPllefsaEPSukoB0HbdQFZRk2Rj0+5sXY1ruo5S5JZUc0whUTACbSqhZlxA9OtUzZQPhmalAMywaPjkrO7orC++A1IIwpamkyFOIj8d/P1DjSMjDbAG0HGUi52C9vffAgefe/o8W1cvFHckayaHsRVutCi55C84IUokhHKKj3xrkT4rV7K4GmB8v2AfkJk3FCH4Zf/9q6Pua5YxtWK9ob4HhDwuY3CEyZQrGMJXbub1NSCoXEVHH9vTG7pfGcibqeaT3k4QFCXgTEMcqWBu945GqIBM96jbg4JKiHZ8vJ6mE7Dy4GaP7DQrZs4AlC+2UyVEGS94EFOuQG5xGdVE/EaJG37cUFQWWxuFP+Fq+RcglTS0w1GTYVHjWTNChrTS8fbGI2aWf2QqZ7mMqNH1HkqwG2C8ilPj2e8nwgPvzh/Bch3y3AQnmLaQbdffPn4ratJZch1WFsEVYKRXZ2p67lUAwqAHxAg92h8vQA+cY4Smq+kUlfBb9Srudfm/F1p0i/us+h6iayAfo8vyXF8xPbrsjptw/lVEsrTGziUECe14TuhKZ5LOXpFiv7bz5qc4XKLY5EtF5dfXb8favWS9sSfrhM8mC4hSMbjoeDQv6DZaBOugpTPY5TBsirzu7yYV6WMOOArbFQcM1CFY/JepNc0Yha+s4O9dcEhFIce2dMOicMHYpCY4pkgMM18Ke+wvzQ2ZxB6jkdtRQjQD60hRVLZVvpp5Y0 v2+r1E2J n2XQxZaQ0I1oWXE4BsspIY9dh5A6M3quEPycjK96QcakVAbKIfn4Kr86/zpxr7mw88AzaMp6B0czQkBIffjjIq9QGeBDrPX74dM41Uu0kpnwLyDuQ+wYBs5g77qUOGr/EIvo2NRvy+IlE3zOx2E0RJzSyKbCZ3KnfLSLp9P+glKi8XANBdwzEh5NRTPaSgLZuRDmJH6iLXbw9dpYN5hBtWnGyMZiNtWhv6CKUFJ1dkH8CSnM3Q6rrs/joWHsXyadh9UtGpTwiE/bGzS9jH1BbVTg5V9UOSGo103MuIxj/keX/XzCMFMSafk+0VVHxLr3jCB61b+/OI0QKkiHJfRCOqwrMWz+g3wrwHFeeRvHWwW4VtLb5kXDlhSTOpT8NyRGUSBUd 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, May 15, 2024 at 3:19=E2=80=AFPM Liam R. Howlett wrote: > > * Jeff Xu [240515 13:18]: > ... > > > The current mseal patch does up-front checking in two different situati= ons: > > 1 when applying mseal() > > Checking for unallocated memory in the given memory range. > > > > 2 When checking mseal flag during mprotect/unmap/remap/mmap > > Checking mseal flag is placed ahead of the main business logic, and > > treated the same as input arguments check. > > > > > Either we are planning to clean this up and do what we can up-front, = or > > > just move the mseal check with the rest. Otherwise we are making a > > > larger mess with more technical dept for a single user, and I think t= his > > > is not an acceptable trade-off. > > > > > The sealing use case is different from regular mm API and this > > didn't create additional technical debt. Please allow me to explain > > those separately. > > > > The main use case and threat model is that an attacker exploits a > > vulnerability and has arbitrary write access to the process, and can > > manipulate some arguments to syscalls from some threads. Placing the > > checking of mseal flag ahead of mprotect main business logic is > > stricter compared with doing it in-place. It is meant to be harder for > > the attacker, e.g. blocking the opportunistically attempt of munmap > > by modifying the size argument. > > If you can manipulate some arguments to syscalls, couldn't it avoid > having the VMA mseal'ed? > The mm sealing can be applied in advance. This type of approach is common in sandboxer, e.g. setup restrictive environments in advance. > Again I don't care where the check goes - but having it happen alone is > pointless. > > > > > The legit app code won't call mprotect/munmap on sealed memory. It is > > irrelevant for both precheck and in-place check approaches, from a > > legit app code point of view. > > So let's do them together. > For the user case I describe in the threat-model, precheck is a better approach. Legit code doesn't care. > ... > > > About tech debt, code-wise , placing pre-check ahead of the main > > business logic of mprotect/munmap APIs, reduces the size of code > > change, and is easy to carry from release to release, or backporting. > > It sounds like the other changes to the looping code in recent kernels > is going to mess up the backporting if we do this with the rest of the > checks. > What other changes do you refer to ? I backported V9 to 5.10 when I ran the performance test on your request, and the backporting to 5.10 is relatively straight forward, the mseal flag check is placed after input arguments check and before the main business logic. > > > > But let's compare the alternatives - doing it in-place without prechec= k. > > - munmap > > munmap calls arch_unmap(mm, start, end) ahead of main business logic, > > the checking of sealing flags would need to be architect specific. In > > addition, if arch_unmap return fails due to sealing, the code should > > still proceed, till the main business logic fails again. > > You are going to mseal the vdso? > How is that relevant ? To answer your question: I don't know at this moment. The initial scope of libc change is sealing the RO/RX part during elf loading.e.g. .text and .RELO > > > > - mremap/mmap > > The check of sealing would be scattered, e.g. checking the src address > > range in-place, dest arrange in-place, unmap in-place, etc. The code > > is complex and prone to error. > > > > -mprotect/madvice > > Easy to change to in-place. > > > > - mseal > > mseal() check unallocated memory in the given memory range in the > > pre-check. Easy to change to in-place (same as mprotect) > > > > The situation in munmap and mremap/mmap make in-place checks less desir= able imo. > > > > > Considering the benchmarks that were provided, performance arguments > > > seem like they are not a concern. > > > > > Yes. Performance is not a factor in making a design choice on this. > > > > > I want to know if we are planning to sort and move existing checks if= we > > > proceed with this change? > > > > > I would argue that we should not change the existing mm code. mseal is > > new and no backward compatible problem. That is not the case for > > mprotect and other mm api. E.g. if we were to change mprotect to add a > > precheck for memory gap, some badly written application might break. > > This is a weak argument. Your new function may break these badly written > applications *if* gcc adds support. If you're not checking the return > type then it doesn't really matter - the application will run into > issues rather quickly anyways. The only thing that you could argue is > the speed - but you've proven that false. > The point I raised here is that there is a risk to modify mm API's established behavior. Kernel doesn't usually make this kind of behavior change. mm sealing is a new functionality, I think applications will need to opt in , e.g. allow dynamic linker to seal .text. > > > > The 'atomic' approach is also really difficult to enforce to the whole > > MM area, mseal() doesn't claim it is atomic. Most regular mm API might > > go deeper in mm data structure to update page tables and HW, etc. The > > rollback in handling those error cases, and performance cost. I'm not > > sure if the benefit is worth the cost. However, atomicity is another > > topic to discuss unrelated to mm sealing. The current design of mm > > sealing is due to its use case and practical coding reason. > > "best effort" is what I'm saying. It's actually not really difficult to > do atomic, but no one cares besides Theo. > OK, if you strongly believe in 'atomic' or 'best effort atomic', whatever it is, consider sending a patch and getting feedback from the community ? > How hard is it to put userfaultfd into your loop and avoid having that > horrible userfaulfd in munmap? For years people see horrible failure > paths and just dump in a huge comment saying "but it's okay because it's > probably not going to happen". But now we're putting this test up > front, and doing it alone - Why? > As a summary of why: - The use case: it makes it harder for attackers to modify memory opportunistically. - Code: Less and simpler code change. Thanks -Jeff > Thanks, > Liam