linux-kernel - Re: [PATCH v2 2/2] mm,page_owner: Fix accounting of pages when migrating

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <Zfwf4Ei_DJVSytbR@localhost.localdomain>
Date: Thu, 21 Mar 2024 12:54:08 +0100
From: Oscar Salvador <osalvador@...e.de>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Matthew Wilcox <willy@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Michal Hocko <mhocko@...e.com>, Marco Elver <elver@...gle.com>,
	Andrey Konovalov <andreyknvl@...il.com>,
	Alexander Potapenko <glider@...gle.com>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Subject: Re: [PATCH v2 2/2] mm,page_owner: Fix accounting of pages when
 migrating

On Thu, Mar 21, 2024 at 12:20:24PM +0100, Vlastimil Babka wrote:
> Understood, but migration is kinda heavy and non-fast-path operation already
> so the extra lookup wouldn't be in a critical fast path.

Ok, you convinced me, let us save that memory.

> When I mean is we have __set_page_owner_handle() setting up everything for
> tail pages and then we have __folio_copy_owner updating only the head page,
> so this will create kinda a mixed up information. Which might not be an
> issue as __folio_copy_owner() should mean it's a proper folio thus compound
> page thus nobody ever will check those mismatched tail pages... Maybe we
> could adjust  __set_page_owner_handle() to skip tail pages for compound
> pages as well and unify this, and tail pages would be only setup for those
> weird high-order non-compound pages so that the odd case in __free_pages()
> works?
> 
> Or maybe page_owner is considered debugging functionality enough so it might
> be worth having the redundant data in tail pages in case something goes
> wrong. But either way now it seems we're not doing it consistently.

So we basically have two options here, 1) is to also update tail pages
in __folio_copy_owner, and 2) is to follow __folio_copy_owner example
and skip tail pages in __set_page_owner.

The thing is, going with 2) might mean, as you pointed out, that if
something goes wrong we lose the information for those pages as
page_owner does not have a record for them.

OTOH, would that information be really useful? Sure we could see the stack, but
AFAICS not even the migrate_reason would be recorded in those tail
pages, which means that if they are migrated and freed, we would only
see that they were freed. (the free stack would point to the migrate
code though, so that is a hint).

Not really sure which path to take here, skipping tail pages would be
less of a complexity but at a risk of loosing information.

Anyway, let me first tackle the issue at hand here, and then I will
think more about that.

Thanks!

-- 
Oscar Salvador
SUSE Labs