public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: "Guilherme G. Piccoli" <gpiccoli@igalia.com>
Cc: Yuxiao Zhang <yuxiaozhang@google.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Tony Luck <tony.luck@intel.com>,
	linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
	wak@google.com
Subject: Re: [PATCH] pstore: ramoops: support pmsg size larger than kmalloc limitation
Date: Wed, 28 Jun 2023 16:24:42 -0700	[thread overview]
Message-ID: <202306281615.360B12EFC@keescook> (raw)
In-Reply-To: <28718037-ccf1-c36b-4eb6-716c518ca98f@igalia.com>

On Wed, Jun 28, 2023 at 03:12:10PM -0300, Guilherme G. Piccoli wrote:
> Also - Kees certainly knows that way better, but - are we 100% sure that
> the region for pstore can be non-contiguous? For some reason, I always
> thought this was a requirement - grepping the code, I found this
> (wrong?) comment:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/pstore/zone.c#n3

The contiguous requirements are entirely for the RAM backend's storage,
so these intermediate memory regions can use non-contiguous physical
backing memory (i.e. vmalloc).

Even the special case of crash-dumping should be fine for the large
buffer used to hold the crash before doing compression.

There are effectively 4 types of allocations in pstore:

1- a physical -> virtual mapping for the RAM backend
2- the allocations (if any) for non-RAM backends to hold a copy of pstore
   records when extracted from the backend storage (e.g NVRAM, EFI vars,
   etc).
3- incoming allocations that are used temporarily to hand data to the
   backend (e.g. the write_compat used in this patch)
4- the allocation used for holding the pstorefs data contents (which I
   need to double-check, but is entirely defined by the backends)

Changes aren't needed for (1), it's fine on its own. This patch changes
allocations for (2) and (3) for pmsg and the RAM backend, if I'm reading
it correctly. I think (4) is included as part of (2), but I need to
check. As long as all paths use kvfree() for the record buffers,
everything should Just Work for RAM. Switching the other backends to
also use kvalloc() would allow for greater flexibility, though.

Anyway, it's on my list to review and test. I'm still working through
other things related to the merge window opening, so I may be a bit slow
for the next week. :)

-Kees

-- 
Kees Cook

  reply	other threads:[~2023-06-28 23:24 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-27 20:25 [PATCH] pstore: ramoops: support pmsg size larger than kmalloc limitation Yuxiao Zhang
2023-06-28  5:30 ` Greg KH
2023-06-28 17:10   ` Yuxiao Zhang
2023-06-28 18:12     ` Guilherme G. Piccoli
2023-06-28 23:24       ` Kees Cook [this message]
2023-06-29 19:22         ` Guilherme G. Piccoli
2023-06-28 17:55 ` Kees Cook
2023-06-30 20:50   ` Yuxiao Zhang
2023-06-30 20:53   ` Yuxiao Zhang
2023-07-18 20:23     ` Yuxiao Zhang
2023-08-17 23:40       ` Kees Cook
  -- strict thread matches above, loose matches on Subject: below --
2022-01-04 19:36 pstore/ramoops - why only collect a partial dmesg? Guilherme G. Piccoli
2023-06-28 18:48 ` [PATCH] pstore: ramoops: support pmsg size larger than kmalloc limitation Yuxiao Zhang
2023-06-28 19:05   ` Guilherme G. Piccoli

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=202306281615.360B12EFC@keescook \
    --to=keescook@chromium.org \
    --cc=gpiccoli@igalia.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tony.luck@intel.com \
    --cc=wak@google.com \
    --cc=yuxiaozhang@google.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