From: Joel Fernandes <joel@joelfernandes.org>
To: Kees Cook <keescook@chromium.org>
Cc: linux-kernel@vger.kernel.org, Anton Vorontsov <anton@enomsg.org>,
Colin Cross <ccross@android.com>, Tony Luck <tony.luck@intel.com>
Subject: Re: [PATCH 8/8] pstore/ram: Correctly calculate usable PRZ bytes
Date: Fri, 2 Nov 2018 11:01:11 -0700 [thread overview]
Message-ID: <20181102180111.GA14942@google.com> (raw)
In-Reply-To: <20181101235200.28584-9-keescook@chromium.org>
On Thu, Nov 01, 2018 at 04:52:00PM -0700, Kees Cook wrote:
> The actual number of bytes stored in a PRZ is smaller than the
> bytes requested by platform data, since there is a header on each
> PRZ. Additionally, if ECC is enabled, there are trailing bytes used
> as well. Normally this mismatch doesn't matter since PRZs are circular
> buffers and the leading "overflow" bytes are just thrown away. However, in
> the case of a compressed record, this rather badly corrupts the results.
Actually this would also mean some data loss for non-compressed records were
also there before, but is now fixed?
> This corruption was visible with "ramoops.mem_size=204800 ramoops.ecc=1".
> Any stored crashes would not be uncompressable (producing a pstorefs
> "dmesg-*.enc.z" file), and triggering errors at boot:
>
> [ 2.790759] pstore: crypto_comp_decompress failed, ret = -22!
>
> Reported-by: Joel Fernandes <joel@joelfernandes.org>
> Fixes: b0aad7a99c1d ("pstore: Add compression support to pstore")
> Signed-off-by: Kees Cook <keescook@chromium.org>
Thanks!
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Also should this be fixed for other backends or are those good? AFAIR, I saw
this for EFI too.
- Joel
> ---
> fs/pstore/ram.c | 15 ++++++---------
> include/linux/pstore.h | 5 ++++-
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 25bede911809..10ac4d23c423 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -814,17 +814,14 @@ static int ramoops_probe(struct platform_device *pdev)
>
> cxt->pstore.data = cxt;
> /*
> - * Console can handle any buffer size, so prefer LOG_LINE_MAX. If we
> - * have to handle dumps, we must have at least record_size buffer. And
> - * for ftrace, bufsize is irrelevant (if bufsize is 0, buf will be
> - * ZERO_SIZE_PTR).
> + * Since bufsize is only used for dmesg crash dumps, it
> + * must match the size of the dprz record (after PRZ header
> + * and ECC bytes have been accounted for).
> */
> - if (cxt->console_size)
> - cxt->pstore.bufsize = 1024; /* LOG_LINE_MAX */
> - cxt->pstore.bufsize = max(cxt->record_size, cxt->pstore.bufsize);
> - cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
> + cxt->pstore.bufsize = cxt->dprzs[0]->buffer_size;
> + cxt->pstore.buf = kzalloc(cxt->pstore.bufsize, GFP_KERNEL);
> if (!cxt->pstore.buf) {
> - pr_err("cannot allocate pstore buffer\n");
> + pr_err("cannot allocate pstore crash dump buffer\n");
> err = -ENOMEM;
> goto fail_clear;
> }
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index 3549f2ba865c..f46e5df76b58 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -90,7 +90,10 @@ struct pstore_record {
> *
> * @buf_lock: spinlock to serialize access to @buf
> * @buf: preallocated crash dump buffer
> - * @bufsize: size of @buf available for crash dump writes
> + * @bufsize: size of @buf available for crash dump bytes (must match
> + * smallest number of bytes available for writing to a
> + * backend entry, since compressed bytes don't take kindly
> + * to being truncated)
> *
> * @read_mutex: serializes @open, @read, @close, and @erase callbacks
> * @flags: bitfield of frontends the backend can accept writes for
> --
> 2.17.1
>
next prev parent reply other threads:[~2018-11-02 18:01 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-01 23:51 [PATCH 0/8] pstore improvements (pstore-next) Kees Cook
2018-11-01 23:51 ` [PATCH linux-next 1/8] pstore/ram: Standardize module name in ramoops Kees Cook
2018-11-01 23:51 ` [PATCH 2/8] pstore: Do not use crash buffer for decompression Kees Cook
2018-11-02 18:24 ` Joel Fernandes
2018-11-14 7:56 ` Kees Cook
2018-11-20 21:43 ` Joel Fernandes
2018-11-29 22:06 ` Kees Cook
2018-11-30 2:26 ` Joel Fernandes
2018-11-01 23:51 ` [PATCH 3/8] pstore/ram: Report backend assignments with finer granularity Kees Cook
2018-11-01 23:51 ` [PATCH 4/8] pstore/ram: Add kern-doc for struct persistent_ram_zone Kees Cook
2018-11-01 23:51 ` [PATCH 5/8] pstore: Improve and update some comments and status output Kees Cook
2018-11-01 23:51 ` [PATCH 6/8] pstore: Replace open-coded << with BIT() Kees Cook
2018-11-01 23:51 ` [PATCH 7/8] pstore: Remove needless lock during console writes Kees Cook
2018-11-02 18:32 ` Joel Fernandes
2018-11-02 20:40 ` Kees Cook
2018-11-02 21:50 ` Joel Fernandes
2018-11-01 23:52 ` [PATCH 8/8] pstore/ram: Correctly calculate usable PRZ bytes Kees Cook
2018-11-02 18:01 ` Joel Fernandes [this message]
2018-11-02 20:00 ` Kees Cook
2018-11-05 4:42 ` Joel Fernandes
2018-11-05 17:04 ` Kees Cook
2018-11-06 4:42 ` Joel Fernandes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181102180111.GA14942@google.com \
--to=joel@joelfernandes.org \
--cc=anton@enomsg.org \
--cc=ccross@android.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tony.luck@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).