LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Joel Fernandes <joel@joelfernandes.org>
To: Kees Cook <keescook@chromium.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	kernel-team@android.com, Anton Vorontsov <anton@enomsg.org>,
	Colin Cross <ccross@android.com>, Tony Luck <tony.luck@intel.com>
Subject: Re: [RFC 5/6] pstore: donot treat empty buffers as valid
Date: Fri, 26 Oct 2018 13:22:05 -0700	[thread overview]
Message-ID: <20181026202205.GB129228@joelaf.mtv.corp.google.com> (raw)
In-Reply-To: <CAGXu5jKtZjZVr=7dAHQPU0OGtBpEKDZBO68-RTvFLbK6qB+cEA@mail.gmail.com>

On Fri, Oct 26, 2018 at 08:39:13PM +0100, Kees Cook wrote:
> On Fri, Oct 26, 2018 at 7:00 PM, Joel Fernandes (Google)
> <joel@joelfernandes.org> wrote:
> > pstore currently calls persistent_ram_save_old even if a buffer is
> > empty. While this appears to work, it is simply not the right thing to
> > do and could lead to bugs so lets avoid that. It also prevent misleading
> > prints in the logs which claim the buffer is valid.
> 
> I need to be better convinced that a present zero length record is the
> same as a non-present record. This seems true, but there is
> potentially still metadata available from a backend. What were the
> misleading prints in logs?

I got something like:
found existing buffer, size 0, start 0

When I was expecting:
no valid data in buffer (sig = ...)

The other thing is a call to persistent_ram_zap is also prevented on the
buffer, which prevent zero-initialize prz->ecc_info.par. Since we are
dropping patch 6/6, the zap will not happen. But I'm not super familiar with
the ecc bits of this code to say that if that's an issue.

About the present zero-length record, I would argue that it should not be
"present" at all. When the system first boots, the record is not present but
the signatures are initialized, then on reboots because the signatures were
initialized, the buffer appears as valid even if it was unused. So for dmesg,
all the max_dump_cnt number of buffers would appear as if they are valid
which is a bit strange because there was no crash at all so it should be in
the same state on reboot, as if there was no crash. That could be a matter of
perspective though so I leave it you how you prefer to do it :)

thanks,

- Joel


  reply	other threads:[~2018-10-26 20:22 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-26 18:00 [RFC 1/6] pstore: map pstore types to names Joel Fernandes (Google)
2018-10-26 18:00 ` [RFC 2/6] pstore: remove type argument from ramoops_get_next_prz Joel Fernandes (Google)
2018-10-26 19:05   ` Kees Cook
2018-10-26 18:00 ` [RFC 3/6] pstore: remove max " Joel Fernandes (Google)
2018-10-26 19:22   ` Joel Fernandes
2018-10-26 19:27     ` Kees Cook
2018-10-26 19:40       ` Joel Fernandes
2018-10-26 19:22   ` Kees Cook
2018-10-26 18:00 ` [RFC 4/6] pstore: further reduce ramoops_get_next_prz arguments by passing record Joel Fernandes (Google)
2018-10-26 19:32   ` Kees Cook
2018-10-26 19:36     ` Joel Fernandes
2018-10-26 18:00 ` [RFC 5/6] pstore: donot treat empty buffers as valid Joel Fernandes (Google)
2018-10-26 19:39   ` Kees Cook
2018-10-26 20:22     ` Joel Fernandes [this message]
2018-10-26 18:00 ` [RFC 6/6] Revert "pstore/ram_core: Do not reset restored zone's position and size" Joel Fernandes (Google)
2018-10-26 18:16   ` Kees Cook
2018-10-26 18:22     ` Joel Fernandes
2018-10-26 19:42       ` Kees Cook
2018-10-26 20:09         ` Joel Fernandes
2018-10-26 19:04 ` [RFC 1/6] pstore: map pstore types to names Kees Cook
2018-10-26 20:35   ` Joel Fernandes
2018-10-26 20:41     ` Kees Cook

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=20181026202205.GB129228@joelaf.mtv.corp.google.com \
    --to=joel@joelfernandes.org \
    --cc=anton@enomsg.org \
    --cc=ccross@android.com \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tony.luck@intel.com \
    --subject='Re: [RFC 5/6] pstore: donot treat empty buffers as valid' \
    /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

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).