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 739B9C282EC for ; Fri, 14 Mar 2025 19:19:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3E3D3280002; Fri, 14 Mar 2025 15:19:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 39146280001; Fri, 14 Mar 2025 15:19:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 25A36280002; Fri, 14 Mar 2025 15:19:23 -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 0887B280001 for ; Fri, 14 Mar 2025 15:19:23 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 147E7B85AD for ; Fri, 14 Mar 2025 19:19:23 +0000 (UTC) X-FDA: 83221120206.21.E216920 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf22.hostedemail.com (Postfix) with ESMTP id 881C8C0015 for ; Fri, 14 Mar 2025 19:19:21 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=f4hW8fqz; spf=pass (imf22.hostedemail.com: domain of kees@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=kees@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741979961; 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=L8HEwx257TZkfYiM77D4/XTRPfWGXs9fPrf/aD8fovo=; b=l1rDO1RgUncCz2X7+Z7I3UmpuHPwG09bktfp4wHm+Kt4vtGXCs3K2rLpHdTmn7OgkKaNDJ UPf8shoUs3KlOq6kcPApGvso1QiNPoS1+aQepel1tgB4LA2hG0ZoBgSTXzV4QRJV22E35+ cD8S7GDl6hKqsAGEY6LmGtDdhXwzp+s= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=f4hW8fqz; spf=pass (imf22.hostedemail.com: domain of kees@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=kees@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741979961; a=rsa-sha256; cv=none; b=zNZEGPHuZR3aRj5Kr8dF2kC0Wf9pwdz1sqpQ8p/CfRJWq7MWJzyPhGKXuoZPkVQa0LDUjU CLEY/ElN7FjiWbJ0shnlzrymPAbhbiz2NExrmTiDZEXhFpdBOU8QLxZmvHvEOYTP3A7kJi yHQNVTvX0EFOOp9tMy1gFa1o9ClWcDU= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id EAEFDA47825; Fri, 14 Mar 2025 19:13:50 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5D8B1C4CEE3; Fri, 14 Mar 2025 19:19:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741979960; bh=cdENQzTHVATYN33v4Wg9T3aBAGHVrDM6ftggIFL/JAo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=f4hW8fqzwX6AKjw4Ii6ah7L31hUotS9mlxUS/lde+FAp6lsEZSBkHXVrgfQcMR/+i EO+JRcaFezJjX/cw8fTRK5VGMOZyFLxXEAluPxodrNbmCsfJuR/WyLp/V1Bv/+yEhe y4XiBmXbLRTpHRkvKritJ0axC79GfAv/uyYmfeTxPqYRXKmA1BvH079IFXiG1WBpQD wXrW0cygxVARz/EembfLs5R1NBlko2hXvT4kcQaSTtIQ4rFlKcVy0LcVVuffTMnNIH G1XEiHw6OnklERzz2XSXuvkNAdtfpJyOI3ZjYjYJZ9mpBp+0F4IWjsn0yHQnyWD3zY 0jo6caTOH4LNw== Date: Fri, 14 Mar 2025 12:19:17 -0700 From: Kees Cook To: Christophe Leroy Cc: Vlastimil Babka , Luis Chamberlain , Petr Pavlu , Sami Tolvanen , Daniel Gomez , Petr Mladek , Jani Nikula , Andrew Morton , John Ogness , Greg Kroah-Hartman , linux-mm , linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] module: Taint the kernel when write-protecting ro_after_init fails Message-ID: <202503141218.D638FB01@keescook> References: <20250306103712.29549-1-petr.pavlu@suse.com> <202503120923.199D458CB@keescook> <6e3ce71a-da5a-4d69-a5ea-4caca761d00f@csgroup.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6e3ce71a-da5a-4d69-a5ea-4caca761d00f@csgroup.eu> X-Stat-Signature: ogqqssfp7f47kbwxtz9fs38wgtjqexfk X-Rspamd-Queue-Id: 881C8C0015 X-Rspam-User: X-Rspamd-Server: rspam09 X-HE-Tag: 1741979961-918580 X-HE-Meta: U2FsdGVkX18SpkZw/hcG6IBLXzSPkpOi/9WWX7SpIp9T5Di/9VABRS1ukXrGcq3FpgQldP65c/sd7nmowp2Bk0giRTQEiqmkpAZ7qqKoZQzCswei/TjsHRzUGz/rmKdNhO40qUd1eViz9er/KxOjMMvS+bOQnRXA+X2pXBsumT8w/aIBjMrKbIWhUFqREssQjuW+M6b60Lnokg8SEW2QjR6Su/o2o9EwI/27O7tJ1UNEFdMVxtlPFoFhlSAGTnN9zBJsNL2yfJ+NI4UD3oNPjbTJaOFCRl2ZMMDeWJOcZIuj9Ry3Hx/DalgjOMPCxK91LApZr3MHZQorW5vdhPbNlQXZLFR/+eM+yJxPIhU2txaRjofyUMET/VwBT+gXesxGS/3KmyND4sbrWQie2MDec/TrLhtQPcKpXXs6BIe6V+mMJe6bzkpqHEUgRHsZXuSkx1vVwaqtcIsL77Dcjouvk3JKu0lLVJWS8raJ9CSUnfKbHjUDBDp4dag37FMwTEtALkUkjhGlV0yNntDaxMENq38tDfSEl6KMTsbTxM8g/EmzxnUWGtKCba6xIuVcH1TUtBr47bC2Kh+PSAdTdQxCgQyVTpeZvRx9DokVc9mGFp7oArKdffM5QEmFHEx4lbD+UVCkuOVBFsHqoK5AYc9TcPViSBap5bD2Tnc6kQj1qgtwPdm3NA3421YYj86HGWuA/kgM8Tmxqb3vz2iHyV733JoYJLTPxXS6KLpI6WqYRch59Q/RdEKCa9dyI71JORaSxM4IxyHmbqH4PYf915nRqWXdqPaohrzb8AyzjQNKWFZ337dmrwzebzcT1ThJAX3+C+VCh8uhaNfc6b/Wb4DaC7njSpavito7flLB0tja1n+KaWD+7c/snC687B03Nx/RTGA+28JwwK/nbtOWprX3X8S515eNfh2Uqa2dEIIfVo3wDrchr5gAXf07tIW5Ht14zGU434eJnEmhcBtlSw1 e6h/xtDF jhrvz 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 Fri, Mar 14, 2025 at 05:48:00PM +0100, Christophe Leroy wrote: > > > Le 12/03/2025 à 17:30, Kees Cook a écrit : > > On Wed, Mar 12, 2025 at 04:45:24PM +0100, Vlastimil Babka wrote: > > > On 3/6/25 17:57, Luis Chamberlain wrote: > > > > + linux-mm since we're adding TAINT_BAD_PAGE > > > > > > > > On Thu, Mar 06, 2025 at 11:36:55AM +0100, Petr Pavlu wrote: > > > > > In the unlikely case that setting ro_after_init data to read-only fails, it > > > > > is too late to cancel loading of the module. The loader then issues only > > > > > a warning about the situation. Given that this reduces the kernel's > > > > > protection, it was suggested to make the failure more visible by tainting > > > > > the kernel. > > > > > > > > > > Allow TAINT_BAD_PAGE to be set per-module and use it in this case. The flag > > > > > is set in similar situations and has the following description in > > > > > Documentation/admin-guide/tainted-kernels.rst: "bad page referenced or some > > > > > unexpected page flags". > > > > > > > > > > Adjust the warning that reports the failure to avoid references to internal > > > > > functions and to add information about the kernel being tainted, both to > > > > > match the style of other messages in the file. Additionally, merge the > > > > > message on a single line because checkpatch.pl recommends that for the > > > > > ability to grep for the string. > > > > > > > > > > Suggested-by: Kees Cook > > > > > Signed-off-by: Petr Pavlu > > > > > --- > > > > > I opted to use TAINT_BAD_PAGE for now because it seemed unnecessary to me > > > > > to introduce a new flag only for this specific case. However, if we end up > > > > > similarly checking set_memory_*() in the boot context, a separate flag > > > > > would be probably better. > > > > > --- > > > > > kernel/module/main.c | 7 ++++--- > > > > > kernel/panic.c | 2 +- > > > > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/kernel/module/main.c b/kernel/module/main.c > > > > > index 1fb9ad289a6f..8f424a107b92 100644 > > > > > --- a/kernel/module/main.c > > > > > +++ b/kernel/module/main.c > > > > > @@ -3030,10 +3030,11 @@ static noinline int do_init_module(struct module *mod) > > > > > rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); > > > > > #endif > > > > > ret = module_enable_rodata_ro_after_init(mod); > > > > > - if (ret) > > > > > - pr_warn("%s: module_enable_rodata_ro_after_init() returned %d, " > > > > > - "ro_after_init data might still be writable\n", > > > > > + if (ret) { > > > > > + pr_warn("%s: write-protecting ro_after_init data failed with %d, the data might still be writable - tainting kernel\n", > > > > > mod->name, ret); > > > > > + add_taint_module(mod, TAINT_BAD_PAGE, LOCKDEP_STILL_OK); > > > > > + } > > > > > mod_tree_remove_init(mod); > > > > > module_arch_freeing_init(mod); > > > > > diff --git a/kernel/panic.c b/kernel/panic.c > > > > > index d8635d5cecb2..794c443bfb5c 100644 > > > > > --- a/kernel/panic.c > > > > > +++ b/kernel/panic.c > > > > > @@ -497,7 +497,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = { > > > > > TAINT_FLAG(CPU_OUT_OF_SPEC, 'S', ' ', false), > > > > > TAINT_FLAG(FORCED_RMMOD, 'R', ' ', false), > > > > > TAINT_FLAG(MACHINE_CHECK, 'M', ' ', false), > > > > > - TAINT_FLAG(BAD_PAGE, 'B', ' ', false), > > > > > + TAINT_FLAG(BAD_PAGE, 'B', ' ', true), > > > > > TAINT_FLAG(USER, 'U', ' ', false), > > > > > TAINT_FLAG(DIE, 'D', ' ', false), > > > > > TAINT_FLAG(OVERRIDDEN_ACPI_TABLE, 'A', ' ', false), > > > > > > > > Reviewed-by: Luis Chamberlain > > > > > > > > For our needs this makes sense, however I am curious if TAINT_BAD_PAGE > > > > is too broadly generic, and also if we're going to add it, if there are > > > > other mm uses for such a thing. > > > > > > I'm not sure BAD_PAGE is a good fit. If there was a new flag that meant "a > > > hardening measure failed", would that have other possible uses? The > > > semantics would be that the kernel self-protection was weakened wrt > > > expectations, even if not yet a corruption due to attack would be detected. > > > Some admins could opt-in to panic in such case anyway, etc. Any other > > > hardening features where such "failure to harden" is possible and could use > > > this too? Kees? > > > > Yeah, it could certainly be used. The direction the hardening stuff has > > taken is to use WARN() (as Linus requires no direct BUG() usage), and to > > recommend that end users tune their warn_limit sysctl as needed. > > > > Being able to TAINT might be useful, but I don't have any places that > > immediately come to mind that seem appropriate for it (besides this > > case). Hm, well, maybe in the case of a W^X test failure? (I note that > > this is also a "safe memory permission" failure...) > > Can be anything that fails in function mark_readonly() ? : > > jump_label_init_ro(); > mark_rodata_ro(); > debug_checkwx(); > rodata_test(); Yeah, works for me! -- Kees Cook