Re: [PATCH v1] mm, sysctl: Add sysctl for controlling VM_MAYEXEC taint

[Robert Foss]

On 2016-08-29 11:25 AM, Will Drewry wrote:



On Fri, Aug 26, 2016 at 4:32 PM, Kirill A. Shutemov
> wrote:

On Fri, Aug 26, 2016 at 12:30:04PM -0400, robert.f...@collabora.com
 wrote:
> From: Will Drewry >
>
> This patch proposes a sysctl knob that allows a privileged user to
> disable ~VM_MAYEXEC tainting when mapping in a vma from a MNT_NOEXEC
> mountpoint.  It does not alter the normal behavior resulting from
> attempting to directly mmap(PROT_EXEC) a vma (-EPERM) nor the behavior
> of any other subsystems checking MNT_NOEXEC.

Wouldn't it be equal to remounting all filesystems without noexec from
attacker POV? It's hardly a fence to make additional mprotect(PROT_EXEC)
call, before starting executing code from such filesystems.

If administrator of the system wants this, he can just mount filesystem
without noexec, no new kernel code required. And it's more fine-grained
than this.

So, no, I don't think we should add knob like this. Unless I miss
something.


I don't believe this patch is necessary anymore (though, thank you
Robert for testing and re-sending!).

The primary offenders wrt to needing to mmap/mprotect a file in /dev/shm
was the older nvidia
driver (binary only iirc) and the Chrome Native Client code.

The reason why half-exec is an "ok" (half) mitigation is because it
blocks simple gadgets and other paths for using loadable libraries or
binaries (via glibc) as it disallows mmap(PROT_EXEC) even though it
allows mprotect(PROT_EXEC).  This stops ld in its tracks since it does
the obvious thing and uses mmap(PROT_EXEC).

I think time has marched on and this patch is now something I can toss
in the dustbin of history. Both Chrome's Native Client and an older
nvidia driver relied on creating-then-unlinking a file in tmpfs, but
there is now a better facility!


NAK.


Agreed - this is old and software that predicated it should be gone.. I
hope. :)


Splendid, patch dropped!
Thanks Will and Kirill!


Rob.





> It is motivated by a common /dev/shm, /tmp usecase. There are few
> facilities for creating a shared memory segment that can be remapped in
> the same process address space with different permissions.

What about using memfd_create(2) for such cases? You'll get a file
descriptor from in-kernel tmpfs (shm_mnt) which is not exposed to
userspace for remount as noexec.


This is a relatively old patch ( https://lwn.net/Articles/455256/
 ) which predated memfd_create().
 memfd_create() is the right solution to this problem!


Thanks again!
will

Re: [PATCH v1] mm, sysctl: Add sysctl for controlling VM_MAYEXEC taint

[Robert Foss]

On 2016-08-29 11:25 AM, Will Drewry wrote:



On Fri, Aug 26, 2016 at 4:32 PM, Kirill A. Shutemov
mailto:kir...@shutemov.name>> wrote:

On Fri, Aug 26, 2016 at 12:30:04PM -0400, robert.f...@collabora.com
 wrote:
> From: Will Drewry mailto:w...@chromium.org>>
>
> This patch proposes a sysctl knob that allows a privileged user to
> disable ~VM_MAYEXEC tainting when mapping in a vma from a MNT_NOEXEC
> mountpoint.  It does not alter the normal behavior resulting from
> attempting to directly mmap(PROT_EXEC) a vma (-EPERM) nor the behavior
> of any other subsystems checking MNT_NOEXEC.

Wouldn't it be equal to remounting all filesystems without noexec from
attacker POV? It's hardly a fence to make additional mprotect(PROT_EXEC)
call, before starting executing code from such filesystems.

If administrator of the system wants this, he can just mount filesystem
without noexec, no new kernel code required. And it's more fine-grained
than this.

So, no, I don't think we should add knob like this. Unless I miss
something.


I don't believe this patch is necessary anymore (though, thank you
Robert for testing and re-sending!).

The primary offenders wrt to needing to mmap/mprotect a file in /dev/shm
was the older nvidia
driver (binary only iirc) and the Chrome Native Client code.

The reason why half-exec is an "ok" (half) mitigation is because it
blocks simple gadgets and other paths for using loadable libraries or
binaries (via glibc) as it disallows mmap(PROT_EXEC) even though it
allows mprotect(PROT_EXEC).  This stops ld in its tracks since it does
the obvious thing and uses mmap(PROT_EXEC).

I think time has marched on and this patch is now something I can toss
in the dustbin of history. Both Chrome's Native Client and an older
nvidia driver relied on creating-then-unlinking a file in tmpfs, but
there is now a better facility!


NAK.


Agreed - this is old and software that predicated it should be gone.. I
hope. :)


Splendid, patch dropped!
Thanks Will and Kirill!


Rob.





> It is motivated by a common /dev/shm, /tmp usecase. There are few
> facilities for creating a shared memory segment that can be remapped in
> the same process address space with different permissions.

What about using memfd_create(2) for such cases? You'll get a file
descriptor from in-kernel tmpfs (shm_mnt) which is not exposed to
userspace for remount as noexec.


This is a relatively old patch ( https://lwn.net/Articles/455256/
 ) which predated memfd_create().
 memfd_create() is the right solution to this problem!


Thanks again!
will

Re: [PATCH v1] mm, sysctl: Add sysctl for controlling VM_MAYEXEC taint

[Kirill A. Shutemov]

