From patchwork Thu Oct 13 21:06:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Guilherme G. Piccoli" X-Patchwork-Id: 614915 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 EE55CC4332F for ; Thu, 13 Oct 2022 21:11:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229593AbiJMVLG (ORCPT ); Thu, 13 Oct 2022 17:11:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47772 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229696AbiJMVLE (ORCPT ); Thu, 13 Oct 2022 17:11:04 -0400 Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EB2E92B189; Thu, 13 Oct 2022 14:10:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=tZIWksLvbZf3hKAivkvuzlnH7vev2LNtmjEPqqMctpQ=; b=a5Q74KM2d0akGfotwLs4Oze7xd 5/ohN2Rdfcek3Yqthbqc2LGy9joEXU4R5QpeNAnDlLzf1ha5JAcbRNncDqmQtHb2A80nnOQRND1ey DMV3d3HpEX7Ctu2BWqyjl6J4hy4X/Y1bDZhh4mo22vep7vA27FVULyLrg5NfBoiXKx12Z2IgsBVyH Fdn2HaMiVb9xNaMVxsijAG0HiClg1pGj8EGtrZVckcDsuYYgrT0B8/IuRqvM9g3vk6s3ToCi34RkO lXJ2bFQ/bWJqwMTmM1DGjbLQtQ1IYSKSltQi3nDMmfCPC/acrPq3UyHoenhrQyc7/MVooEIcJzJTN +EaVCKHA==; Received: from 201-43-120-40.dsl.telesp.net.br ([201.43.120.40] helo=localhost) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim) id 1oj5Tk-0010yp-8K; Thu, 13 Oct 2022 23:10:57 +0200 From: "Guilherme G. Piccoli" To: linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org, linux-efi@vger.kernel.org, kernel-dev@igalia.com, kernel@gpiccoli.net, keescook@chromium.org, anton@enomsg.org, ccross@android.com, tony.luck@intel.com, ardb@kernel.org, "Guilherme G. Piccoli" Subject: [PATCH V2 1/3] pstore: Alert on backend write error Date: Thu, 13 Oct 2022 18:06:46 -0300 Message-Id: <20221013210648.137452-2-gpiccoli@igalia.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221013210648.137452-1-gpiccoli@igalia.com> References: <20221013210648.137452-1-gpiccoli@igalia.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-efi@vger.kernel.org The pstore dump function doesn't alert at all on errors - despite pstore is usually a last resource and if it fails users won't be able to read the kernel log, this is not the case for server users with serial access, for example. So, let's at least attempt to inform such advanced users on the first backend writing error detected during the kmsg dump - this is also very useful for pstore debugging purposes. Signed-off-by: Guilherme G. Piccoli Acked-by: Ard Biesheuvel --- V2: - Show error message late, outside of the critical region (thanks Kees for the idea!). fs/pstore/platform.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 06c2c66af332..cbc0b468c1ab 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -393,6 +393,7 @@ static void pstore_dump(struct kmsg_dumper *dumper, const char *why; unsigned int part = 1; unsigned long flags = 0; + int saved_ret = 0; int ret; why = kmsg_dump_reason_str(reason); @@ -463,12 +464,21 @@ static void pstore_dump(struct kmsg_dumper *dumper, if (ret == 0 && reason == KMSG_DUMP_OOPS) { pstore_new_entry = 1; pstore_timer_kick(); + } else { + /* Preserve only the first non-zero returned value. */ + if (!saved_ret) + saved_ret = ret; } total += record.size; part++; } spin_unlock_irqrestore(&psinfo->buf_lock, flags); + + if (saved_ret) { + pr_err_once("backend (%s) writing error (%d)\n", psinfo->name, + saved_ret); + } } static struct kmsg_dumper pstore_dumper = { From patchwork Thu Oct 13 21:06:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Guilherme G. Piccoli" X-Patchwork-Id: 616573 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 8D04AC4332F for ; Thu, 13 Oct 2022 21:11:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229651AbiJMVL0 (ORCPT ); Thu, 13 Oct 2022 17:11:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49444 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229801AbiJMVLU (ORCPT ); Thu, 13 Oct 2022 17:11:20 -0400 Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BD42A18F91B; Thu, 13 Oct 2022 14:11:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=GcklzwrLgeVXmrc9kawBXnG2QLvECr568P4pKI8MuFw=; b=BrDPtCLAEqR1VvN0IuOE/DgCzT tBBEh7LHeyN1ZhVNYID1zIqeGO/7a9E9hIARvHp/F1DAx3lnJsc3fj5v/6cK7++KHCreXrewhNGBn duu2ZbRtli6s+BYpqFWJ5oA2OaZo8ihKZ9qBCBcnCnWMHspHWzTSxbcaP3GQAkIbH55NUdaWMNLy5 PCC5gF0PSP36WeF07kyV5fD0x16REiu6K57wIQpAt5Mx/atoRzKEyBJrnREKe+/6rBX/rNtjFyl93 zNA2EL9YSHln9xMZ8qxN0j+sVQ05MvjCuEj5sckgmGH5RDqKgFeJaB3EotBNj9s0z7/1OHZDq//fr HnDf9VRg==; Received: from 201-43-120-40.dsl.telesp.net.br ([201.43.120.40] helo=localhost) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim) id 1oj5U4-0010zH-BX; Thu, 13 Oct 2022 23:11:17 +0200 From: "Guilherme G. Piccoli" To: linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org, linux-efi@vger.kernel.org, kernel-dev@igalia.com, kernel@gpiccoli.net, keescook@chromium.org, anton@enomsg.org, ccross@android.com, tony.luck@intel.com, ardb@kernel.org, "Guilherme G. Piccoli" Subject: [PATCH V2 2/3] efi: pstore: Follow convention for the efi-pstore backend name Date: Thu, 13 Oct 2022 18:06:47 -0300 Message-Id: <20221013210648.137452-3-gpiccoli@igalia.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221013210648.137452-1-gpiccoli@igalia.com> References: <20221013210648.137452-1-gpiccoli@igalia.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-efi@vger.kernel.org For some reason, the efi-pstore backend name (exposed through the pstore infrastructure) is hardcoded as "efi", whereas all the other backends follow a kind of convention in using the module name. Let's do it here as well, to make user's life easier (they might use this info for unloading the module backend, for example). Acked-by: Ard Biesheuvel Signed-off-by: Guilherme G. Piccoli --- V2: - Added Ard's ACK - thanks! drivers/firmware/efi/efi-pstore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c index 3bddc152fcd4..97a9e84840a0 100644 --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c @@ -207,7 +207,7 @@ static int efi_pstore_erase(struct pstore_record *record) static struct pstore_info efi_pstore_info = { .owner = THIS_MODULE, - .name = "efi", + .name = KBUILD_MODNAME, .flags = PSTORE_FLAGS_DMESG, .open = efi_pstore_open, .close = efi_pstore_close, From patchwork Thu Oct 13 21:06:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Guilherme G. Piccoli" X-Patchwork-Id: 614914 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 8F68FC43217 for ; Thu, 13 Oct 2022 21:11:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229772AbiJMVLo (ORCPT ); Thu, 13 Oct 2022 17:11:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50812 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229984AbiJMVLl (ORCPT ); Thu, 13 Oct 2022 17:11:41 -0400 Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AC9CC18F922; Thu, 13 Oct 2022 14:11:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=zxaa0wrHR2Xk0QAem3sMoc/ttgR5Mks9tB52oq2LMoI=; b=dIqmKbS9ZsZSomzzqJQLshwLGd Hpi7MiR63y0OnqraByMoaYaKLWJVpZBjjSq2MURwcN8ln/FIqz/f/x2iUdbRep/j0TkhNWW1pW5tJ nOcemyt+qiusxHo52qEvdh8O1QwyGFJEChAE80mQTx2DHnNSZkJ45Bo/dMyODtRaNMZEA4TiUOasN qlVoN2P/lFJz9N23Tf45XgfgewIf0QhDp/1lk9rIGZ01FLiwGTfyUpVwtmu50urQUediF1V+zK2OP 9hEQPQhT6HBDzUF3K5W74HOdvkj6s29Ou/IYvqwVSMZ2QFKDSTpWwxDJ1Bet68fYfuFu9ACMD1jCm 1L7ts2Fg==; Received: from 201-43-120-40.dsl.telesp.net.br ([201.43.120.40] helo=localhost) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim) id 1oj5UL-0010za-VR; Thu, 13 Oct 2022 23:11:34 +0200 From: "Guilherme G. Piccoli" To: linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org, linux-efi@vger.kernel.org, kernel-dev@igalia.com, kernel@gpiccoli.net, keescook@chromium.org, anton@enomsg.org, ccross@android.com, tony.luck@intel.com, ardb@kernel.org, "Guilherme G. Piccoli" Subject: [PATCH V2 3/3] efi: pstore: Add module parameter for setting the record size Date: Thu, 13 Oct 2022 18:06:48 -0300 Message-Id: <20221013210648.137452-4-gpiccoli@igalia.com> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221013210648.137452-1-gpiccoli@igalia.com> References: <20221013210648.137452-1-gpiccoli@igalia.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-efi@vger.kernel.org By default, the efi-pstore backend hardcode the UEFI variable size as 1024 bytes. The historical reasons for that were discussed by Ard in threads [0][1]: "there is some cargo cult from prehistoric EFI times going on here, it seems. Or maybe just misinterpretation of the maximum size for the variable *name* vs the variable itself.". "OVMF has OvmfPkg/OvmfPkgX64.dsc: gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 OvmfPkg/OvmfPkgX64.dsc: gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 where the first one is without secure boot and the second with secure boot. Interestingly, the default is gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400 so this is probably where this 1k number comes from." With that, and since there is not such a limit in the UEFI spec, we have the confidence to hereby add a module parameter to enable advanced users to change the UEFI record size for efi-pstore data collection, this way allowing a much easier reading of the collected log, which is not scattered anymore among many small files. Through empirical analysis we observed that extreme low values (like 8 bytes) could eventually cause writing issues, so given that and the OVMF default discussed, we limited the minimum value to 1024 bytes, which also is still the default. [0] https://lore.kernel.org/lkml/CAMj1kXF4UyRMh2Y_KakeNBHvkHhTtavASTAxXinDO1rhPe_wYg@mail.gmail.com/ [1] https://lore.kernel.org/lkml/CAMj1kXFy-2KddGu+dgebAdU9v2sindxVoiHLWuVhqYw+R=kqng@mail.gmail.com/ Cc: Ard Biesheuvel Signed-off-by: Guilherme G. Piccoli --- V2: - Fixed a memory corruption bug in the code (that wasn't causing trouble before due to the fixed sized of record_size), thanks Ard for spotting this! - Added Ard's archeology in the commit message plus a comment with the reasoning behind the minimum value. drivers/firmware/efi/efi-pstore.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c index 97a9e84840a0..827e32427ddb 100644 --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c @@ -10,7 +10,9 @@ MODULE_IMPORT_NS(EFIVAR); #define DUMP_NAME_LEN 66 -#define EFIVARS_DATA_SIZE_MAX 1024 +static unsigned int record_size = 1024; +module_param(record_size, uint, 0444); +MODULE_PARM_DESC(record_size, "size of each pstore UEFI var (in bytes, min/default=1024)"); static bool efivars_pstore_disable = IS_ENABLED(CONFIG_EFI_VARS_PSTORE_DEFAULT_DISABLE); @@ -30,7 +32,7 @@ static int efi_pstore_open(struct pstore_info *psi) if (err) return err; - psi->data = kzalloc(EFIVARS_DATA_SIZE_MAX, GFP_KERNEL); + psi->data = kzalloc(record_size, GFP_KERNEL); if (!psi->data) return -ENOMEM; @@ -52,7 +54,7 @@ static inline u64 generic_id(u64 timestamp, unsigned int part, int count) static int efi_pstore_read_func(struct pstore_record *record, efi_char16_t *varname) { - unsigned long wlen, size = EFIVARS_DATA_SIZE_MAX; + unsigned long wlen, size = record_size; char name[DUMP_NAME_LEN], data_type; efi_status_t status; int cnt; @@ -133,7 +135,7 @@ static ssize_t efi_pstore_read(struct pstore_record *record) efi_status_t status; for (;;) { - varname_size = EFIVARS_DATA_SIZE_MAX; + varname_size = record_size; /* * If this is the first read() call in the pstore enumeration, @@ -224,11 +226,20 @@ static __init int efivars_pstore_init(void) if (efivars_pstore_disable) return 0; - efi_pstore_info.buf = kmalloc(4096, GFP_KERNEL); + /* + * Notice that 1024 is the minimum here to prevent issues with + * decompression algorithms that were spotted during tests; + * even in the case of not using compression, smaller values would + * just pollute more the pstore FS with many small collected files. + */ + if (record_size < 1024) + record_size = 1024; + + efi_pstore_info.buf = kmalloc(record_size, GFP_KERNEL); if (!efi_pstore_info.buf) return -ENOMEM; - efi_pstore_info.bufsize = 1024; + efi_pstore_info.bufsize = record_size; if (pstore_register(&efi_pstore_info)) { kfree(efi_pstore_info.buf);