Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1547445 > unrolled thread
| Started by | "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> |
|---|---|
| First post | 2016-12-27 03:00 +0100 |
| Last post | 2017-01-05 18:00 +0100 |
| Articles | 20 on this page of 52 — 9 participants |
Back to article view | Back to linux.kernel
[PATCHv2 00/29] 5-level paging "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:00 +0100
[PATCHv2 17/29] x86/asm: remove __VIRTUAL_MASK_SHIFT==47 assert "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:00 +0100
[PATCHv2 03/29] asm-generic: introduce __ARCH_USE_5LEVEL_HACK "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:00 +0100
[RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:00 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2016-12-27 03:20 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-12-27 03:40 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2016-12-27 04:30 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-01-02 10:20 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Carlos O'Donell <carlos@redhat.com> - 2016-12-29 04:00 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2016-12-31 03:10 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-01-02 09:40 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "H.J. Lu" <hjl.tools@gmail.com> - 2017-01-13 21:20 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Arnd Bergmann <arnd@arndb.de> - 2017-01-02 09:50 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2017-01-03 07:10 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Arnd Bergmann <arnd@arndb.de> - 2017-01-03 14:30 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2017-01-03 19:40 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2017-01-03 23:10 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Arnd Bergmann <arnd@arndb.de> - 2017-01-04 15:00 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Arnd Bergmann <arnd@arndb.de> - 2017-01-03 23:20 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-01-03 17:10 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2017-01-03 19:30 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-01-04 15:30 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2017-01-05 19:20 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Dave Hansen <dave.hansen@intel.com> - 2017-01-05 20:20 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2017-01-05 20:40 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Dave Hansen <dave.hansen@intel.com> - 2017-01-05 20:40 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2017-01-05 21:20 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Dave Hansen <dave.hansen@intel.com> - 2017-01-05 21:50 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2017-01-05 22:30 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Dave Hansen <dave.hansen@intel.com> - 2017-01-06 00:20 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-01-11 15:30 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2017-01-11 19:10 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-01-11 19:40 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Dave Hansen <dave.hansen@intel.com> - 2017-01-11 20:00 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andy Lutomirski <luto@amacapital.net> - 2017-01-11 20:30 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Linus Torvalds <torvalds@linux-foundation.org> - 2017-01-11 20:40 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Andi Kleen <ak@linux.intel.com> - 2017-01-11 22:50 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-01-11 20:40 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Linus Torvalds <torvalds@linux-foundation.org> - 2017-01-11 20:40 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR Dave Hansen <dave.hansen@intel.com> - 2017-01-11 19:30 +0100
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-01-05 21:20 +0100
[PATCHv2 19/29] x86/paravirt: make paravirt code support 5-level paging "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:00 +0100
[PATCHv2 27/29] x86/mm: add support for 5-level paging for KASLR "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:00 +0100
[PATCHv2 12/29] x86/mm: add support of p4d_t in vmalloc_fault() "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:00 +0100
[PATCHv2 18/29] x86/mm: define virtual memory map for 5-level paging "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:00 +0100
[PATCHv2 04/29] arch, mm: convert all architectures to use 5level-fixup.h "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:10 +0100
[PATCHv2 02/29] asm-generic: introduce 5level-fixup.h "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:10 +0100
[PATCHv2 14/29] x86/kexec: support p4d_t "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:10 +0100
[PATCHv2 08/29] x86: basic changes into headers for 5-level paging "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:10 +0100
[PATCHv2 05/29] asm-generic: introduce <asm-generic/pgtable-nop4d.h> "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:10 +0100
[PATCHv2 15/29] x86: convert the rest of the code to support p4d_t "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-27 03:10 +0100
Re: [PATCHv2 00/29] 5-level paging "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-01-05 18:00 +0100
Page 2 of 3 — ← Prev page 1 [2] 3 Next page →
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2017-01-03 19:30 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sVC0p-1Og-5@gated-at.bofh.it> |
| In reply to | #1549912 |
On Tue, Jan 3, 2017 at 8:04 AM, Kirill A. Shutemov <kirill@shutemov.name> wrote: > On Mon, Jan 02, 2017 at 10:08:28PM -0800, Andy Lutomirski wrote: >> On Mon, Jan 2, 2017 at 12:44 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Tuesday, December 27, 2016 4:54:13 AM CET Kirill A. Shutemov wrote: >> >> As with other resources you can set the limit lower than current usage. >> >> It would affect only future virtual address space allocations. >> >> I still don't buy all these use cases: >> >> >> >> >> Use-cases for new rlimit: >> >> >> >> - Bumping the soft limit to RLIM_INFINITY, allows current process all >> >> its children to use addresses above 47-bits. >> >> OK, I get this, but only as a workaround for programs that make >> assumptions about the address space and don't use some mechanism (to >> be designed?) to work correctly in spite of a larger address space. > > I guess you've misread the case. It's opt-in for large adrress space, not > other way around. > > I believe 47-bit VA by default is right way to go to make the transition > without breaking userspace. What I meant was: setting the rlimit to anything other than -1ULL is a workaround, but otherwise I agree. This still makes little sense if set by PAM or other conventional rlimit tools. >> >> >> >> - Lowering the hard limit to 47-bits would prevent current process all >> >> its children to use addresses above 47-bits, unless a process has >> >> CAP_SYS_RESOURCES. >> >> I've tried and I can't imagine any reason to do this. > > That's just if something went wrong and we want to stop an application > from use addresses above 47-bit. But CAP_SYS_RESOURCES still makes no sense in this context. > >> >> - It’s also can be handy to lower hard or soft limit to arbitrary >> >> address. User-mode emulation in QEMU may lower the limit to 32-bit >> >> to emulate 32-bit machine on 64-bit host. >> >> I don't understand. QEMU user-mode emulation intercepts all syscalls. >> What QEMU would *actually* want is a way to say "allocate me some >> memory with the high N bits clear". mmap-via-int80 on x86 should be >> fixed to do this, but a new syscall with an explicit parameter would >> work, as would a prctl changing the current limit. > > Look at mess in mmap_find_vma(). QEmu has to guess where is free virtual > memory. That's unnessesary complex. > > prctl would work for this too. new-mmap would *not*: there are more ways > to allocate vitual address space: shmat(), mremap(). Changing all of them > just for this is stupid. Fair enough. Except that mmap-via-int80, shmat-via-int80, etc should still work (if I understand what qemu needs correctly), as would the prctl. > >> >> >> >> TODO: >> >> - port to non-x86; >> >> >> >> Not-yet-signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> >> >> Cc: linux-api@vger.kernel.org >> > >> > This seems to nicely address the same problem on arm64, which has >> > run into the same issue due to the various page table formats >> > that can currently be chosen at compile time. >> >> On further reflection, I think this has very little to do with paging >> formats except insofar as paging formats make us notice the problem. >> The issue is that user code wants to be able to assume an upper limit >> on an address, and it gets an upper limit right now that depends on >> architecture due to paging formats. But someone really might want to >> write a *portable* 64-bit program that allocates memory with the high >> 16 bits clear. So let's add such a mechanism directly. >> >> As a thought experiment, what if x86_64 simply never allocated "high" >> (above 2^47-1) addresses unless a new mmap-with-explicit-limit syscall >> were used? Old glibc would continue working. Old VMs would work. >> New programs that want to use ginormous mappings would have to use the >> new syscall. This would be totally stateless and would have no issues >> with CRIU. > > Except, we need more than mmap as I mentioned. > > And what about stack? I'm not sure that everybody would be happy with > stack in the middle of address space. I would, personally. I think that, for very large address spaces, we should allocate a large block of stack and get rid of the "stack grows down forever" legacy idea. Then we would never need to worry about the stack eventually hitting some other allocation. And 2^57 bytes is hilariously large for a default stack.
[toc] | [prev] | [next] | [standalone]
| From | "Kirill A. Shutemov" <kirill@shutemov.name> |
|---|---|
| Date | 2017-01-04 15:30 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sVUJI-5S7-49@gated-at.bofh.it> |
| In reply to | #1550065 |
On Tue, Jan 03, 2017 at 10:27:22AM -0800, Andy Lutomirski wrote: > On Tue, Jan 3, 2017 at 8:04 AM, Kirill A. Shutemov <kirill@shutemov.name> wrote: > > And what about stack? I'm not sure that everybody would be happy with > > stack in the middle of address space. > > I would, personally. I think that, for very large address spaces, we > should allocate a large block of stack and get rid of the "stack grows > down forever" legacy idea. Then we would never need to worry about > the stack eventually hitting some other allocation. And 2^57 bytes is > hilariously large for a default stack. The stack in the middle of address space can prevent creating other huuuge contiguous mapping. Databases may want this. -- Kirill A. Shutemov
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2017-01-05 19:20 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sWkNQ-6Cu-15@gated-at.bofh.it> |
| In reply to | #1550867 |
On Wed, Jan 4, 2017 at 6:19 AM, Kirill A. Shutemov <kirill@shutemov.name> wrote: > On Tue, Jan 03, 2017 at 10:27:22AM -0800, Andy Lutomirski wrote: >> On Tue, Jan 3, 2017 at 8:04 AM, Kirill A. Shutemov <kirill@shutemov.name> wrote: >> > And what about stack? I'm not sure that everybody would be happy with >> > stack in the middle of address space. >> >> I would, personally. I think that, for very large address spaces, we >> should allocate a large block of stack and get rid of the "stack grows >> down forever" legacy idea. Then we would never need to worry about >> the stack eventually hitting some other allocation. And 2^57 bytes is >> hilariously large for a default stack. > > The stack in the middle of address space can prevent creating other huuuge > contiguous mapping. Databases may want this. Fair enough. OTOH, 2^47 is nowhere near the middle if we were to put it near the top of the legacy address space.
[toc] | [prev] | [next] | [standalone]
| From | Dave Hansen <dave.hansen@intel.com> |
|---|---|
| Date | 2017-01-05 20:20 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sWlJT-7ej-9@gated-at.bofh.it> |
| In reply to | #1547448 |
On 12/26/2016 05:54 PM, Kirill A. Shutemov wrote: > MM would use min(RLIMIT_VADDR, TASK_SIZE) as upper limit of virtual > address available to map by userspace. What happens to existing mappings above the limit when this upper limit is dropped? Similarly, why do we do with an application running with something incompatible with the larger address space that tries to raise the limit? Say, legacy MPX.
[toc] | [prev] | [next] | [standalone]
| From | "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> |
|---|---|
| Date | 2017-01-05 20:40 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sWm3f-7kY-15@gated-at.bofh.it> |
| In reply to | #1552256 |
On Thu, Jan 05, 2017 at 11:13:57AM -0800, Dave Hansen wrote: > On 12/26/2016 05:54 PM, Kirill A. Shutemov wrote: > > MM would use min(RLIMIT_VADDR, TASK_SIZE) as upper limit of virtual > > address available to map by userspace. > > What happens to existing mappings above the limit when this upper limit > is dropped? Nothing: we only prevent creating new mappings. All existing are not affected. The semantics here the same as with other resource limits. > Similarly, why do we do with an application running with something > incompatible with the larger address space that tries to raise the > limit? Say, legacy MPX. It has to know what it does. Yes, it can change limit to the point where application is unusable. But you can to the same with other limits. -- Kirill A. Shutemov
[toc] | [prev] | [next] | [standalone]
| From | Dave Hansen <dave.hansen@intel.com> |
|---|---|
| Date | 2017-01-05 20:40 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sWm3f-7kY-17@gated-at.bofh.it> |
| In reply to | #1552270 |
On 01/05/2017 11:29 AM, Kirill A. Shutemov wrote: > On Thu, Jan 05, 2017 at 11:13:57AM -0800, Dave Hansen wrote: >> On 12/26/2016 05:54 PM, Kirill A. Shutemov wrote: >>> MM would use min(RLIMIT_VADDR, TASK_SIZE) as upper limit of virtual >>> address available to map by userspace. >> >> What happens to existing mappings above the limit when this upper limit >> is dropped? > > Nothing: we only prevent creating new mappings. All existing are not > affected. > > The semantics here the same as with other resource limits. > >> Similarly, why do we do with an application running with something >> incompatible with the larger address space that tries to raise the >> limit? Say, legacy MPX. > > It has to know what it does. Yes, it can change limit to the point where > application is unusable. But you can to the same with other limits. I'm not sure I'm comfortable with this. Do other rlimit changes cause silent data corruption? I'm pretty sure doing this to MPX would.
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2017-01-05 21:20 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sWmFY-7Qg-15@gated-at.bofh.it> |
| In reply to | #1552274 |
On Thu, Jan 5, 2017 at 11:39 AM, Dave Hansen <dave.hansen@intel.com> wrote: > On 01/05/2017 11:29 AM, Kirill A. Shutemov wrote: >> On Thu, Jan 05, 2017 at 11:13:57AM -0800, Dave Hansen wrote: >>> On 12/26/2016 05:54 PM, Kirill A. Shutemov wrote: >>>> MM would use min(RLIMIT_VADDR, TASK_SIZE) as upper limit of virtual >>>> address available to map by userspace. >>> >>> What happens to existing mappings above the limit when this upper limit >>> is dropped? >> >> Nothing: we only prevent creating new mappings. All existing are not >> affected. >> >> The semantics here the same as with other resource limits. >> >>> Similarly, why do we do with an application running with something >>> incompatible with the larger address space that tries to raise the >>> limit? Say, legacy MPX. >> >> It has to know what it does. Yes, it can change limit to the point where >> application is unusable. But you can to the same with other limits. > > I'm not sure I'm comfortable with this. Do other rlimit changes cause > silent data corruption? I'm pretty sure doing this to MPX would. > What actually goes wrong in this case? That is, what combination of MPX setup of subsequent allocations will cause a problem, and is the problem worse than just a segfault? IMO it would be really nice to keep the messy case confined to MPX. FWIW, this problem is kind of generic. If you run code in a process, MPX or otherwise, that assumes something about pointer values and then create a pointer that violates its assumptions, you will cause problems. For example, some VMs use high bits to store metadata. If you feed a pointer that's too big to such code, boom. This is exactly why high addresses need to be opt-in.
[toc] | [prev] | [next] | [standalone]
| From | Dave Hansen <dave.hansen@intel.com> |
|---|---|
| Date | 2017-01-05 21:50 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sWn8Z-83K-11@gated-at.bofh.it> |
| In reply to | #1552302 |
On 01/05/2017 12:14 PM, Andy Lutomirski wrote: >> I'm not sure I'm comfortable with this. Do other rlimit changes cause >> silent data corruption? I'm pretty sure doing this to MPX would. >> > What actually goes wrong in this case? That is, what combination of > MPX setup of subsequent allocations will cause a problem, and is the > problem worse than just a segfault? IMO it would be really nice to > keep the messy case confined to MPX. The MPX bounds tables are indexed by virtual address. They need to grow if the virtual address space grows. There's an MSR that controls whether we use the 48-bit or 57-bit layout. It basically decides whether we need a 2GB (48-bit) or 1TB (57-bit) bounds directory. The question is what we do with legacy MPX applications. We obviously can't let them just allocate a 2GB table and then go let the hardware pretend it's 1TB in size. We also can't hand the hardware using a 2GB table an address >48-bits. Ideally, I'd like to make sure that legacy MPX can't be enabled if this RLIMIT is set over 48-bits (really 47). I'd also like to make sure that legacy MPX is active, that the RLIMIT can't be raised because all hell will break loose when the new addresses show up. Remember, we already have (legacy MPX) binaries in the wild that have no knowledge of this stuff. So, we can implicitly have the kernel bump this rlimit around, but we can't expect userspace to do it, ever.
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2017-01-05 22:30 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sWnLH-83-5@gated-at.bofh.it> |
| In reply to | #1552321 |
On Thu, Jan 5, 2017 at 12:49 PM, Dave Hansen <dave.hansen@intel.com> wrote: > On 01/05/2017 12:14 PM, Andy Lutomirski wrote: >>> I'm not sure I'm comfortable with this. Do other rlimit changes cause >>> silent data corruption? I'm pretty sure doing this to MPX would. >>> >> What actually goes wrong in this case? That is, what combination of >> MPX setup of subsequent allocations will cause a problem, and is the >> problem worse than just a segfault? IMO it would be really nice to >> keep the messy case confined to MPX. > > The MPX bounds tables are indexed by virtual address. They need to grow > if the virtual address space grows. There's an MSR that controls > whether we use the 48-bit or 57-bit layout. It basically decides > whether we need a 2GB (48-bit) or 1TB (57-bit) bounds directory. > > The question is what we do with legacy MPX applications. We obviously > can't let them just allocate a 2GB table and then go let the hardware > pretend it's 1TB in size. We also can't hand the hardware using a 2GB > table an address >48-bits. > > Ideally, I'd like to make sure that legacy MPX can't be enabled if this > RLIMIT is set over 48-bits (really 47). I'd also like to make sure that > legacy MPX is active, that the RLIMIT can't be raised because all hell > will break loose when the new addresses show up. > > Remember, we already have (legacy MPX) binaries in the wild that have no > knowledge of this stuff. So, we can implicitly have the kernel bump > this rlimit around, but we can't expect userspace to do it, ever. If you s/rlimit/prctl, then I think this all makes sense with one exception. It would be a bit sad if the personality-setting tool didn't work if compiled with MPX. So what if we had a second prctl field that is the value that kicks in after execve()? --Andy
[toc] | [prev] | [next] | [standalone]
| From | Dave Hansen <dave.hansen@intel.com> |
|---|---|
| Date | 2017-01-06 00:20 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sWpua-1ii-15@gated-at.bofh.it> |
| In reply to | #1552340 |
On 01/05/2017 01:27 PM, Andy Lutomirski wrote: > On Thu, Jan 5, 2017 at 12:49 PM, Dave Hansen <dave.hansen@intel.com> wrote: ... >> Remember, we already have (legacy MPX) binaries in the wild that have no >> knowledge of this stuff. So, we can implicitly have the kernel bump >> this rlimit around, but we can't expect userspace to do it, ever. > > If you s/rlimit/prctl, then I think this all makes sense with one > exception. It would be a bit sad if the personality-setting tool > didn't work if compiled with MPX. Ahh, because if you have MPX enabled you *can't* sanely switch between the two modes because you suddenly go from having small bounds tables to having big ones? It's not the simplest thing in the world to do, but there's nothing keeping the personality-setting tool from doing all the work. It can do: new_bd = malloc(1TB); prctl(MPX_DISABLE_MANAGEMENT); memcpy(new_bd, old_bd, LEGACY_MPX_BD_SIZE); set_bounds_config(new_bd | ENABLE_BIT); prctl(WIDER_VADDR_WIDTH); prctl(MPX_ENABLE_MANAGEMENT); > So what if we had a second prctl field that is the value that kicks in > after execve()? Yeah, that's a pretty sane way to do it too. execve() is a nice chokepoint.
[toc] | [prev] | [next] | [standalone]
| From | "Kirill A. Shutemov" <kirill@shutemov.name> |
|---|---|
| Date | 2017-01-11 15:30 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYs4y-7ZN-17@gated-at.bofh.it> |
| In reply to | #1552321 |
On Thu, Jan 05, 2017 at 12:49:44PM -0800, Dave Hansen wrote:
> On 01/05/2017 12:14 PM, Andy Lutomirski wrote:
> >> I'm not sure I'm comfortable with this. Do other rlimit changes cause
> >> silent data corruption? I'm pretty sure doing this to MPX would.
> >>
> > What actually goes wrong in this case? That is, what combination of
> > MPX setup of subsequent allocations will cause a problem, and is the
> > problem worse than just a segfault? IMO it would be really nice to
> > keep the messy case confined to MPX.
>
> The MPX bounds tables are indexed by virtual address. They need to grow
> if the virtual address space grows. There's an MSR that controls
> whether we use the 48-bit or 57-bit layout. It basically decides
> whether we need a 2GB (48-bit) or 1TB (57-bit) bounds directory.
>
> The question is what we do with legacy MPX applications. We obviously
> can't let them just allocate a 2GB table and then go let the hardware
> pretend it's 1TB in size. We also can't hand the hardware using a 2GB
> table an address >48-bits.
>
> Ideally, I'd like to make sure that legacy MPX can't be enabled if this
> RLIMIT is set over 48-bits (really 47). I'd also like to make sure that
> legacy MPX is active, that the RLIMIT can't be raised because all hell
> will break loose when the new addresses show up.
I think we can do this. See the patch below.
Basically, we refuse to enable MPX and issue warning in dmesg if there's
anything mapped above 47-bits. Once MPX is enabled, mmap_max_addr() cannot
be higher than 47-bits too.
Function call from mmap_max_addr() is unfortunate, but I don't see a
way around.
As we add support of MAWA it will get somewhat more complex, but general
idea should be the same.
Build-tested only.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 07cc4f27ca41..f97b149145f8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1742,7 +1742,6 @@ config X86_SMAP
config X86_INTEL_MPX
prompt "Intel MPX (Memory Protection Extensions)"
def_bool n
- depends on !X86_5LEVEL
depends on CPU_SUP_INTEL
---help---
MPX provides hardware features that can be used in
diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
index 0b416d4cf73b..ba9005f9bf87 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -56,11 +56,8 @@
#ifdef CONFIG_X86_INTEL_MPX
siginfo_t *mpx_generate_siginfo(struct pt_regs *regs);
+int kernel_managing_mpx_tables(struct mm_struct *mm);
int mpx_handle_bd_fault(void);
-static inline int kernel_managing_mpx_tables(struct mm_struct *mm)
-{
- return (mm->context.bd_addr != MPX_INVALID_BOUNDS_DIR);
-}
static inline void mpx_mm_init(struct mm_struct *mm)
{
/*
@@ -80,10 +77,6 @@ static inline int mpx_handle_bd_fault(void)
{
return -EINVAL;
}
-static inline int kernel_managing_mpx_tables(struct mm_struct *mm)
-{
- return 0;
-}
static inline void mpx_mm_init(struct mm_struct *mm)
{
}
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index e02917126859..589610a4f099 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -869,6 +869,7 @@ extern int set_tsc_mode(unsigned int val);
#ifdef CONFIG_X86_INTEL_MPX
extern int mpx_enable_management(void);
extern int mpx_disable_management(void);
+extern int kernel_managing_mpx_tables(struct mm_struct *mm);
#else
static inline int mpx_enable_management(void)
{
@@ -878,8 +879,22 @@ static inline int mpx_disable_management(void)
{
return -EINVAL;
}
+static inline int kernel_managing_mpx_tables(struct mm_struct *mm)
+{
+ return 0;
+}
#endif /* CONFIG_X86_INTEL_MPX */
+#define mmap_max_addr() \
+({ \
+ unsigned long max_addr = min(TASK_SIZE, rlimit(RLIMIT_VADDR)); \
+ /* At the moment, MPX cannot handle addresses above 47-bits */ \
+ if (max_addr > USER_VADDR_LIM && \
+ kernel_managing_mpx_tables(current->mm)) \
+ max_addr = USER_VADDR_LIM; \
+ max_addr; \
+})
+
extern u16 amd_get_nb_id(int cpu);
extern u32 amd_get_nodes_per_socket(void);
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index 324e5713d386..04fa386a165a 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -354,10 +354,22 @@ int mpx_enable_management(void)
*/
bd_base = mpx_get_bounds_dir();
down_write(&mm->mmap_sem);
+
+ /*
+ * MPX doesn't support addresses above 47-bits yes.
+ * Make sure nothing is mapped there before enabling.
+ */
+ if (find_vma(mm, 1UL << 47)) {
+ pr_warn("%s (%d): MPX cannot handle addresses above 47-bits. "
+ "Disabling.", current->comm, current->pid);
+ ret = -ENXIO;
+ goto out;
+ }
+
mm->context.bd_addr = bd_base;
if (mm->context.bd_addr == MPX_INVALID_BOUNDS_DIR)
ret = -ENXIO;
-
+out:
up_write(&mm->mmap_sem);
return ret;
}
@@ -516,6 +528,11 @@ static int do_mpx_bt_fault(void)
return allocate_bt(mm, (long __user *)bd_entry);
}
+int kernel_managing_mpx_tables(struct mm_struct *mm)
+{
+ return (mm->context.bd_addr != MPX_INVALID_BOUNDS_DIR);
+}
+
int mpx_handle_bd_fault(void)
{
/*
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f0f23afe0838..d463b800d8ce 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -3661,9 +3661,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
void cpufreq_remove_update_util_hook(int cpu);
#endif /* CONFIG_CPU_FREQ */
+#ifndef mmap_max_addr
+#define mmap_max_addr mmap_max_addr
static inline unsigned long mmap_max_addr(void)
{
return min(TASK_SIZE, rlimit(RLIMIT_VADDR));
}
+#endif
#endif
--
Kirill A. Shutemov
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2017-01-11 19:10 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYvvs-1Kq-13@gated-at.bofh.it> |
| In reply to | #1556520 |
On Wed, Jan 11, 2017 at 6:29 AM, Kirill A. Shutemov <kirill@shutemov.name> wrote: > On Thu, Jan 05, 2017 at 12:49:44PM -0800, Dave Hansen wrote: >> On 01/05/2017 12:14 PM, Andy Lutomirski wrote: >> >> I'm not sure I'm comfortable with this. Do other rlimit changes cause >> >> silent data corruption? I'm pretty sure doing this to MPX would. >> >> >> > What actually goes wrong in this case? That is, what combination of >> > MPX setup of subsequent allocations will cause a problem, and is the >> > problem worse than just a segfault? IMO it would be really nice to >> > keep the messy case confined to MPX. >> >> The MPX bounds tables are indexed by virtual address. They need to grow >> if the virtual address space grows. There's an MSR that controls >> whether we use the 48-bit or 57-bit layout. It basically decides >> whether we need a 2GB (48-bit) or 1TB (57-bit) bounds directory. >> >> The question is what we do with legacy MPX applications. We obviously >> can't let them just allocate a 2GB table and then go let the hardware >> pretend it's 1TB in size. We also can't hand the hardware using a 2GB >> table an address >48-bits. >> >> Ideally, I'd like to make sure that legacy MPX can't be enabled if this >> RLIMIT is set over 48-bits (really 47). I'd also like to make sure that >> legacy MPX is active, that the RLIMIT can't be raised because all hell >> will break loose when the new addresses show up. > > I think we can do this. See the patch below. > > Basically, we refuse to enable MPX and issue warning in dmesg if there's > anything mapped above 47-bits. Once MPX is enabled, mmap_max_addr() cannot > be higher than 47-bits too. > > Function call from mmap_max_addr() is unfortunate, but I don't see a > way around. How about preventing the max addr from being changed to too high a value while MPX is on instead of overriding the set value? This would have the added benefit that it would prevent silent failures where you think you've enabled large addresses but MPX is also on and mmap refuses to return large addresses.
[toc] | [prev] | [next] | [standalone]
| From | "Kirill A. Shutemov" <kirill@shutemov.name> |
|---|---|
| Date | 2017-01-11 19:40 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYvYu-1Uj-35@gated-at.bofh.it> |
| In reply to | #1556795 |
On Wed, Jan 11, 2017 at 10:09:17AM -0800, Andy Lutomirski wrote: > On Wed, Jan 11, 2017 at 6:29 AM, Kirill A. Shutemov > <kirill@shutemov.name> wrote: > > On Thu, Jan 05, 2017 at 12:49:44PM -0800, Dave Hansen wrote: > >> On 01/05/2017 12:14 PM, Andy Lutomirski wrote: > >> >> I'm not sure I'm comfortable with this. Do other rlimit changes cause > >> >> silent data corruption? I'm pretty sure doing this to MPX would. > >> >> > >> > What actually goes wrong in this case? That is, what combination of > >> > MPX setup of subsequent allocations will cause a problem, and is the > >> > problem worse than just a segfault? IMO it would be really nice to > >> > keep the messy case confined to MPX. > >> > >> The MPX bounds tables are indexed by virtual address. They need to grow > >> if the virtual address space grows. There's an MSR that controls > >> whether we use the 48-bit or 57-bit layout. It basically decides > >> whether we need a 2GB (48-bit) or 1TB (57-bit) bounds directory. > >> > >> The question is what we do with legacy MPX applications. We obviously > >> can't let them just allocate a 2GB table and then go let the hardware > >> pretend it's 1TB in size. We also can't hand the hardware using a 2GB > >> table an address >48-bits. > >> > >> Ideally, I'd like to make sure that legacy MPX can't be enabled if this > >> RLIMIT is set over 48-bits (really 47). I'd also like to make sure that > >> legacy MPX is active, that the RLIMIT can't be raised because all hell > >> will break loose when the new addresses show up. > > > > I think we can do this. See the patch below. > > > > Basically, we refuse to enable MPX and issue warning in dmesg if there's > > anything mapped above 47-bits. Once MPX is enabled, mmap_max_addr() cannot > > be higher than 47-bits too. > > > > Function call from mmap_max_addr() is unfortunate, but I don't see a > > way around. > > How about preventing the max addr from being changed to too high a > value while MPX is on instead of overriding the set value? This would > have the added benefit that it would prevent silent failures where you > think you've enabled large addresses but MPX is also on and mmap > refuses to return large addresses. Setting rlimit high doesn't mean that you necessary will get access to full address space, even without MPX in picture. TASK_SIZE limits the available address space too. I think it's consistent with other resources in rlimit: setting RLIMIT_RSS to unlimited doesn't really means you are not subject to other resource management. -- Kirill A. Shutemov
[toc] | [prev] | [next] | [standalone]
| From | Dave Hansen <dave.hansen@intel.com> |
|---|---|
| Date | 2017-01-11 20:00 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYwhQ-21a-45@gated-at.bofh.it> |
| In reply to | #1556823 |
On 01/11/2017 10:37 AM, Kirill A. Shutemov wrote: >> How about preventing the max addr from being changed to too high a >> value while MPX is on instead of overriding the set value? This would >> have the added benefit that it would prevent silent failures where you >> think you've enabled large addresses but MPX is also on and mmap >> refuses to return large addresses. > Setting rlimit high doesn't mean that you necessary will get access to > full address space, even without MPX in picture. TASK_SIZE limits the > available address space too. OK, sure... If you want to take another mechanism into account with respect to MPX, we can do that. We'd just need to change every mechanism we want to support to ensure that it can't transition in ways that break MPX. What are you arguing here, though? Since we *might* be limited by something else that we should not care about controlling the rlimit? > I think it's consistent with other resources in rlimit: setting RLIMIT_RSS > to unlimited doesn't really means you are not subject to other resource > management. The farther we get into this, the more and more I think using an rlimit is a horrible idea. Its semantics aren't a great match, and you seem to be resistant to making *this* rlimit differ from the others when there's an entirely need to do so. We're already being bitten by "legacy" rlimit. IOW, being consistent with *other* rlimit behavior buys us nothing, only complexity.
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2017-01-11 20:30 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYwKR-2sb-1@gated-at.bofh.it> |
| In reply to | #1556848 |
On Wed, Jan 11, 2017 at 10:49 AM, Dave Hansen <dave.hansen@intel.com> wrote: > On 01/11/2017 10:37 AM, Kirill A. Shutemov wrote: >>> How about preventing the max addr from being changed to too high a >>> value while MPX is on instead of overriding the set value? This would >>> have the added benefit that it would prevent silent failures where you >>> think you've enabled large addresses but MPX is also on and mmap >>> refuses to return large addresses. >> Setting rlimit high doesn't mean that you necessary will get access to >> full address space, even without MPX in picture. TASK_SIZE limits the >> available address space too. > > OK, sure... If you want to take another mechanism into account with > respect to MPX, we can do that. We'd just need to change every > mechanism we want to support to ensure that it can't transition in ways > that break MPX. > > What are you arguing here, though? Since we *might* be limited by > something else that we should not care about controlling the rlimit? > >> I think it's consistent with other resources in rlimit: setting RLIMIT_RSS >> to unlimited doesn't really means you are not subject to other resource >> management. > > The farther we get into this, the more and more I think using an rlimit > is a horrible idea. Its semantics aren't a great match, and you seem to > be resistant to making *this* rlimit differ from the others when there's > an entirely need to do so. We're already being bitten by "legacy" > rlimit. IOW, being consistent with *other* rlimit behavior buys us > nothing, only complexity. Taking a step back, I think it would be fantastic if we could find a way to make this work without any inheritable settings at all. Perhaps we could have a per-mm value that is initialized to 2^47-1 on execve() and can be raised by ELF note or by prctl()? Getting it right for 32-bit would require a bit of thought. The ELF note would make a high stack possible and, without the ELF note, we'd get a low stack but high mmap(). Then the messy bits can be glibc's problem and a toolchain problem as it should be, given that the only reason we need a limit at all is because of messy userspace code. Sure, the low stack prevents the *whole* address space from being used in one big block for databases, but 2^57 - 2^47 ought to be good enough. I'm not 100% sure this is workable but, if it is, it makes everyone's life easier. There's no need to muck around with setarch(1) or similar hacks.
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-01-11 20:40 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYwUx-2va-5@gated-at.bofh.it> |
| In reply to | #1556861 |
On Wed, Jan 11, 2017 at 11:20 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> Taking a step back, I think it would be fantastic if we could find a
> way to make this work without any inheritable settings at all.
> Perhaps we could have a per-mm value that is initialized to 2^47-1 on
> execve() and can be raised by ELF note or by prctl()?
I definitely think this is the right model. No inheritable settings,
no suid issues, no worries. Make people who want the large address
space (and there aren't going to be a lot of them) just mark their
binaries at compile time.
And as to the stack location: I think it should just be the same
regardless - up in "high" virtual memory in the 47-bit model. Because
as you say, if you actually end up having 57 bits of address space,
that still gives you basically the whole VM for data mappings -
they'll just be up above the stack.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Andi Kleen <ak@linux.intel.com> |
|---|---|
| Date | 2017-01-11 22:50 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYyWl-3GV-1@gated-at.bofh.it> |
| In reply to | #1556864 |
On Wed, Jan 11, 2017 at 11:31:25AM -0800, Linus Torvalds wrote: > On Wed, Jan 11, 2017 at 11:20 AM, Andy Lutomirski <luto@amacapital.net> wrote: > > > > Taking a step back, I think it would be fantastic if we could find a > > way to make this work without any inheritable settings at all. > > Perhaps we could have a per-mm value that is initialized to 2^47-1 on > > execve() and can be raised by ELF note or by prctl()? > > I definitely think this is the right model. No inheritable settings, > no suid issues, no worries. Make people who want the large address > space (and there aren't going to be a lot of them) just mark their > binaries at compile time. Compile time is inconvenient if you want to test some existing random binary if it works. I tried to write a tool which patched ELF notes into binaries some time ago for another project, but it ran into difficulties and didn't work everywhere. An inheritance scheme is much nicer for such use cases. -Andi
[toc] | [prev] | [next] | [standalone]
| From | "Kirill A. Shutemov" <kirill@shutemov.name> |
|---|---|
| Date | 2017-01-11 20:40 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYwUx-2va-3@gated-at.bofh.it> |
| In reply to | #1556861 |
On Wed, Jan 11, 2017 at 11:20:38AM -0800, Andy Lutomirski wrote: > On Wed, Jan 11, 2017 at 10:49 AM, Dave Hansen <dave.hansen@intel.com> wrote: > > On 01/11/2017 10:37 AM, Kirill A. Shutemov wrote: > >>> How about preventing the max addr from being changed to too high a > >>> value while MPX is on instead of overriding the set value? This would > >>> have the added benefit that it would prevent silent failures where you > >>> think you've enabled large addresses but MPX is also on and mmap > >>> refuses to return large addresses. > >> Setting rlimit high doesn't mean that you necessary will get access to > >> full address space, even without MPX in picture. TASK_SIZE limits the > >> available address space too. > > > > OK, sure... If you want to take another mechanism into account with > > respect to MPX, we can do that. We'd just need to change every > > mechanism we want to support to ensure that it can't transition in ways > > that break MPX. > > > > What are you arguing here, though? Since we *might* be limited by > > something else that we should not care about controlling the rlimit? > > > >> I think it's consistent with other resources in rlimit: setting RLIMIT_RSS > >> to unlimited doesn't really means you are not subject to other resource > >> management. > > > > The farther we get into this, the more and more I think using an rlimit > > is a horrible idea. Its semantics aren't a great match, and you seem to > > be resistant to making *this* rlimit differ from the others when there's > > an entirely need to do so. We're already being bitten by "legacy" > > rlimit. IOW, being consistent with *other* rlimit behavior buys us > > nothing, only complexity. > > Taking a step back, I think it would be fantastic if we could find a > way to make this work without any inheritable settings at all. > Perhaps we could have a per-mm value that is initialized to 2^47-1 on > execve() and can be raised by ELF note or by prctl()? One thing that inheritance give us is ability to change available address space from outside of binary. Both ELF note and prctl() doesn't really work here. Running legacy binary with full address space is valuable option. As well as limiting address space for binary with ELF note or prctl() in case of breakage in a field. Sure, we can use personality(2) or invent other interface for this. But to me rlimit covers both normal and emergency use-cases relatively well. -- Kirill A. Shutemov
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-01-11 20:40 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYwUx-2va-9@gated-at.bofh.it> |
| In reply to | #1556865 |
On Wed, Jan 11, 2017 at 11:32 AM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
>
> Running legacy binary with full address space is valuable option.
I disagree.
It's simply not valuable enough to worry about. Especially when there
is a fairly trivial wrapper approach: just make a full-address-space
wrapper than acts as a binary loader (think "specialized ld.so").
Sure, the wrapper may be "fairly trivial" but not necessarily
pleasant: you have to parse ELF sections etc and basically load the
binary by hand. But there are libraries for that, and loading an ELF
executable isn't rocket surgery, it's just possibly tedious.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Dave Hansen <dave.hansen@intel.com> |
|---|---|
| Date | 2017-01-11 19:30 +0100 |
| Subject | Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR |
| Message-ID | <sYvOO-1QW-13@gated-at.bofh.it> |
| In reply to | #1556520 |
On 01/11/2017 06:29 AM, Kirill A. Shutemov wrote:
> +#define mmap_max_addr() \
> +({ \
> + unsigned long max_addr = min(TASK_SIZE, rlimit(RLIMIT_VADDR)); \
> + /* At the moment, MPX cannot handle addresses above 47-bits */ \
> + if (max_addr > USER_VADDR_LIM && \
> + kernel_managing_mpx_tables(current->mm)) \
> + max_addr = USER_VADDR_LIM; \
> + max_addr; \
> +})
The bad part about this is that it adds code to a relatively fast path,
and the check that it's doing will not change its result for basically
the entire life of the process.
I'd much rather see this checking done at the point that MPX is enabled
and at the point the limit is changed. Those are both super-rare paths.
> extern u16 amd_get_nb_id(int cpu);
> extern u32 amd_get_nodes_per_socket(void);
>
> diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
> index 324e5713d386..04fa386a165a 100644
> --- a/arch/x86/mm/mpx.c
> +++ b/arch/x86/mm/mpx.c
> @@ -354,10 +354,22 @@ int mpx_enable_management(void)
> */
> bd_base = mpx_get_bounds_dir();
> down_write(&mm->mmap_sem);
> +
> + /*
> + * MPX doesn't support addresses above 47-bits yes.
> + * Make sure nothing is mapped there before enabling.
> + */
> + if (find_vma(mm, 1UL << 47)) {
> + pr_warn("%s (%d): MPX cannot handle addresses above 47-bits. "
> + "Disabling.", current->comm, current->pid);
> + ret = -ENXIO;
> + goto out;
> + }
I don't think allowing userspace to spam unlimited amounts of message
into the kernel log is a good idea. :) But a WARN_ONCE() might not kill
any puppies.
[toc] | [prev] | [next] | [standalone]
Page 2 of 3 — ← Prev page 1 [2] 3 Next page →
Back to top | Article view | linux.kernel
csiph-web