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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 54D09C433EF for ; Tue, 7 Jun 2022 10:59:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=2zBaVcJfsD3l8SQItdsNIBRuixJlJFY0lyKUNNgvva8=; b=ymDj6bmlzRCp+E CbeDLR0sAwYX0Qegf6DRQEQXuZb2pLX5LDwpEZt3W67RgKsJfikCaFrZB3dhLMgni2KR9kHU79OEm o80KR+z6XYvD6kt1Xgaq46ny9rOMM7m54ZSNnv0CHXKwMeKn1m6q89ppeeB+q5TaRi+h+RTNB8Fj5 GG1hSc5vAM6igXhg0VsXeV4J7zDLvMko7BPIENgED8LzyY2BvgXXEvJABbIrzLZPfPV6mWzMP1Fjm EQUlMOBLutPDSksv0Z1NsaAyoOfUd4E8ZRoSROJzeEJNfDC+98VRNUwxGEAlKXQ84F2UarfKSkNL1 k/IXF2hz3U5uuH0+H5Jw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nyWuD-006sdz-5t; Tue, 07 Jun 2022 10:57:49 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nyWt3-006s7e-Uf for linux-arm-kernel@lists.infradead.org; Tue, 07 Jun 2022 10:56:40 +0000 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 ams.source.kernel.org (Postfix) with ESMTPS id 4BF57B81F03 for ; Tue, 7 Jun 2022 10:56:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F3ED0C36AFE for ; Tue, 7 Jun 2022 10:56:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1654599395; bh=NKzuHbkieQxLoWjyC1x/EN9NN+GQBUg1WX2wv6pDfL0=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=tkwxPVfAhH7CJ3DSFc01umCeI2NG+rftGJwas/UMMMez7/ZlC3cEluAX1PLeEVudf 4FsBYlxa/NHyEzWeOL8Myvdp2JvDGOICW4qNhaaXSyHScz7P7eOdJSnp2HMlV9Kuaf aCJa+IYKmwn/CwE9VDRI7WUo/rJPWErQCIhCEGW7Lik9BzMeH3HbvY5lYq1lAphKFA 5HJWLULzrg7Z4gGPs9pels7R+3ichqOwAmtb4TJzD3Y21Sq3lpYyBHJS2iUYzQVa5v 5rVA8dch2eiKOcKJvL5IvBl0fyJPExCCT5qRTpZQ3hXcBvKHIvyBMUL3h0QfKE0/a7 VjP3vr3vbnvag== Received: by mail-oa1-f54.google.com with SMTP id 586e51a60fabf-f16a3e0529so22699156fac.2 for ; Tue, 07 Jun 2022 03:56:34 -0700 (PDT) X-Gm-Message-State: AOAM533fzstWpRgQM+dr8SmlFqNhlTpJbQ7gKFcJMBt0f2l0A7IxwDNB 4ooYIscokj0kxXOe+ZJCzXSID/Q/mGOtwrRn6ME= X-Google-Smtp-Source: ABdhPJxhYGdGLx2yHSyMyGxBsWpnSLuLp5S3Cd5eUQSa1FMAKtjD0WGuvymfVc1RLRcZHETjPDQyKen+Rk/IFcqTrOk= X-Received: by 2002:a05:6871:5c8:b0:f3:3c1c:126f with SMTP id v8-20020a05687105c800b000f33c1c126fmr16121139oan.126.1654599394092; Tue, 07 Jun 2022 03:56:34 -0700 (PDT) MIME-Version: 1.0 References: <20220607100210.683136-1-Jason@zx2c4.com> In-Reply-To: From: Ard Biesheuvel Date: Tue, 7 Jun 2022 12:56:20 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] random: do not use jump labels before they are initialized To: "Jason A. Donenfeld" Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Stephen Boyd , Catalin Marinas , Russell King , Arnd Bergmann , Phil Elwell X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220607_035638_360209_27AA8547 X-CRM114-Status: GOOD ( 52.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 7 Jun 2022 at 12:42, Jason A. Donenfeld wrote: > > Hey again, > > On Tue, Jun 07, 2022 at 12:28:08PM +0200, Jason A. Donenfeld wrote: > > Hi Ard, > > > > On Tue, Jun 07, 2022 at 12:13:29PM +0200, Ard Biesheuvel wrote: > > > Hi Jason, > > > > > > On Tue, 7 Jun 2022 at 12:04, Jason A. Donenfeld wrote: > > > > > > > > [ I would like to pursue fixing this more directly first before actually > > > > merging this, but I thought I'd send this to the list now anyway as a > > > > the "backup" plan. If I can't figure out how to make headway on the > > > > main plan in the next few days, it'll be easy to just do this. ] > > > > > > > > > > What more direct fix did you have in mind here? > > > > A non-broken version of https://lore.kernel.org/lkml/20220603121543.360283-1-Jason@zx2c4.com/ > > > > As I mentioned in https://lore.kernel.org/lkml/Yp8kQrBgE3WVqqC5@zx2c4.com/ , > > > > I would like a few days to see if there's some trivial way of not > > needing that on arm32. If it turns out to be easy, then I'd prefer the > > direct fix akin to the arm64 one. If it turns out to be not easy, then > > I'll merge the backup commit. > > > > > > diff --git a/drivers/char/random.c b/drivers/char/random.c > > > > index 4862d4d3ec49..f9a020ec08b9 100644 > > > > --- a/drivers/char/random.c > > > > +++ b/drivers/char/random.c > > > > @@ -650,7 +650,8 @@ static void __cold _credit_init_bits(size_t bits) > > > > > > > > if (orig < POOL_READY_BITS && new >= POOL_READY_BITS) { > > > > crng_reseed(); /* Sets crng_init to CRNG_READY under base_crng.lock. */ > > > > - execute_in_process_context(crng_set_ready, &set_ready); > > > > + if (static_key_initialized) > > > > + execute_in_process_context(crng_set_ready, &set_ready); > > > > > > Can we just drop this entirely, and rely on the hunk below to set the > > > static key? What justifies having two code paths that set the static > > > key in different ways on different architectures? > > > > No, we can't. The hunk below (A) is called from init/main.c some time after > > jump_label_init(). The hunk above (B) is called whenever we reach the > > 256-bit mark. > > > > The order is (B)(A) on machines that get seeded from efi or device tree. > > The order is (A)(B) on all other machines, which reach the 256-bit mark > > at "some point"... could be after a second, a minute, whenever enough > > estimated entropy has been accounted. > > Just thinking about this a bit more, I should note that this is not the > first problem caused by EFI/DT doing its thing mega early in the boot > process. Dominik and I fixed up what felt like endless bugs all of > basically that same form back in January. Before it mostly had to do > with workqueues not being available yet. Now it has to do with jump > labels. > > So in thinking about how to fix this, the two approaches thus far > discussed are: > > 1. initialize jump labels earlier, e.g. the arm64 patch (and proposed > arm32 patch). > 2. defer the static key enabling until later, e.g. this patch. > > As a third, I could just defer doing anything with the bootloader seed > until random_init(). This might actually be the simplest solution... > I'll sketch something out. A downside, which might be sort of > significant, is that a few odd things actually use randomness before > random_init() is called. So these would miss out on having that seed. > I'll have to look what exactly to see if we're actually getting anything > real out of that. > This is kind of the point of using a firmware provided seed, i.e., that it is available much earlier than anything else. Could we do this to defer the static key manipulation? That way, the first call to crng_reseed() that occurs after the static keys API becomes available will set the static key, and patch itself away at the same time. ---------->8------------- diff --git a/drivers/char/random.c b/drivers/char/random.c index b691b9d59503..fad4e1a043ce 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -110,7 +110,8 @@ EXPORT_SYMBOL(rng_is_initialized); static void __cold crng_set_ready(struct work_struct *work) { - static_branch_enable(&crng_is_ready); + if (static_key_initialized) + static_branch_enable(&crng_is_ready); } /* Used by wait_for_random_bytes(), and considered an entropy collector, below. */ @@ -202,6 +203,7 @@ static void extract_entropy(void *buf, size_t len); /* This extracts a new crng key from the input pool. */ static void crng_reseed(void) { + static struct execute_work set_ready; unsigned long flags; unsigned long next_gen; u8 key[CHACHA_KEY_SIZE]; @@ -221,8 +223,10 @@ static void crng_reseed(void) ++next_gen; WRITE_ONCE(base_crng.generation, next_gen); WRITE_ONCE(base_crng.birth, jiffies); - if (!static_branch_likely(&crng_is_ready)) + if (!static_branch_likely(&crng_is_ready)) { + execute_in_process_context(crng_set_ready, &set_ready); crng_init = CRNG_READY; + } spin_unlock_irqrestore(&base_crng.lock, flags); memzero_explicit(key, sizeof(key)); } @@ -634,7 +638,6 @@ static void extract_entropy(void *buf, size_t len) static void __cold _credit_init_bits(size_t bits) { - static struct execute_work set_ready; unsigned int new, orig, add; unsigned long flags; @@ -650,7 +653,6 @@ static void __cold _credit_init_bits(size_t bits) if (orig < POOL_READY_BITS && new >= POOL_READY_BITS) { crng_reseed(); /* Sets crng_init to CRNG_READY under base_crng.lock. */ - execute_in_process_context(crng_set_ready, &set_ready); wake_up_interruptible(&crng_init_wait); kill_fasync(&fasync, SIGIO, POLL_IN); pr_notice("crng init done\n"); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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 D905FCCA47C for ; Tue, 7 Jun 2022 11:02:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239921AbiFGLC0 (ORCPT ); Tue, 7 Jun 2022 07:02:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52818 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242454AbiFGLAI (ORCPT ); Tue, 7 Jun 2022 07:00:08 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C22A337A32 for ; Tue, 7 Jun 2022 03:56:37 -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 ams.source.kernel.org (Postfix) with ESMTPS id 43DD7B81F02 for ; Tue, 7 Jun 2022 10:56:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E5FABC34115 for ; Tue, 7 Jun 2022 10:56:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1654599394; bh=NKzuHbkieQxLoWjyC1x/EN9NN+GQBUg1WX2wv6pDfL0=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=mJQLwNsHZlQHm3kZn3CiBIGshtEJ4sMbydDNYYfGqdLAJwSUppmmSveQYV+6BWxuo PcWS9h7x7D/qsF0FEhd8tWoJqe21nXhZe0FcUue12BV7S4h+7UpRdHhhUOJRpD6Byc eSWN6Irh2Az8+6D6I6Tspm6386q3I0Mzv948rLBtrWAYRwNBrr4nLmGD+BHzBXT9Ah VVn+z3f0KVN/wChKeIfZyW779umDe4KfsvYpD8loIDxnKEZb9rxqH1GaA27GQyud4c 98Kb6sX+n3n0W6uT1bp3yNVj5hlPmyS014L85rpgb/8rxCrV5zNM8N9wj3UeS9mNnq TfTFRjt61COzg== Received: by mail-oa1-f50.google.com with SMTP id 586e51a60fabf-fdfe64231dso235390fac.1 for ; Tue, 07 Jun 2022 03:56:34 -0700 (PDT) X-Gm-Message-State: AOAM532TINJNzDEK7LrMHpXuZA2HsKTyssEFuz3YFf4mfW6mudAIUfe5 XkzJ5D1m5GRxVdATabMFrRqxzUIQQKgAJEIrTgU= X-Google-Smtp-Source: ABdhPJxhYGdGLx2yHSyMyGxBsWpnSLuLp5S3Cd5eUQSa1FMAKtjD0WGuvymfVc1RLRcZHETjPDQyKen+Rk/IFcqTrOk= X-Received: by 2002:a05:6871:5c8:b0:f3:3c1c:126f with SMTP id v8-20020a05687105c800b000f33c1c126fmr16121139oan.126.1654599394092; Tue, 07 Jun 2022 03:56:34 -0700 (PDT) MIME-Version: 1.0 References: <20220607100210.683136-1-Jason@zx2c4.com> In-Reply-To: From: Ard Biesheuvel Date: Tue, 7 Jun 2022 12:56:20 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] random: do not use jump labels before they are initialized To: "Jason A. Donenfeld" Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Stephen Boyd , Catalin Marinas , Russell King , Arnd Bergmann , Phil Elwell Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 7 Jun 2022 at 12:42, Jason A. Donenfeld wrote: > > Hey again, > > On Tue, Jun 07, 2022 at 12:28:08PM +0200, Jason A. Donenfeld wrote: > > Hi Ard, > > > > On Tue, Jun 07, 2022 at 12:13:29PM +0200, Ard Biesheuvel wrote: > > > Hi Jason, > > > > > > On Tue, 7 Jun 2022 at 12:04, Jason A. Donenfeld wrote: > > > > > > > > [ I would like to pursue fixing this more directly first before actually > > > > merging this, but I thought I'd send this to the list now anyway as a > > > > the "backup" plan. If I can't figure out how to make headway on the > > > > main plan in the next few days, it'll be easy to just do this. ] > > > > > > > > > > What more direct fix did you have in mind here? > > > > A non-broken version of https://lore.kernel.org/lkml/20220603121543.360283-1-Jason@zx2c4.com/ > > > > As I mentioned in https://lore.kernel.org/lkml/Yp8kQrBgE3WVqqC5@zx2c4.com/ , > > > > I would like a few days to see if there's some trivial way of not > > needing that on arm32. If it turns out to be easy, then I'd prefer the > > direct fix akin to the arm64 one. If it turns out to be not easy, then > > I'll merge the backup commit. > > > > > > diff --git a/drivers/char/random.c b/drivers/char/random.c > > > > index 4862d4d3ec49..f9a020ec08b9 100644 > > > > --- a/drivers/char/random.c > > > > +++ b/drivers/char/random.c > > > > @@ -650,7 +650,8 @@ static void __cold _credit_init_bits(size_t bits) > > > > > > > > if (orig < POOL_READY_BITS && new >= POOL_READY_BITS) { > > > > crng_reseed(); /* Sets crng_init to CRNG_READY under base_crng.lock. */ > > > > - execute_in_process_context(crng_set_ready, &set_ready); > > > > + if (static_key_initialized) > > > > + execute_in_process_context(crng_set_ready, &set_ready); > > > > > > Can we just drop this entirely, and rely on the hunk below to set the > > > static key? What justifies having two code paths that set the static > > > key in different ways on different architectures? > > > > No, we can't. The hunk below (A) is called from init/main.c some time after > > jump_label_init(). The hunk above (B) is called whenever we reach the > > 256-bit mark. > > > > The order is (B)(A) on machines that get seeded from efi or device tree. > > The order is (A)(B) on all other machines, which reach the 256-bit mark > > at "some point"... could be after a second, a minute, whenever enough > > estimated entropy has been accounted. > > Just thinking about this a bit more, I should note that this is not the > first problem caused by EFI/DT doing its thing mega early in the boot > process. Dominik and I fixed up what felt like endless bugs all of > basically that same form back in January. Before it mostly had to do > with workqueues not being available yet. Now it has to do with jump > labels. > > So in thinking about how to fix this, the two approaches thus far > discussed are: > > 1. initialize jump labels earlier, e.g. the arm64 patch (and proposed > arm32 patch). > 2. defer the static key enabling until later, e.g. this patch. > > As a third, I could just defer doing anything with the bootloader seed > until random_init(). This might actually be the simplest solution... > I'll sketch something out. A downside, which might be sort of > significant, is that a few odd things actually use randomness before > random_init() is called. So these would miss out on having that seed. > I'll have to look what exactly to see if we're actually getting anything > real out of that. > This is kind of the point of using a firmware provided seed, i.e., that it is available much earlier than anything else. Could we do this to defer the static key manipulation? That way, the first call to crng_reseed() that occurs after the static keys API becomes available will set the static key, and patch itself away at the same time. ---------->8------------- diff --git a/drivers/char/random.c b/drivers/char/random.c index b691b9d59503..fad4e1a043ce 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -110,7 +110,8 @@ EXPORT_SYMBOL(rng_is_initialized); static void __cold crng_set_ready(struct work_struct *work) { - static_branch_enable(&crng_is_ready); + if (static_key_initialized) + static_branch_enable(&crng_is_ready); } /* Used by wait_for_random_bytes(), and considered an entropy collector, below. */ @@ -202,6 +203,7 @@ static void extract_entropy(void *buf, size_t len); /* This extracts a new crng key from the input pool. */ static void crng_reseed(void) { + static struct execute_work set_ready; unsigned long flags; unsigned long next_gen; u8 key[CHACHA_KEY_SIZE]; @@ -221,8 +223,10 @@ static void crng_reseed(void) ++next_gen; WRITE_ONCE(base_crng.generation, next_gen); WRITE_ONCE(base_crng.birth, jiffies); - if (!static_branch_likely(&crng_is_ready)) + if (!static_branch_likely(&crng_is_ready)) { + execute_in_process_context(crng_set_ready, &set_ready); crng_init = CRNG_READY; + } spin_unlock_irqrestore(&base_crng.lock, flags); memzero_explicit(key, sizeof(key)); } @@ -634,7 +638,6 @@ static void extract_entropy(void *buf, size_t len) static void __cold _credit_init_bits(size_t bits) { - static struct execute_work set_ready; unsigned int new, orig, add; unsigned long flags; @@ -650,7 +653,6 @@ static void __cold _credit_init_bits(size_t bits) if (orig < POOL_READY_BITS && new >= POOL_READY_BITS) { crng_reseed(); /* Sets crng_init to CRNG_READY under base_crng.lock. */ - execute_in_process_context(crng_set_ready, &set_ready); wake_up_interruptible(&crng_init_wait); kill_fasync(&fasync, SIGIO, POLL_IN); pr_notice("crng init done\n");