[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <57167F16.6090507@upv.es>
Date: Tue, 19 Apr 2016 20:55:18 +0200
From: Hector Marco-Gisbert <hecmargi@....es>
To: Kees Cook <keescook@...omium.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
James Morris <james.l.morris@...cle.com>,
"Serge E. Hallyn" <serge@...lyn.com>,
linux-security-module <linux-security-module@...r.kernel.org>,
Ismael Ripoll <iripoll@....es>, Eric Paris <eparis@...hat.com>
Subject: Re: [PATCH] Honor mmap_min_addr with the actual minimum
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 ?
Thanks,
Hector.
> On Wed, Apr 6, 2016 at 12:07 PM, Hector Marco-Gisbert <hecmargi@....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
>
>>
>> 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@....es>
>> Acked-by: Ismael Ripoll Ripoll <iripoll@....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)
Powered by blists - more mailing lists