2022-07-13 10:27:50

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/3] mm/vmalloc: introduce vmalloc_exec which allocates RO+X memory

On Wed, Jul 13, 2022 at 12:18:44AM -0700, Song Liu wrote:
> Dynamically allocated kernel texts, such as module texts, bpf programs,
> and ftrace trampolines, are used in more and more scenarios. Currently,
> these users allocate meory with module_alloc, fill the memory with text,
> and then use set_memory_[ro|x] to protect the memory.
>
> This approach has two issues:
> 1) each of these user occupies one or more RO+X page, and thus one or
> more entry in the page table and the iTLB;
> 2) frequent allocate/free of RO+X pages causes fragmentation of kernel
> direct map [1].
>
> BPF prog pack [2] addresses this from the BPF side. Now, make the same
> logic available to other users of dynamic kernel text.
>
> The new API is like:
>
> void *vmalloc_exec(size_t size);
> void vfree_exec(void *addr, size_t size);
>
> vmalloc_exec has different handling for small and big allocations
> (> PMD_SIZE * num_possible_nodes). bigger allocations have dedicated
> vmalloc allocation; while small allocations share a vmalloc_exec_pack
> with other allocations.
>
> Once allocated, the vmalloc_exec_pack is filled with invalid instructions

*sigh*, again, INT3 is a *VALID* instruction.

> and protected with RO+X. Some text_poke feature is required to make
> changes to the vmalloc_exec_pack. Therefore, vmalloc_exec requires changes
> from the arch (to provide text_poke family APIs), and the user (to use
> text poke APIs to make any changes to the memory).

I hate the naming; this isn't just vmalloc, this is a whole different
allocator build on top of things.

I'm also not convinced this is the right way to go about doing this;
much of the design here is because of how the module range is mixing
text and data and working around that.

So how about instead we separate them? Then much of the problem goes
away, you don't need to track these 2M chunks at all.

Start by adding VM_TOPDOWN_VMAP, which instead of returning the lowest
(leftmost) vmap_area that fits, picks the higests (rightmost).

Then add module_alloc_data() that uses VM_TOPDOWN_VMAP and make
ARCH_WANTS_MODULE_DATA_IN_VMALLOC use that instead of vmalloc (with a
weak function doing the vmalloc).

This gets you bottom of module range is RO+X only, top is shattered
between different !X types.

Then track the boundary between X and !X and ensure module_alloc_data()
and module_alloc() never cross over and stay strictly separated.

Then change all module_alloc() users to expect RO+X memory, instead of
RW.

Then make sure any extention of the X range is 2M aligned.

And presto, *everybody* always uses 2M TLB for text, modules, bpf,
ftrace, the lot and nobody is tracking chunks.

Maybe migration can be eased by instead providing module_alloc_text()
and ARCH_WANTS_MODULE_ALLOC_TEXT.


2022-07-13 16:54:09

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/3] mm/vmalloc: introduce vmalloc_exec which allocates RO+X memory



> On Jul 13, 2022, at 3:20 AM, Peter Zijlstra <[email protected]> wrote:
>
> On Wed, Jul 13, 2022 at 12:18:44AM -0700, Song Liu wrote:
>> Dynamically allocated kernel texts, such as module texts, bpf programs,
>> and ftrace trampolines, are used in more and more scenarios. Currently,
>> these users allocate meory with module_alloc, fill the memory with text,
>> and then use set_memory_[ro|x] to protect the memory.
>>
>> This approach has two issues:
>> 1) each of these user occupies one or more RO+X page, and thus one or
>> more entry in the page table and the iTLB;
>> 2) frequent allocate/free of RO+X pages causes fragmentation of kernel
>> direct map [1].
>>
>> BPF prog pack [2] addresses this from the BPF side. Now, make the same
>> logic available to other users of dynamic kernel text.
>>
>> The new API is like:
>>
>> void *vmalloc_exec(size_t size);
>> void vfree_exec(void *addr, size_t size);
>>
>> vmalloc_exec has different handling for small and big allocations
>> (> PMD_SIZE * num_possible_nodes). bigger allocations have dedicated
>> vmalloc allocation; while small allocations share a vmalloc_exec_pack
>> with other allocations.
>>
>> Once allocated, the vmalloc_exec_pack is filled with invalid instructions
>
> *sigh*, again, INT3 is a *VALID* instruction.

