Messages in this thread | | | Date | Wed, 20 Apr 2016 15:12:41 -0700 | Subject | Re: [PATCH] Honor mmap_min_addr with the actual minimum | From | Kees Cook <> |
| |
On Tue, Apr 19, 2016 at 11:55 AM, Hector Marco-Gisbert <hecmargi@upv.es> wrote: >> On Wed, Apr 6, 2016 at 12:07 PM, Hector Marco-Gisbert <hecmargi@upv.es> wrote: >>> The minimum address that a process is allowed to mmap when LSM is >>> enabled is 0x10000 (65536). This value is tunable and exported via >>> /proc/sys/vm/mmap_min_addr but it is not honored with the actual >>> minimum value. >> >> I think this is working as intended already, based on the commit log >> for 788084aba2ab7348257597496befcbccabdc98a3 >> >> See cap_mmap_addr (which uses dac_mmap_min_addr) vs SELinux's hook >> (which uses CONFIG_LSM_MMAP_MIN_ADDR), and everything else (that uses >> mmap_min_addr). >> >> Without CONFIG_LSM_MMAP_MIN_ADDR, dac_mmap_min_addr always == mmap_min_addr. >> >> With CONFIG_LSM_MMAP_MIN_ADDR, dac_mmap_min_addr can be less than >> mmap_min_addr, but mmap_min_addr will always be at least >> CONFIG_LSM_MMAP_MIN_ADDR. >> >> Eric may be able to shed more light on this... >> >> -Kees > > Ok, I see your point, but it seems that minimum address that a process is > allowed to map is mmap_min_addr and not dac_mmap_min_addr. > This is because mmap_min_addr can be seen as the max(dac_mmap_min_addr, > CONFIG_LSM_MMAP_MIN_ADDR) which is correct (the minimum allowed address) but > /proc/sys/vm/mmap_min_addr contains dac_mmap_min_addr which is not the minimum. > > For example, if we set the CONFIG_LSM_MMAP_MIN_ADDR to 65536 and > /proc/sys/vm/mmap_min_addr to 4096, then assuming that selinux_mmap_addr() has > no permissions (it returns !=0), the minimum allowed address is 65536 not 4096. > The mmap check is done in the security_mmap_addr(addr) function in mm/mmap.c > file. It seems that we are exporting the dac_mmap_min_addr instead of the actual > minimum. > > Is this behavior intended ? I'm missing something here ?
I think it is -- the minimum is correct, it's just that the sysctl may be reporting the dac value. Eric, are you able to chime in on this?
-Kees
> > Thanks, > Hector. > >> >>> >>> It can be easily checked in a system typing: >>> >>> $ cat /proc/sys/vm/mmap_min_addr >>> 4096 # <= Incorrect, it should be 65536 >>> >>> $ echo 1024 > /proc/sys/vm/mmap_min_addr >>> $ cat /proc/sys/vm/mmap_min_addr >>> 1024 # <= Incorrect, it should be 65536 >>> >>> After applying the patch: >>> >>> $ cat /proc/sys/vm/mmap_min_addr >>> 65536 # <= It is correct >>> >>> $ echo 1024 > /proc/sys/vm/mmap_min_addr >>> $ cat /proc/sys/vm/mmap_min_addr >>> 65536 # <= It is correct >>> >>> >>> >>> Signed-off-by: Hector Marco-Gisbert <hecmargi@upv.es> >>> Acked-by: Ismael Ripoll Ripoll <iripoll@upv.es> >>> --- >>> security/min_addr.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/security/min_addr.c b/security/min_addr.c >>> index f728728..96d1811 100644 >>> --- a/security/min_addr.c >>> +++ b/security/min_addr.c >>> @@ -15,10 +15,12 @@ unsigned long dac_mmap_min_addr = CONFIG_DEFAULT_MMAP_MIN_ADDR; >>> static void update_mmap_min_addr(void) >>> { >>> #ifdef CONFIG_LSM_MMAP_MIN_ADDR >>> - if (dac_mmap_min_addr > CONFIG_LSM_MMAP_MIN_ADDR) >>> + if (dac_mmap_min_addr > CONFIG_LSM_MMAP_MIN_ADDR) { >>> mmap_min_addr = dac_mmap_min_addr; >>> - else >>> + } else { >>> mmap_min_addr = CONFIG_LSM_MMAP_MIN_ADDR; >>> + dac_mmap_min_addr = CONFIG_LSM_MMAP_MIN_ADDR; >>> + } >>> #else >>> mmap_min_addr = dac_mmap_min_addr; >>> #endif >>> -- >>> 1.9.1 >>> >> >> >> > > -- > Dr. Hector Marco-Gisbert @ http://hmarco.org/ > Cyber Security Researcher @ http://cybersecurity.upv.es > Universitat Politècnica de València (Spain)
-- Kees Cook Chrome OS & Brillo Security
| |