Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Date:   Mon, 4 Dec 2023 11:54:34 -0800
From:   Andrew Morton <akpm@linux-foundation.org>
To:     York Jasper Niebuhr <yjnworkstation@gmail.com>
Cc:     linux-kernel@vger.kernel.org, rppt@kernel.org,
        linux-security-module@vger.kernel.org
Subject: Re: [PATCH] mm: init_mlocked_on_free
Message-Id: <20231204115434.65f04d1de5041038ab5e2b8d@linux-foundation.org>
In-Reply-To: <20231202134218.151074-1-yjnworkstation@gmail.com>
References: <20231202134218.151074-1-yjnworkstation@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sat,  2 Dec 2023 14:42:18 +0100 York Jasper Niebuhr <yjnworkstation@gmail.com> wrote:

> Adds the "PG_ofinit" page flag to specify if a page should be zeroed on
> free.

That's a problem - unused page flags are few, and are a treasured
resource.  Matthew Wilcox is a suitable reviewer, but you didn't cc the
linux-mm mailing list.

Please address this concern in future changelogs.

> Implements the "init_mlocked_on_free" boot option. When this boot option
> is set, any mlock'ed pages are zeroed on munmap, exit or exec. If the
> pages are munlock'ed beforehand, no initialization will take place. This
> boot option is meant to combat the performance hit of "init_on_free" as
> reported in commit 6471384af2a6 ("mm: security: introduce
> init_on_alloc=1 and init_on_free=1 boot options"). With
> "init_mlocked_on_free", only relevant data will be freed while
> everything else is left untouched by the kernel.

It would be helpful to provide a full description of the performance
benefits right here in the changelog, please.  Including example
quantitative testing results.  See if you can persuade us to consume
another page flag.  

Also, can we avoid using a page flag?   Can this be done on a per-vma
basis rather than per-page?

> Optimally, userspace programs will clear any key material or other
> confidential memory before exit and munlock the according memory
> regions. If a program crashes, however, userspace key managers will not
> be able to zero this data. If this happens, the memory will not be
> explicitly munlock'ed before exit either, so the kernel will zero the
> data and prevent data leaks. If the program finishes properly, no pages
> will be initialized again, as they were already munlock'ed.
> 
> In general, leaving memory mlock'ed until unmap, exit or exec can be used
> to specify exactly what memory should be initialized on free.
> 
> CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON can be set to enable
> "init_mlocked_on_free" by default.
> 

Please address the above and send us a v2?