I am fully aware "invalid" or "illegal" is not accurate, but I am not
sure what to use. Shall we call them "safe" instructions?

>
>> and protected with RO+X. Some text_poke feature is required to make
>> changes to the vmalloc_exec_pack. Therefore, vmalloc_exec requires changes
>> from the arch (to provide text_poke family APIs), and the user (to use
>> text poke APIs to make any changes to the memory).
>
> I hate the naming; this isn't just vmalloc, this is a whole different
> allocator build on top of things.
>
> I'm also not convinced this is the right way to go about doing this;
> much of the design here is because of how the module range is mixing
> text and data and working around that.

Hmm.. I am not sure mixed data/text is the only problem here.

>
> So how about instead we separate them? Then much of the problem goes
> away, you don't need to track these 2M chunks at all.

If we manage the memory in < 2MiB granularity, either 4kB or smaller,
we still need some way to track which parts are being used, no? I mean
the bitmap.

>
> Start by adding VM_TOPDOWN_VMAP, which instead of returning the lowest
> (leftmost) vmap_area that fits, picks the higests (rightmost).
>
> Then add module_alloc_data() that uses VM_TOPDOWN_VMAP and make
> ARCH_WANTS_MODULE_DATA_IN_VMALLOC use that instead of vmalloc (with a
> weak function doing the vmalloc).
>
> This gets you bottom of module range is RO+X only, top is shattered
> between different !X types.
>
> Then track the boundary between X and !X and ensure module_alloc_data()
> and module_alloc() never cross over and stay strictly separated.
>
> Then change all module_alloc() users to expect RO+X memory, instead of
> RW.
>
> Then make sure any extention of the X range is 2M aligned.
>
> And presto, *everybody* always uses 2M TLB for text, modules, bpf,
> ftrace, the lot and nobody is tracking chunks.
>
> Maybe migration can be eased by instead providing module_alloc_text()
> and ARCH_WANTS_MODULE_ALLOC_TEXT.

If we have the text/data separation, can we just put text after _etext?

Right now, we allocate huge pages for _stext to round_down(_etext, 2MB),
and 4kB pages for round_down(_etext, 2MB) to round_up(_etext, 4kB). To
make this more efficient, we can allocate huge pages for _stext to
round_up(_etext, 2MB), and use _etext to round_up(_etext, 2MB) as the
first pool of memory for module_alloc_text(). Once we used all the
memory there, we allocate more huge pages after round_up(_etext, 2MB).

I am not sure how to make this work, but I guess this is similar to
the idea you are describing here? However, we will need some bitmap
to track the usage of these memory pools, right?

Thanks,
Song

2022-07-13 21:33:01

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/3] mm/vmalloc: introduce vmalloc_exec which allocates RO+X memory

On Wed, Jul 13, 2022 at 03:48:35PM +0000, Song Liu wrote:

> > So how about instead we separate them? Then much of the problem goes
> > away, you don't need to track these 2M chunks at all.
>
> If we manage the memory in < 2MiB granularity, either 4kB or smaller,
> we still need some way to track which parts are being used, no? I mean
> the bitmap.

I was thinking the vmalloc vmap_area tree could help out there.

2022-07-13 21:45:30

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/3] mm/vmalloc: introduce vmalloc_exec which allocates RO+X memory



> On Jul 13, 2022, at 1:26 PM, Peter Zijlstra <[email protected]> wrote:
>
> On Wed, Jul 13, 2022 at 03:48:35PM +0000, Song Liu wrote:
>
>>> So how about instead we separate them? Then much of the problem goes
>>> away, you don't need to track these 2M chunks at all.
>>
>> If we manage the memory in < 2MiB granularity, either 4kB or smaller,
>> we still need some way to track which parts are being used, no? I mean
>> the bitmap.
>
> I was thinking the vmalloc vmap_area tree could help out there.

Interesting. vmap_area tree indeed keeps a lot of useful information.

