Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Uladzislau Rezki <urezki@gmail.com>
Date:   Mon, 15 Feb 2021 13:51:54 +0100
To:     Topi Miettinen <toiwoton@gmail.com>
Cc:     Uladzislau Rezki <urezki@gmail.com>,
        linux-hardening@vger.kernel.org, akpm@linux-foundation.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Andy Lutomirski <luto@kernel.org>,
        Jann Horn <jannh@google.com>,
        Kees Cook <keescook@chromium.org>,
        Linux API <linux-api@vger.kernel.org>,
        Matthew Wilcox <willy@infradead.org>,
        Mike Rapoport <rppt@kernel.org>
Subject: Re: [PATCH v2] mm/vmalloc: randomize vmalloc() allocations
Message-ID: <20210215125154.GA2259@pc638.lan>
References: <20201212175635.4217-1-toiwoton@gmail.com>
 <795016db-93fa-72ea-f5dd-16b9c56a62e9@gmail.com>
 <20210213115521.GA1907@pc638.lan>
 <8d60ba59-1d16-bb76-f3d3-f1b0c5a5b306@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8d60ba59-1d16-bb76-f3d3-f1b0c5a5b306@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Sat, Feb 13, 2021 at 03:43:39PM +0200, Topi Miettinen wrote:
> On 13.2.2021 13.55, Uladzislau Rezki wrote:
> > > Hello,
> > > 
> > > Is there a chance of getting this reviewed and maybe even merged, please?
> > > 
> > > -Topi
> > > 
> > I can review it and help with it. But before that i would like to
> > clarify if such "randomization" is something that you can not leave?
> 
> This happens to interest me and I don't mind the performance loss since I
> think there's also an improvement in security. I suppose (perhaps wrongly)
> that others may also be interested in such features. For example, also
> `nosmt` can take away a big part of CPU processing capability.
>
OK. I was thinking about if it is done for some production systems or
some specific projects where this is highly demanded.

>
> Does this
> answer your question, I'm not sure what you mean with leaving? I hope you
> would not want me to go away and leave?
>
No-no, that was a type :) Sorry for that. I just wanted to figure out
who really needs it.

> > For example on 32bit system vmalloc space is limited, such randomization
> > can slow down it, also it will lead to failing of allocations much more,
> > thus it will require repeating with different offset.
> 
> I would not use `randomize_vmalloc=1` on a 32 bit systems, because in
> addition to slow down, the address space could become so fragmented that
> large allocations may not fit anymore. Perhaps the documentation should warn
> about this more clearly. I haven't tried this on a 32 bit system though and
> there the VM layout is very different.
> 
For 32-bit systems that would introduce many issues not limited to fragmentations.

> __alloc_vm_area() scans the vmalloc space starting from a random address up
> to end of the area. If this fails, the scan is restarted from the bottom of
> the area up to this random address. Thus the entire area is scanned.
> 
> > Second. There is a space or region for modules. Using various offsets
> > can waste of that memory, thus can lead to failing of module loading.
> 
> The allocations for modules (or BPF code) are also randomized within their
> dedicated space. I don't think other allocations should affect module space.
> Within this module space, fragmentation may also be possible because there's
> only 1,5GB available. The largest allocation on my system seems to be 11M at
> the moment, others are 1M or below and most are 8k. The possibility of an
> allocation failing probably depends on the fill ratio. In practice haven't
> seen problems with this.
> 
I think it depends on how many modules your system loads. If it is a big
system it might be that such fragmentation and wasting of module space
may lead to modules loading.

> It would be possible to have finer control, for example
> `randomize_vmalloc=3` (1 = general vmalloc, 2 = modules, bitwise ORed) or
> `randomize_vmalloc=general,modules`.
> 
> I experimented by trying to change how the modules are compiled
> (-mcmodel=medium or -mcmodel=large) so that they could be located in the
> normal vmalloc space, but instead I found a bug in the compiler (-mfentry
> produces incorrect code for -mcmodel=large, now fixed).
> 
> > On the other side there is a per-cpu allocator. Interfering with it
> > also will increase a rate of failing.
> 
> I didn't notice the per-cpu allocator before. I'm probably missing
> something, but it seems to be used for a different purpose (for allocating
> the vmap_area structure objects instead of the address space range), so
> where do you see interference?
> 


   A                       B
 ---->                   <----
<---------------------------><--------->
|   vmalloc address space    |
|<--------------------------->


A - is a vmalloc allocations;
B - is a percpu-allocator.

--
Vlad Rezki