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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 71C27C433EF for ; Sun, 20 Mar 2022 21:05:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343650AbiCTVGq (ORCPT ); Sun, 20 Mar 2022 17:06:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59686 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233463AbiCTVGp (ORCPT ); Sun, 20 Mar 2022 17:06:45 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E75C859384; Sun, 20 Mar 2022 14:05:20 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 5E67861236; Sun, 20 Mar 2022 21:05:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 12504C340E9; Sun, 20 Mar 2022 21:05:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1647810319; bh=UIufJWZ3MuBNRWZitDo5bz/ICqU3OT4ma2pqcVW9quo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Q/YeSF99s3ptwPI2JMC5i19K6BE9RKpPZCjEraQQFdLdF1yFErptPiX4LyqCjR0XY 7DcaRXSl7+GUAMWl2Xm6kdH871vCtcX+w6+NjWIhSDmuTpqar7w+msnNfTvP6vp6Qi FXNdEcgQQg4Z2mAY0wOdgj0xqvqkY4wWEN+CUDJ71UpxeU6Czh+pMaooES38gqLyaA ztYpE5Al7tUAVon+i6EUJAOzcuK/uGjuev5ubk0dmwDyrf4oHho7/1DgOgnD50nBSZ wROO4RunAl2zBWHWS93Wq5dRxLOfUNsxZ6jT9GBo5dOCE1+aUQOw7dxq3ESuHTzSeI SsMIj8xp5O9YQ== Date: Sun, 20 Mar 2022 23:06:20 +0200 From: Jarkko Sakkinen To: =?iso-8859-1?Q?Micka=EBl_Sala=FCn?= Cc: David Howells , David Woodhouse , "David S . Miller" , Eric Snowberg , =?iso-8859-1?Q?Micka=EBl_Sala=FCn?= , Paul Moore , keyrings@vger.kernel.org, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 2/2] certs: Remove panic() calls from system_trusted_keyring_init() Message-ID: References: <20220311174741.250424-1-mic@digikod.net> <20220311174741.250424-3-mic@digikod.net> <57a2e3ef-5baa-16ef-7865-245134a26e25@digikod.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <57a2e3ef-5baa-16ef-7865-245134a26e25@digikod.net> Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org On Thu, Mar 17, 2022 at 09:30:02AM +0100, Mickaël Salaün wrote: > > On 17/03/2022 08:36, Jarkko Sakkinen wrote: > > On Fri, Mar 11, 2022 at 06:47:41PM +0100, Mickaël Salaün wrote: > > > From: Mickaël Salaün > > > > > > Replace panic() calls from device_initcall(system_trusted_keyring_init) > > > with proper error handling using -ENODEV. > > > > > > Suggested-by: Jarkko Sakkinen [1] > > > Link: https://lore.kernel.org/r/Yik0C2t7G272YZ73@iki.fi [1] > > > Signed-off-by: Mickaël Salaün > > > Link: https://lore.kernel.org/r/20220311174741.250424-3-mic@digikod.net > > > --- > > > certs/system_keyring.c | 26 ++++++++++++++++++++------ > > > 1 file changed, 20 insertions(+), 6 deletions(-) > > > > > > diff --git a/certs/system_keyring.c b/certs/system_keyring.c > > > index 05b66ce9d1c9..428046a7aa7f 100644 > > > --- a/certs/system_keyring.c > > > +++ b/certs/system_keyring.c > > > @@ -148,8 +148,10 @@ static __init int system_trusted_keyring_init(void) > > > KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH), > > > KEY_ALLOC_NOT_IN_QUOTA, > > > NULL, NULL); > > > - if (IS_ERR(builtin_trusted_keys)) > > > - panic("Can't allocate builtin trusted keyring\n"); > > > + if (IS_ERR(builtin_trusted_keys)) { > > > + pr_err("Can't allocate builtin trusted keyring\n"); > > > + return -ENODEV; > > > + } > > > #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING > > > secondary_trusted_keys = > > > @@ -161,14 +163,26 @@ static __init int system_trusted_keyring_init(void) > > > KEY_ALLOC_NOT_IN_QUOTA, > > > get_builtin_and_secondary_restriction(), > > > NULL); > > > - if (IS_ERR(secondary_trusted_keys)) > > > - panic("Can't allocate secondary trusted keyring\n"); > > > + if (IS_ERR(secondary_trusted_keys)) { > > > + pr_err("Can't allocate secondary trusted keyring\n"); > > > + goto err_secondary; > > > + } > > > - if (key_link(secondary_trusted_keys, builtin_trusted_keys) < 0) > > > - panic("Can't link trusted keyrings\n"); > > > + if (key_link(secondary_trusted_keys, builtin_trusted_keys) < 0) { > > > + pr_err("Can't link trusted keyrings\n"); > > > + goto err_link; > > > + } > > > #endif > > > return 0; > > > + > > > +err_link: > > > + key_put(secondary_trusted_keys); > > > + > > > +err_secondary: > > > + key_put(builtin_trusted_keys); > > > + > > > + return -ENODEV; > > > } > > > /* > > > -- > > > 2.35.1 > > > > > > > Changes make sense to me but you should implement all this to the original > > patch set. > > You agreed to add this patch on top of the others a few days ago: https://lore.kernel.org/r/f8b1ea77afe8d6698b4a2122254ff8be310412b1.camel@kernel.org > > What do you think about Paul's concerns? Yes, but I missed this part. I think the right call would be to include Paul's concerns documented. BR, Jarkko