Currently, powerpc supports CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC,
which leaves module_alloc just for module text. If this works, we get
separation between RO+X and RW memory. What would it take to enable
CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC for x86_64?

Thanks,
Song

2022-07-14 05:31:39

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/3] mm/vmalloc: introduce vmalloc_exec which allocates RO+X memory

On Wed, Jul 13, 2022 at 12:20:09PM +0200, Peter Zijlstra wrote:
> Start by adding VM_TOPDOWN_VMAP, which instead of returning the lowest
> (leftmost) vmap_area that fits, picks the higests (rightmost).
>
> Then add module_alloc_data() that uses VM_TOPDOWN_VMAP and make
> ARCH_WANTS_MODULE_DATA_IN_VMALLOC use that instead of vmalloc (with a
> weak function doing the vmalloc).
>
> This gets you bottom of module range is RO+X only, top is shattered
> between different !X types.
>
> Then track the boundary between X and !X and ensure module_alloc_data()
> and module_alloc() never cross over and stay strictly separated.
>
> Then change all module_alloc() users to expect RO+X memory, instead of
> RW.
>
> Then make sure any extention of the X range is 2M aligned.
>
> And presto, *everybody* always uses 2M TLB for text, modules, bpf,
> ftrace, the lot and nobody is tracking chunks.
>
> Maybe migration can be eased by instead providing module_alloc_text()
> and ARCH_WANTS_MODULE_ALLOC_TEXT.

This all looks pretty sensible. How are we going to do the initial
write to the executable memory, though?

2022-07-14 07:47:01

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/3] mm/vmalloc: introduce vmalloc_exec which allocates RO+X memory

On Wed, Jul 13, 2022 at 10:16:36PM -0700, Christoph Hellwig wrote:
> On Wed, Jul 13, 2022 at 12:20:09PM +0200, Peter Zijlstra wrote:
> > Start by adding VM_TOPDOWN_VMAP, which instead of returning the lowest
> > (leftmost) vmap_area that fits, picks the higests (rightmost).
> >
> > Then add module_alloc_data() that uses VM_TOPDOWN_VMAP and make
> > ARCH_WANTS_MODULE_DATA_IN_VMALLOC use that instead of vmalloc (with a
> > weak function doing the vmalloc).
> >
> > This gets you bottom of module range is RO+X only, top is shattered
> > between different !X types.
> >
> > Then track the boundary between X and !X and ensure module_alloc_data()
> > and module_alloc() never cross over and stay strictly separated.
> >
> > Then change all module_alloc() users to expect RO+X memory, instead of
> > RW.
> >
> > Then make sure any extention of the X range is 2M aligned.
> >
> > And presto, *everybody* always uses 2M TLB for text, modules, bpf,
> > ftrace, the lot and nobody is tracking chunks.
> >
> > Maybe migration can be eased by instead providing module_alloc_text()
> > and ARCH_WANTS_MODULE_ALLOC_TEXT.
>
> This all looks pretty sensible. How are we going to do the initial
> write to the executable memory, though?

With something like text_poke_memcpy(). I suppose that the proposed
ARCH_WANTS_MODULE_ALLOC_TEXT needs to imply availability of that too.

If the 4K copy thing ends up being a bottleneck we can easily extend
that to have a 2M option as well.

2022-07-14 10:58:09

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/3] mm/vmalloc: introduce vmalloc_exec which allocates RO+X memory

On Wed, Jul 13, 2022 at 09:20:55PM +0000, Song Liu wrote:
>
>
> > On Jul 13, 2022, at 1:26 PM, Peter Zijlstra <[email protected]> wrote:
> >
> > On Wed, Jul 13, 2022 at 03:48:35PM +0000, Song Liu wrote:
> >
> >>> So how about instead we separate them? Then much of the problem goes
> >>> away, you don't need to track these 2M chunks at all.
> >>
> >> If we manage the memory in < 2MiB granularity, either 4kB or smaller,
> >> we still need some way to track which parts are being used, no? I mean
> >> the bitmap.
> >
> > I was thinking the vmalloc vmap_area tree could help out there.
>
> Interesting. vmap_area tree indeed keeps a lot of useful information.
>
> Currently, powerpc supports CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC,