On Fri, Aug 26, 2016 at 12:30:04PM -0400, robert.f...@collabora.com wrote:
> From: Will Drewry 
> 
> This patch proposes a sysctl knob that allows a privileged user to
> disable ~VM_MAYEXEC tainting when mapping in a vma from a MNT_NOEXEC
> mountpoint.  It does not alter the normal behavior resulting from
> attempting to directly mmap(PROT_EXEC) a vma (-EPERM) nor the behavior
> of any other subsystems checking MNT_NOEXEC.

Wouldn't it be equal to remounting all filesystems without noexec from
attacker POV? It's hardly a fence to make additional mprotect(PROT_EXEC)
call, before starting executing code from such filesystems.

If administrator of the system wants this, he can just mount filesystem
without noexec, no new kernel code required. And it's more fine-grained
than this.

So, no, I don't think we should add knob like this. Unless I miss
something.

NAK.

> It is motivated by a common /dev/shm, /tmp usecase. There are few
> facilities for creating a shared memory segment that can be remapped in
> the same process address space with different permissions.

What about using memfd_create(2) for such cases? You'll get a file
descriptor from in-kernel tmpfs (shm_mnt) which is not exposed to
userspace for remount as noexec.

-- 
 Kirill A. Shutemov

Re: [PATCH v1] mm, sysctl: Add sysctl for controlling VM_MAYEXEC taint

[Kirill A. Shutemov]

On Fri, Aug 26, 2016 at 12:30:04PM -0400, robert.f...@collabora.com wrote:
> From: Will Drewry 
> 
> This patch proposes a sysctl knob that allows a privileged user to
> disable ~VM_MAYEXEC tainting when mapping in a vma from a MNT_NOEXEC
> mountpoint.  It does not alter the normal behavior resulting from
> attempting to directly mmap(PROT_EXEC) a vma (-EPERM) nor the behavior
> of any other subsystems checking MNT_NOEXEC.

Wouldn't it be equal to remounting all filesystems without noexec from
attacker POV? It's hardly a fence to make additional mprotect(PROT_EXEC)
call, before starting executing code from such filesystems.

If administrator of the system wants this, he can just mount filesystem
without noexec, no new kernel code required. And it's more fine-grained
than this.

So, no, I don't think we should add knob like this. Unless I miss
something.

NAK.

> It is motivated by a common /dev/shm, /tmp usecase. There are few
> facilities for creating a shared memory segment that can be remapped in
> the same process address space with different permissions.

What about using memfd_create(2) for such cases? You'll get a file
descriptor from in-kernel tmpfs (shm_mnt) which is not exposed to
userspace for remount as noexec.

-- 
 Kirill A. Shutemov

Re: [PATCH v1] mm, sysctl: Add sysctl for controlling VM_MAYEXEC taint

[kbuild test robot]

Hi Will,

[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.8-rc3 next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for 
convenience) to record what (public, well-known) commit your patch series was 
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:
https://github.com/0day-ci/linux/commits/robert-foss-collabora-com/mm-sysctl-Add-sysctl-for-controlling-VM_MAYEXEC-taint/20160827-003416
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: sh-rsk7269_defconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 

All errors (new ones prefixed by >>):

>> mm/util.c:433:46: error: 'CONFIG_MMAP_NOEXEC_TAINT' undeclared here (not in 
>> a function)
int sysctl_mmap_noexec_taint __read_mostly = CONFIG_MMAP_NOEXEC_TAINT;
 ^

vim +/CONFIG_MMAP_NOEXEC_TAINT +433 mm/util.c

   427  EXPORT_SYMBOL_GPL(__page_mapcount);
   428  
   429  int sysctl_overcommit_memory __read_mostly = OVERCOMMIT_GUESS;
   430  int sysctl_overcommit_ratio __read_mostly = 50;
   431  unsigned long sysctl_overcommit_kbytes __read_mostly;
   432  int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
 > 433  int sysctl_mmap_noexec_taint __read_mostly = CONFIG_MMAP_NOEXEC_TAINT;
   434  unsigned long sysctl_user_reserve_kbytes __read_mostly = 1UL << 17; /* 
128MB */
   435  unsigned long sysctl_admin_reserve_kbytes __read_mostly = 1UL << 13; /* 
8MB */
   436  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH v1] mm, sysctl: Add sysctl for controlling VM_MAYEXEC taint

[kbuild test robot]

Hi Will,

[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.8-rc3 next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for 
convenience) to record what (public, well-known) commit your patch series was 
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:
https://github.com/0day-ci/linux/commits/robert-foss-collabora-com/mm-sysctl-Add-sysctl-for-controlling-VM_MAYEXEC-taint/20160827-003416
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: sh-rsk7269_defconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 

All errors (new ones prefixed by >>):

>> mm/util.c:433:46: error: 'CONFIG_MMAP_NOEXEC_TAINT' undeclared here (not in 
>> a function)
int sysctl_mmap_noexec_taint __read_mostly = CONFIG_MMAP_NOEXEC_TAINT;
 ^

vim +/CONFIG_MMAP_NOEXEC_TAINT +433 mm/util.c

   427  EXPORT_SYMBOL_GPL(__page_mapcount);
   428  
   429  int sysctl_overcommit_memory __read_mostly = OVERCOMMIT_GUESS;
   430  int sysctl_overcommit_ratio __read_mostly = 50;
   431  unsigned long sysctl_overcommit_kbytes __read_mostly;
   432  int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
 > 433  int sysctl_mmap_noexec_taint __read_mostly = CONFIG_MMAP_NOEXEC_TAINT;
   434  unsigned long sysctl_user_reserve_kbytes __read_mostly = 1UL << 17; /* 
128MB */
   435  unsigned long sysctl_admin_reserve_kbytes __read_mostly = 1UL << 13; /* 
8MB */
   436  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data