linux-kernel - Re: [PATCH v3 1/2] kho: add support for preserving vmalloc allocations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mafs0bjnaimvc.fsf@kernel.org>
Date: Tue, 16 Sep 2025 14:48:55 +0200
From: Pratyush Yadav <pratyush@...nel.org>
To: Mike Rapoport <rppt@...nel.org>
Cc: Pratyush Yadav <me@...avpratyush.com>,  Pratyush Yadav
 <pratyush@...nel.org>,  Andrew Morton <akpm@...ux-foundation.org>,
  Alexander Graf <graf@...zon.com>,  Baoquan He <bhe@...hat.com>,
  Changyuan Lyu <changyuanl@...gle.com>,  Chris Li <chrisl@...nel.org>,
  Jason Gunthorpe <jgg@...dia.com>,  Pasha Tatashin
 <pasha.tatashin@...een.com>,  kexec@...ts.infradead.org,
  linux-mm@...ck.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] kho: add support for preserving vmalloc allocations

On Mon, Sep 15 2025, Mike Rapoport wrote:

> On Tue, Sep 09, 2025 at 04:33:27PM +0200, Pratyush Yadav wrote:
>> Hi Mike,
>> 
>> Couple more thoughts.
>> 
>> On Mon, Sep 08 2025, Pratyush Yadav wrote:
>> > On Mon, Sep 08 2025, Mike Rapoport wrote:
>> >> +
>> >> +	while (chunk) {
>> >> +		struct page *page;
>> >> +
>> >> +		for (int i = 0; i < chunk->hdr.num_elms; i++) {
>> >> +			phys_addr_t phys = chunk->phys[i];
>> >> +
>> >> +			for (int j = 0; j < (1 << order); j++) {
>> >> +				page = phys_to_page(phys);
>> >> +				kho_restore_page(page, 0);
>> >> +				pages[idx++] = page;
>> >
>> > This can buffer-overflow if the previous kernel was buggy and added too
>> > many pages. Perhaps keep check for this?
>> 
>> Thinking about this a bit more, I think this should check that we found
>> _exactly_ chunk->hdr.total_pages pages, and should error out otherwise.
>> If too few are found, pages array will contain bogus data, if too many,
>> buffer overflow.
>
> Sure, I can add the checks, but it feels superfluous to me.

See my reasoning in the other reply:
https://lore.kernel.org/linux-mm/mafs0frcmin3t.fsf@kernel.org/

>  
>> Also, I am not a fan of using kho_restore_page() directly. I think the
>> vmalloc preservation is a layer above core KHO, and it should use the
>> public KHO APIs. It really doesn't need to poke into internal APIs. If
>> any of the public APIs are insufficient, we should add new ones.
>> 
>> I don't suppose I'd insist on it, but something to consider since you
>> are likely going to do another revision anyway.
>
> I think vmalloc is as basic as folio. At some point we probably converge to 
> kho_preserve(void *) that will choose the right internal handler. like
> folio, vmalloc, kmalloc etc.

Sure, but do you need to use the internal APIs? Because doing this way
would miss some improvements for the public APIs. See my patch for
adding more sanity checking to kho_restore_folio() for example:
https://lore.kernel.org/linux-mm/20250910153443.95049-1-pratyush@kernel.org/

vmalloc preservation would miss this improvement since it uses the
internal API, even though it will clearly benefit from it.

-- 
Regards,
Pratyush Yadav