Only PPC32; and it's due to a constraint in their MMU vs page
protections.

> which leaves module_alloc just for module text. If this works, we get
> separation between RO+X and RW memory. What would it take to enable
> CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC for x86_64?

The VM_TOPDOWN_VMAP flag and ensuring the data and code regions never
overlap. Once you have that you can enable it.

Specifically the problem is that data needs to be in the s32 immediate
range just like code, so we're constrained to the module range. Given
that constraint, the easiest solution is to use the different ends of
that range.

2022-08-05 05:32:16

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/3] mm/vmalloc: introduce vmalloc_exec which allocates RO+X memory

Hi Peter,

> On Jul 13, 2022, at 3:20 AM, Peter Zijlstra <[email protected]> wrote:
>

[...]

>
> So how about instead we separate them? Then much of the problem goes
> away, you don't need to track these 2M chunks at all.
>
> Start by adding VM_TOPDOWN_VMAP, which instead of returning the lowest
> (leftmost) vmap_area that fits, picks the higests (rightmost).
>
> Then add module_alloc_data() that uses VM_TOPDOWN_VMAP and make
> ARCH_WANTS_MODULE_DATA_IN_VMALLOC use that instead of vmalloc (with a
> weak function doing the vmalloc).
>
> This gets you bottom of module range is RO+X only, top is shattered
> between different !X types.
>
> Then track the boundary between X and !X and ensure module_alloc_data()
> and module_alloc() never cross over and stay strictly separated.
>
> Then change all module_alloc() users to expect RO+X memory, instead of
> RW.
>
> Then make sure any extention of the X range is 2M aligned.
>
> And presto, *everybody* always uses 2M TLB for text, modules, bpf,
> ftrace, the lot and nobody is tracking chunks.
>
> Maybe migration can be eased by instead providing module_alloc_text()
> and ARCH_WANTS_MODULE_ALLOC_TEXT.

I finally got some time to look into the code. A few questions:

1. AFAICT, vmap_area tree only works with PAGE_SIZE aligned addresses.
For the sharing to be more efficient, I think we need to go with
smaller granularity. Will this work? Shall we pick a smaller
granularity, say 64 bytes? Or shall we go all the way to 1 byte?

2. I think we will need multiple vmap_area's sharing the same vm_struct.
Do we need to add refcount to vm_struct?

Thanks,
Song



2022-08-05 05:32:16

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/3] mm/vmalloc: introduce vmalloc_exec which allocates RO+X memory

Hi Peter,

> On Jul 13, 2022, at 3:20 AM, Peter Zijlstra <[email protected]> wrote:
>

[...]

>
> So how about instead we separate them? Then much of the problem goes
> away, you don't need to track these 2M chunks at all.
>
> Start by adding VM_TOPDOWN_VMAP, which instead of returning the lowest
> (leftmost) vmap_area that fits, picks the higests (rightmost).
>
> Then add module_alloc_data() that uses VM_TOPDOWN_VMAP and make
> ARCH_WANTS_MODULE_DATA_IN_VMALLOC use that instead of vmalloc (with a
> weak function doing the vmalloc).
>
> This gets you bottom of module range is RO+X only, top is shattered
> between different !X types.
>
> Then track the boundary between X and !X and ensure module_alloc_data()
> and module_alloc() never cross over and stay strictly separated.
>
> Then change all module_alloc() users to expect RO+X memory, instead of
> RW.
>
> Then make sure any extention of the X range is 2M aligned.
>
> And presto, *everybody* always uses 2M TLB for text, modules, bpf,
> ftrace, the lot and nobody is tracking chunks.
>
> Maybe migration can be eased by instead providing module_alloc_text()
> and ARCH_WANTS_MODULE_ALLOC_TEXT.

I finally got some time to look into the code. A few questions:

1. AFAICT, vmap_area tree only works with PAGE_SIZE aligned addresses.
For the sharing to be more efficient, I think we need to go with
smaller granularity. Will this work? Shall we pick a smaller
granularity, say 64 bytes? Or shall we go all the way to 1 byte?

2. I think we will need multiple vmap_area's sharing the same vm_struct.
Do we need to add refcount to vm_struct?

Thanks,
Song