From nobody Mon Feb 9 05:53:23 2026 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 63437C6FA8F for ; Wed, 30 Aug 2023 22:21:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343793AbjH3WVN (ORCPT ); Wed, 30 Aug 2023 18:21:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51058 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343738AbjH3WVG (ORCPT ); Wed, 30 Aug 2023 18:21:06 -0400 X-Greylist: delayed 3360 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Wed, 30 Aug 2023 15:20:45 PDT Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 766B0CE7 for ; Wed, 30 Aug 2023 15:20:45 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 4F4EEB8201B for ; Wed, 30 Aug 2023 21:23:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 24FFBC433C8; Wed, 30 Aug 2023 21:23:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1693430588; bh=WNWg7VP05g6kGXkXozvlUrP9YXpC0uBWHnqinPizuqQ=; h=From:To:Cc:Subject:Date:From; b=BVN4reKnzN3NNsB/Gz28AQmjOkNAsuAeGqKRq8BcxVMTIrxb/jWaRUfJ13AEqHKnQ HQPGcREwKPjdjY0u6Nm+EBi74Hz6iRmTAA0L5l6lgDH54jogtqfw0tsAbxGodhwZAw klzhptnEypYGk6uhTisqDlQhIVv9YjnzSSyEd84talsSGlswDnRsaH++HNks9akQsO gdRKLdMapITMIAsdUX4LMuDhB1YphtxApd8SeuARlzgDO0tbiZvdruXdHTIMjZAgwR 9k1IdMyiLM2wN/7y451D8Vsq9S9uGtJ7+DzRmUlLpSNOkBNMNnfP4e7+gHr4KgKLR8 FnCmVQqLaZkaw== From: Ard Biesheuvel To: linux-kernel@vger.kernel.org Cc: Ard Biesheuvel , Linus Torvalds , Eric Biggers , Kees Cook , Herbert Xu Subject: [PATCH] pstore: Base compression input buffer size on estimated compressed size Date: Wed, 30 Aug 2023 23:22:38 +0200 Message-Id: <20230830212238.135900-1-ardb@kernel.org> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=6123; i=ardb@kernel.org; h=from:subject; bh=WNWg7VP05g6kGXkXozvlUrP9YXpC0uBWHnqinPizuqQ=; b=owGbwMvMwCFmkMcZplerG8N4Wi2JIeX9ZtlTTOKX2Jd9UzZfIv9tvWTdpPqbs+7+tC5adXzJ3 qTpN+8qdZSyMIhxMMiKKbIIzP77bufpiVK1zrNkYeawMoEMYeDiFICJbP7E8E//z/ZQ1+O93FGb fgfP6p6fu9NUTEKwNp6rXSl1nqrMCnVGhr5vn1tjFTaVSy+JFPap+yArd1oohd/v1PKUVFv1dWt EWAA= X-Developer-Key: i=ardb@kernel.org; a=openpgp; fpr=F43D03328115A198C90016883D200E9CA6329909 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Commit 1756ddea6916 ("pstore: Remove worst-case compression size logic") removed some clunky per-algorithm worst case size estimation routines on the basis that we can always store pstore records uncompressed, and these worst case estimations are about how much the size might inadvertently *increase* due to encapsulation overhead when the input cannot be compressed at all. So if compression results in a size increase, we just store the original data instead. However, it seems that the the original code was misinterpreting these calculations as an estimation of how much uncompressed data might fit into a compressed buffer of a given size, and it was using the results to consume the input data in larger chunks than the pstore record size, relying on the compression to ensure that what ultimately gets stored fits into the available space. One result of this, as observed and reported by Linus, is that upgrading to a newer kernel that includes the given commit may result in pstore decompression errors reported in the kernel log. This is due to the fact that the existing records may unexpectedly decompress to a size that is larger than the pstore record size. Another potential problem caused by this change is that we may underutilize the fixed sized records on pstore backends such as ramoops. And on pstore backends with variable sized records such as EFI, we will end up creating many more entries than before to store the same amount of compressed data. So let's fix both issues, by bringing back the typical case estimation of how much ASCII text captured from the dmesg log might fit into a pstore record of a given size after compression. The original implementation used the computation given below for zlib, and so simply taking 2x as a ballpark number seems appropriate here. switch (size) { /* buffer range for efivars */ case 1000 ... 2000: cmpr =3D 56; break; case 2001 ... 3000: cmpr =3D 54; break; case 3001 ... 3999: cmpr =3D 52; break; /* buffer range for nvram, erst */ case 4000 ... 10000: cmpr =3D 45; break; default: cmpr =3D 60; break; } return (size * 100) / cmpr; While at it, rate limit the error message so we don't flood the log unnecessarily on systems that have accumulated a lot of pstore history. Cc: Linus Torvalds Cc: Eric Biggers Cc: Kees Cook Cc: Herbert Xu Signed-off-by: Ard Biesheuvel --- fs/pstore/platform.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 62356d542ef67f60..a866b70ea5933a1d 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -98,7 +98,14 @@ MODULE_PARM_DESC(kmsg_bytes, "amount of kernel log to sn= apshot (in bytes)"); =20 static void *compress_workspace; =20 +/* + * Compression is only used for dmesg output, which consists of low-entropy + * ASCII text, and so we can assume a 2x compression factor is achievable. + */ +#define DMESG_COMP_FACTOR 2 + static char *big_oops_buf; +static size_t max_uncompressed_size; =20 void pstore_set_kmsg_bytes(int bytes) { @@ -216,7 +223,7 @@ static void allocate_buf_for_compression(void) * uncompressed record size, since any record that would be expanded by * compression is just stored uncompressed. */ - buf =3D kvzalloc(psinfo->bufsize, GFP_KERNEL); + buf =3D kvzalloc(DMESG_COMP_FACTOR * psinfo->bufsize, GFP_KERNEL); if (!buf) { pr_err("Failed %zu byte compression buffer allocation for: %s\n", psinfo->bufsize, compress); @@ -233,6 +240,7 @@ static void allocate_buf_for_compression(void) =20 /* A non-NULL big_oops_buf indicates compression is available. */ big_oops_buf =3D buf; + max_uncompressed_size =3D DMESG_COMP_FACTOR * psinfo->bufsize; =20 pr_info("Using crash dump compression: %s\n", compress); } @@ -246,6 +254,7 @@ static void free_buf_for_compression(void) =20 kvfree(big_oops_buf); big_oops_buf =3D NULL; + max_uncompressed_size =3D 0; } =20 void pstore_record_init(struct pstore_record *record, @@ -305,7 +314,7 @@ static void pstore_dump(struct kmsg_dumper *dumper, record.buf =3D psinfo->buf; =20 dst =3D big_oops_buf ?: psinfo->buf; - dst_size =3D psinfo->bufsize; + dst_size =3D max_uncompressed_size ?: psinfo->bufsize; =20 /* Write dump header. */ header_size =3D snprintf(dst, dst_size, "%s#%d Part%u\n", why, @@ -326,8 +335,15 @@ static void pstore_dump(struct kmsg_dumper *dumper, record.compressed =3D true; record.size =3D zipped_len; } else { - record.size =3D header_size + dump_size; - memcpy(psinfo->buf, dst, record.size); + /* + * Compression failed, so the buffer is most + * likely filled with binary data that does not + * compress as well as ASCII text. Copy as much + * of the uncompressed data as possible into + * the pstore record, and discard the rest. + */ + record.size =3D psinfo->bufsize; + memcpy(psinfo->buf, dst, psinfo->bufsize); } } else { record.size =3D header_size + dump_size; @@ -583,7 +599,7 @@ static void decompress_record(struct pstore_record *rec= ord, } =20 /* Allocate enough space to hold max decompression and ECC. */ - workspace =3D kvzalloc(psinfo->bufsize + record->ecc_notice_size, + workspace =3D kvzalloc(max_uncompressed_size + record->ecc_notice_size, GFP_KERNEL); if (!workspace) return; @@ -591,11 +607,11 @@ static void decompress_record(struct pstore_record *r= ecord, zstream->next_in =3D record->buf; zstream->avail_in =3D record->size; zstream->next_out =3D workspace; - zstream->avail_out =3D psinfo->bufsize; + zstream->avail_out =3D max_uncompressed_size; =20 ret =3D zlib_inflate(zstream, Z_FINISH); if (ret !=3D Z_STREAM_END) { - pr_err("zlib_inflate() failed, ret =3D %d!\n", ret); + pr_err_ratelimited("zlib_inflate() failed, ret =3D %d!\n", ret); kvfree(workspace); return; } --=20 2.39.2