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 C91B4C02192 for ; Fri, 7 Feb 2025 04:52:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3BE306B0083; Thu, 6 Feb 2025 23:52:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 36D586B0085; Thu, 6 Feb 2025 23:52:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 234ED6B0088; Thu, 6 Feb 2025 23:52:52 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 05D4D6B0083 for ; Thu, 6 Feb 2025 23:52:51 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 87EDC4C232 for ; Fri, 7 Feb 2025 04:52:51 +0000 (UTC) X-FDA: 83091928542.01.6CD3EC5 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf17.hostedemail.com (Postfix) with ESMTP id 6BA7140003 for ; Fri, 7 Feb 2025 04:52:34 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=J4hOTD94; spf=pass (imf17.hostedemail.com: domain of kees@kernel.org designates 139.178.84.217 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=1738903969; 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=6eVP2G6SlyKBrZMvJThe1P6q6E3wV599K7FNUTSvEDg=; b=6SrjFxkzKmLgbF0F36MJ7lg2tC6SbZUa5GjqG80Ja9ajmQqIox8JBCn+qYgWA5GzbM0A1W R+DEeZmIk6nhmHfRIGGU8H41vI4Uu1/hlPFRLDWhGNmUQMrbmPTRhWLLu63L7qJYRclro3 iKcR/Y7LpXr95D/kc3kpy2S9YbWSg50= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=J4hOTD94; spf=pass (imf17.hostedemail.com: domain of kees@kernel.org designates 139.178.84.217 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=1738903969; a=rsa-sha256; cv=none; b=n+sWvH5+fDxvo81Vcvi1N86r5zaI5+lpRkfid9wYX+xWQtyDMv5L72WOHV/xQUBap8s8ve D9GG/bQob7P0onnyXrJgGBQK8JvvDjhaXh2hofeSpJpIPmHpeZ4RqXcgLyMXwIG4cNMU7b LhNVmD+DlyBeEde6fSpUfYfmDuzavpg= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 6C94D5C5663; Fri, 7 Feb 2025 04:51:53 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C1AC9C4CED1; Fri, 7 Feb 2025 04:52:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1738903952; bh=DHjArMeAK3MeqFPs8ewPHAdYTmkFt/+F2+q7rMjVEsQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=J4hOTD940F1xlXDm0mc5K3mDWMF8PSxayTzbGZ7yFgncZQhreabSjsW/SPkrxvWyj nSL6ezpoAY094IRO9xVo7ABLzhV1eguxz++lt/TzXCZ8K45nVXR4iXLQO5O5OvWp0K ATK9xHVbtmFdnug59ERW+h66BXmzxYNeXv29CdAsmMdv00eYEe0sd6vVofADQf05te lT0NWx4WrW5hPsI8JfAq6/8JcdwZWb3xVM2NWlDEGMKosdaJAlWafYQrvED6iGX8qh kmzhADTz8zfNueFUjkLH68uqsQf0kpB9zw+24EnTrALzamINKTukN0O7a9JCGIPQDt St5IHezNhourg== Date: Thu, 6 Feb 2025 20:52:32 -0800 From: Kees Cook To: Kevin Brodsky Cc: linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org, Andrew Morton , Mark Brown , Catalin Marinas , Dave Hansen , David Howells , "Eric W. Biederman" , Jann Horn , Jeff Xu , Joey Gouly , Linus Walleij , Andy Lutomirski , Marc Zyngier , Peter Zijlstra , Pierre Langlois , Quentin Perret , "Mike Rapoport (IBM)" , Ryan Roberts , Thomas Gleixner , Will Deacon , Matthew Wilcox , Qi Zheng , linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, x86@kernel.org Subject: Re: [RFC PATCH 8/8] mm: Add basic tests for kpkeys_hardened_cred Message-ID: <202502062024.BCB0DED1D5@keescook> References: <20250203102809.1223255-1-kevin.brodsky@arm.com> <20250203102809.1223255-9-kevin.brodsky@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250203102809.1223255-9-kevin.brodsky@arm.com> X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 6BA7140003 X-Stat-Signature: czoqqzsbi4r5ax6cbm54a4rhej7k7uwr X-HE-Tag: 1738903954-994501 X-HE-Meta: U2FsdGVkX1/EMmN2RgCi8YQf3hs9lu1X4dFd09CFPuOmcjGkbuOAKvvdPFPTSDjRqsZghVppJm+TnCF1ab24kMza/STnJJMaFY/GMdoi09iphyW2j8cXOXxBPXcUqUpXOnv4B/lGaRnw9U2ckpDn1w3UmJ6gy8/K6tmHU+3LERYYlxLz6+C6E2n6fuBa5LLEvO/604R0aheCRaVAGojiB/PRs+eotER/37R0bGZ9w2zmdGNjo9GVotYIXUeAzB0saIw1apaLrjd3B8+SKuJ+4d09ckSf2cwCoJF76thcOkOA3qxKPiUsN5bRSx8LlOxTpLRRmzcxffZE2v83+HAaLxfWIpu2eeMEhD+41O6qizvFt7uZIodkNViACllLL4VFKaqKJ/Wyj6b/jJ6SXG55pnmYQkNKz3noEeopt9p29Gv3K2r7gb0yipAg5xc6WMs9oekMiC4bex2kd6mnJZaSu8+bn7iq9WTr+qhX+r4jWmrEJF3stecqbIiy2229w6NqWJzHSgwZjVnYg3sXteRvATenbe3rwQQJf8OGnkoUEJXR2mpm1bWveIs26NcXkXBOjRqsclmVLbPAmCf4Tjald1yg6OwqeTcnh0hcJ15SKOZTGHCh+Mh/6xgbz5bZdBgAT2ba2R9guCB3PJaV8qgLrII2u3+N3b/xo8dh0Mq0nt2XAkwf84HznCRDDoouuQ8DDJHFZEQUpDVk5ZvehO+73Yce0ItcPEJ7X6AMkPzHUYmVTmUP5O9KrBcv+8DWnpbDspKTfM18G32ALQW6PFGC279HEMV3D840um66lTWrDhnYTaUK/0Um/BHCi5OMkCWKrp7V1RP2RFp0E4dGkk/NIFkmTBW0X0dG7IyTgYi7Gtwm7HgglT4czyJP4XCPkP9c5MseMDU7JAAGuuO6KW+bHJCzFJcPF3gH5HLJcXavXL20ZhO3RTl8Q/5SZvJSwOl/XQL2/qLceggovYP9f4V M5nFJmPa qX/IWrhYcJhbifZYeEI0VXWJXsKPYUp2vByaKpUIQQNaHPvoFTnpbOmLSXPdYcB3ft3pPYYuYPZCf1npZ1LK7yp/O3D9I4lxD2S3mCaXd1prJ5MOuaG8p7DZH6xMFrTKyQJGCqDgdB7h4VPOYC8I9mPLlFlpMycfg8t1sCDWtdbLfGlCHUtgRYid7FIEr1fynrm/M8+GVUNwZsuQlPpXZNfgMeVxP+zxDD7yQsW0v44NUPIQ25pkamgkau7AX9PMaxDncFmdDXwGK3MLkxb9GNqQ5uVMBtz2duORLFxjS8OX7BsYPbQCWT787JC9MdS8pu+W7cNMFoCVpHcoxmOhRytpnaQ4fVpXnSnLXAlINs0n5jKdbNGb/QIDg7tpTEgFozXsgJOPdPhhK+McAaQlD6kDAuA== 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 Mon, Feb 03, 2025 at 10:28:09AM +0000, Kevin Brodsky wrote: > Add basic tests for the kpkeys_hardened_pgtables feature: try to > perform a direct write to current->{cred,real_cred} and ensure it > fails. > > Signed-off-by: Kevin Brodsky > --- > mm/Makefile | 1 + > mm/kpkeys_hardened_cred_test.c | 42 ++++++++++++++++++++++++++++++++++ Current file naming convention[1] would be to name this as: mm/tests/kpkeys_hardened_cred_kunit.c > security/Kconfig.hardening | 11 +++++++++ > 3 files changed, 54 insertions(+) > create mode 100644 mm/kpkeys_hardened_cred_test.c > > diff --git a/mm/Makefile b/mm/Makefile > index f7263b7f45b8..2024226902d4 100644 > --- a/mm/Makefile > +++ b/mm/Makefile > @@ -149,3 +149,4 @@ obj-$(CONFIG_TMPFS_QUOTA) += shmem_quota.o > obj-$(CONFIG_PT_RECLAIM) += pt_reclaim.o > obj-$(CONFIG_KPKEYS_HARDENED_PGTABLES) += kpkeys_hardened_pgtables.o > obj-$(CONFIG_KPKEYS_HARDENED_PGTABLES_TEST) += kpkeys_hardened_pgtables_test.o > +obj-$(CONFIG_KPKEYS_HARDENED_CRED_TEST) += kpkeys_hardened_cred_test.o And for the Kconfig convention says[2] this should be: CONFIG_KPKEYS_HARDENED_CRED_KUNIT_TEST > diff --git a/mm/kpkeys_hardened_cred_test.c b/mm/kpkeys_hardened_cred_test.c > new file mode 100644 > index 000000000000..46048098f99d > --- /dev/null > +++ b/mm/kpkeys_hardened_cred_test.c > @@ -0,0 +1,42 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#include > +#include > + > +static void write_cred(struct kunit *test) > +{ > + long zero = 0; > + int ret; > + > + ret = copy_to_kernel_nofault((unsigned long *)current->cred, &zero, sizeof(zero)); > + KUNIT_EXPECT_EQ_MSG(test, ret, -EFAULT, > + "Write to current->cred wasn't prevented"); > + > + ret = copy_to_kernel_nofault((unsigned long *)current->real_cred, &zero, sizeof(zero)); > + KUNIT_EXPECT_EQ_MSG(test, ret, -EFAULT, > + "Write to current->real_cred wasn't prevented"); This is a good negative test. I would include a positive test as well. i.e. make sure you can run copy_from_kernel_nofault() to read it successfully. Otherwise you don't know if you're just getting a bad address -- we want to distinguish between them. (This is more true for the next suggestion, since current->cred being broken would be much more obvious.) While current->cred is good and easy, I would like to see prepare_creds() exercised too to get a new cred and validate that it is equally directly readable and directly not writable, and then use the correct accessors to perform a successful write to the cred, read back the change, etc. (i.e. validate the expected behavior too.) > +} > + > +static int kpkeys_hardened_cred_suite_init(struct kunit_suite *suite) > +{ > + if (!arch_kpkeys_enabled()) { > + pr_err("Cannot run kpkeys_hardened_cred tests: kpkeys are not supported\n"); > + return 1; > + } Instead of failing ("return 1") I think this should be a "skip" (it is expected to not work if there is no support) in each test instead: if (!arch_kpkeys_enabled()) kunit_skip(test, "kpkeys are not supported\n"); I'm very happy to see tests! :) -Kees [1] https://docs.kernel.org/dev-tools/kunit/style.html#test-file-and-module-names [2] https://docs.kernel.org/dev-tools/kunit/style.html#test-kconfig-entries -- Kees Cook