From: Zhaoyang Huang <[email protected]>
There is no explicit gfp flags to let the allocation skip zero
operation when CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y. I would like to make
__GFP_SKIP_ZERO be visible even if kasan is not configured.
Signed-off-by: Zhaoyang Huang <[email protected]>
---
include/linux/gfp_types.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
mode change 100644 => 100755 include/linux/gfp_types.h
diff --git a/include/linux/gfp_types.h b/include/linux/gfp_types.h
old mode 100644
new mode 100755
index d88c46ca82e1..4e9d50bba269
--- a/include/linux/gfp_types.h
+++ b/include/linux/gfp_types.h
@@ -46,12 +46,11 @@ typedef unsigned int __bitwise gfp_t;
#define ___GFP_THISNODE 0x200000u
#define ___GFP_ACCOUNT 0x400000u
#define ___GFP_ZEROTAGS 0x800000u
+#define ___GFP_SKIP_ZERO 0x1000000u
#ifdef CONFIG_KASAN_HW_TAGS
-#define ___GFP_SKIP_ZERO 0x1000000u
#define ___GFP_SKIP_KASAN_UNPOISON 0x2000000u
#define ___GFP_SKIP_KASAN_POISON 0x4000000u
#else
-#define ___GFP_SKIP_ZERO 0
#define ___GFP_SKIP_KASAN_UNPOISON 0
#define ___GFP_SKIP_KASAN_POISON 0
#endif
--
2.25.1
On Fri, Sep 1, 2023 at 12:29 PM Zhaoyang Huang <[email protected]> wrote:
>
> loop alex
(adding more people who took part in the previous discussions)
>
> On Thu, Aug 31, 2023 at 8:16 PM Matthew Wilcox <[email protected]> wrote:
> >
> > On Thu, Aug 31, 2023 at 06:52:52PM +0800, zhaoyang.huang wrote:
> > > From: Zhaoyang Huang <[email protected]>
> > >
> > > There is no explicit gfp flags to let the allocation skip zero
> > > operation when CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y. I would like to make
> > > __GFP_SKIP_ZERO be visible even if kasan is not configured.
Hi all,
This is a recurring question, as people keep encountering performance
problems on systems with init_on_alloc=1
(https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1862822 being
one of the examples).
Brad Spengler has also pointed out
(https://twitter.com/spendergrsec/status/1296461651659694082) that
there are cases where there is no security vs. performance tradeoff
(e.g. kmemdup() and kstrdup()).
An opt-out flag was included in the initial init_on_alloc series, but
back then Michal Hocko has noted that it might easily get out of
control: https://patchwork.kernel.org/project/linux-hardening/patch/[email protected]/#22600229.
Now that init_on_alloc is actually being used by people which may have
different preferences wrt. security and performance (in the cases
where this tradeoff exists), we must be very careful with the opt-out
GFP flag. Not initializing a particular allocation site in the
upstream kernel will affect every downstream user, and some may
consider this a security regression.
Another problematic case is an OS vendor mandating init_on_alloc
everywhere, but a third party driver vendor doing kmalloc(...,
__GFP_SKIP_ZERO) for their allocations.
So I think a working opt-out scheme for the heap initialization should
be two-step:
1. The code owner may decide that a particular allocation site needs
an opt-out, and make the upstream code change;
2. The OS vendor has the ability to override that decision for the
kernel they ship without the need to patch the source.
Let me quoute the idea briefly outlined at
https://lore.kernel.org/lkml/CAG_fn=UQEuvJ9WXou_sW3moHcVQZJ9NvJ5McNcsYE8xw_WEYGw@mail.gmail.com/
(unfortunately the discussion got derailed a bit):
"""
1. Add a ___GFP_SKIP_ZERO flag that is not intended to be used
explicitly (e.g. disallow passing it to kmalloc(), add a checkpatch.pl
warning). Explicitly passing an opt-out flag to allocation functions
was considered harmful previously:
https://lore.kernel.org/kernel-hardening/[email protected]/
2. Define new allocation API that will allow opt-out:
struct page *alloc_pages_uninit(gfp_t gfp, unsigned int order, const
char *key);
void *kmalloc_uninit(size_t size, gfp_t flags, const char *key);
void *kmem_cache_alloc_uninit(struct kmem_cache *, gfp_t flags,
const char *key);
, where @key is an arbitrary string that identifies a single
allocation or a group of allocations.
3. Provide a boot-time flag that holds a comma-separated list of
opt-out keys that actually take effect:
init_on_alloc.skip="xyz-camera-driver,some_big_buffer".
"""
A draft implementation at
https://github.com/ramosian-glider/linux/commit/00791be14eb1113eae615c74b652f94b5cc3c336
(which probably does not apply anymore) may give some insight into how
this is supposed to work.
There's plenty of room for bikeshedding here (does the command-line
flag opt-in or opt-out? should we use function names instead of some
"keys"? can we allow overriding every allocation site without the need
for alloc_pages_uninit()?), but if the overall scheme is viable I can
probably proceed with an RFC.
> >
> > This bypasses a security feature so you're going to have to do a little
> > better than "I want it".
> Thanks for pointing this out. What I want to do is to give the user a
> way to exempt some types of pages from being zeroed, which could help
> on performance issues. Could we have the most safety concern admin
> use INIT_ON_FREE while the less concerned use INIT_ON_ALLOC &
> __GFP_SKIP_ZERO as a light version method?
init_on_free has a more significant performance impact, and might be
more problematic for production use (even more opt-outs would've been
needed).
As a side note, I don't think we should repurpose the same
__GFP_SKIP_ZERO flag used by KASAN, because the semantics of the flags
may not match 1:1.
On Mon, Sep 04, 2023 at 10:34:25AM -0700, Linus Torvalds wrote:
> On Mon, 4 Sept 2023 at 00:55, Michal Hocko <[email protected]> wrote:
> >
> > Sooner or later this will become an
> > unreviewable mess so the value of init_on_alloc will become very
> > dubious.
>
> The value of init_on_alloc is *already* very dubious.
>
> Exactly because people will turn it off, because it hurts performance
> so much - and in pointless ways.
>
> You do realize that distributions - well, at least Fedora - simply
> don't turn INIT_ON_ALLOC_DEFAULT_ON on at all?
>
> So the current state of init_on_alloc is that nobody sane uses it. You
> have to think you're special to enable it, because it is *so* bad.
>
> Security people need to realize that the primary point of computing is
> NEVER EVER security. Security is entirely pointless without a usable
> system.
>
> Unless security people realize that they are always secondary, they
> aren't security people, they are just random wankers.
>
> And people who state this truism had better not get shamed for
> standing up to stupidity.
>
Android and Ubuntu both set CONFIG_INIT_ON_ALLOC_DEFAULT_ON. I think this makes
it clear that the init-on-alloc feature is useful for a substantial amount of
users even in its current form.
I would caution against checking the kernel config for the distro you happen to
be using and extrapolating that to all Linux systems.
Regardless, I'm in favor of a per allocation opt-out flag like GFP_SKIP_ZERO.
There are clear cases where it makes sense, for example some places in the VFS
where the performance impact is large and the code has been carefully reviewed.
I don't really like the idea
(https://lore.kernel.org/lkml/CAG_fn=UQEuvJ9WXou_sW3moHcVQZJ9NvJ5McNcsYE8xw_WEYGw@mail.gmail.com/)
of making the system administrator have to opt out allocation sites individually
via a kernel command-line parameter. Yes, it makes the opt out less likely to
be abused as two parties (developer and system administrator) have to consent to
each individual opt out. So it theory it sounds good. But I feel that doesn't
outweigh the fact that it would be complicated and hard to use. How about just
having two options: one to always honor GFP_SKIP_ZERO in the code and one to
always ignore it.
- Eric