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 E9D58C282D1 for ; Thu, 6 Mar 2025 16:57:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 715DB280003; Thu, 6 Mar 2025 11:57:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6A09A280001; Thu, 6 Mar 2025 11:57:25 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 53FD1280003; Thu, 6 Mar 2025 11:57:25 -0500 (EST) 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 32023280001 for ; Thu, 6 Mar 2025 11:57:25 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 61F5E1CCBB1 for ; Thu, 6 Mar 2025 16:57:25 +0000 (UTC) X-FDA: 83191732050.25.012032D Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf12.hostedemail.com (Postfix) with ESMTP id B609C40006 for ; Thu, 6 Mar 2025 16:57:23 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="a/zKuPZt"; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf12.hostedemail.com: domain of mcgrof@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=mcgrof@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741280243; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=7srWv7xcUSzl8W9TJ3dMoVn4dZLUVmsjyRauqp2pg6s=; b=Q+Rt0D86GRoZ33kIjanqqNJpv4WVJO5RC1Ag5iz5pStgsszsV4pPrVQ6oRlcjKRrDm+B6R ckHK74IjBJaDSH6MVRzUjwztdoRBr8oOGJE9pOm3C8yhcYQITqhgnrueIG2dGoFLQQ55+a XVaAroXT7kh/WiXI01Q53W4nwG6G35I= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="a/zKuPZt"; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf12.hostedemail.com: domain of mcgrof@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=mcgrof@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741280243; a=rsa-sha256; cv=none; b=f0KEuVfc6IaNAdno8rap2GxMpcC+EKljtu80IZpVCkd05keTDkmCIx6ekHTntd7kcCo/iO YH8w2kzyYfeY96VnXhohTKk8O6R6u+O/4QEInQLeJP08Fi+hTooO078BNScQFOLUA9G+bD 2MXNGCS/YG0XY/7OY01z0nFzAbhssGY= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id D1C195C58CF; Thu, 6 Mar 2025 16:55:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 03E35C4CEE0; Thu, 6 Mar 2025 16:57:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741280242; bh=aJZfEajPBD2iDdwcbtY5Avx4D4TADjWaPWTqb27CuZk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=a/zKuPZtB/q7WJWachcbscNMt2RpxJbhxrweb9D3AFQ/J7a9xhz/lAS2i/IK2RuEO dM+XbQKheP5Epwae9QZ0ElwT6NH3EOYRqBZZlj4alZ1wGwH2jEuAFBU0x9/3ujuII3 SSI6bB8f5xLQ3JfurvV3sk2ftg7vZqY7wWl66KlPIQRPg0Dl06P99XbDBwgA41Nw87 nk2q6mEcSIEXKKD7mqrg66FA7z8tFQM7oNXx5dymWmzxYHw2iasdIUluCpEPuGPoim bbFLeodMSiPJSJPCUCpg8mule2+aswhuVJubxyqf+kPMNI5q2+rN8cPv6/X179U0pn fgj067cXZm8nw== Date: Thu, 6 Mar 2025 08:57:20 -0800 From: Luis Chamberlain To: Petr Pavlu Cc: Sami Tolvanen , Daniel Gomez , Kees Cook , 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: References: <20250306103712.29549-1-petr.pavlu@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250306103712.29549-1-petr.pavlu@suse.com> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: B609C40006 X-Rspam-User: X-Stat-Signature: othuag4bpj655ibzsy99bfpzubseagh9 X-HE-Tag: 1741280243-427881 X-HE-Meta: U2FsdGVkX1/nvB84ENpuWQmEDmkmfXta5VZhrjQhSOjCywmcntKfVrqX3GXd5ySjb+6j3+4BFygqfHTL4adbjT5tKhc1huhMsHtZui8Ge5EKiPLhYaU86hPLRHwuaFmQh1RqK1YP5Q7ds3fXYkceyam+Fb2AN1p+/dU4llND+iFcVu8zPiF6hDsF2waiIChTP/AIOO8BEru9fYwmhdT2m/yULF4IUScgqf4oWah9nus02Uzf9Fi+J/FllZhspbnBnULG1rIF8nW/+hl9b1e3qHnDO3/GINpR2gRXqQiCir15kmcqR75KAcC8clvbQJ0tyuKOx5jumbNqSMm25pgxAR0pvd2FS1yQbMT9R4bQ+tTzMEF1XcwoK7qPvaH96orsKBvztGBuoEI7g+v5dH2Dq79wW7GbonGZ2Qz7dWVh9hTtNMzbbGwTlxh+2QcHby4zct9nY5huY22C9Wd8oSEoZTAil+y40hdFN6VsUIKqnFRH1zdtDcbqGRc07VfutadnyoGBpO/AbjRnOj/ykGYfsILD68GSl1UerGwP3AVLct2KHS69gXmdTqsnD8Bellvo6FI5M0K6VvbYACEoFheLpGLKCqEyAlKQyuPOx77K/tDy6lhyqL8pr52PIBikE35Qs1zyfdjf4jxaOxVluxBSS4xwchouYXhvkhJ+HbhPjgKvxNMgG+z6s2PX6ccDzuaT2lTQrZD2kKHrGwogj6FOO/W1N4/4a3KIA8T1+e669uuVGBSz1nLU5oY7aVWugOc2OEao85x7fF42W+VTilw10zaGUeZuHJEdPvNiMqKCr1cg5DtG+8nE45aNW/z0BMpGsFEH0U1EaOMP/NmpoBfgtdMmf81smnQLfiwfQphinOwWJVZbiZozwCKSuQhe3WxDr6AYbygA9zRxYhx0re0uEuFhDRWX3sT41B8LfRPepJSdiMmFTpMg79Vg83ueWOeP4RuwCvdeUVaiNiMoo62 FZezalB4 kvvIGG2H2GU7HW+cXwyapq+2tNSQpgGbZmsHGcEzYyPEQoWWf0eIhd8IFffku+YGsyzZFUjia1EUKojH/qibtwSg6bjKHRkJZxC1oWU3RahqoHDH97g0jnA075eAuioXfB20+TABnt4+nuLXqIIWLJiW+6bC7jwqCRz3FhtwJado1vzEp0rQqGF+9ZHXBU5WOzyL7oTjqv8D39PALZt3f5FTKDsHnHv++c2DV3NeHphit/UuZTExgMpfSUApLKyQh01NQ339c1zPqpkjTNZnpwmA32vgKm8Li49jqacJUpEIWBWPIaaSOEcXFZge4RAN6NZLDug7dCZrT8xpDA8Ha+DxebSZ3BTIfY34F 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: + 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. Luis