Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1382797 > unrolled thread

Re: [PATCH] Honor mmap_min_addr with the actual minimum

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] Honor mmap_min_addr with the actual minimum Hector Marco-Gisbert <hecmargi@upv.es> - 2016-04-19 21:00 +0200
    Re: [PATCH] Honor mmap_min_addr with the actual minimum Kees Cook <keescook@chromium.org> - 2016-04-21 00:20 +0200

#1382797 — 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@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
> 
>>
>> 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)

[toc] | [next] | [standalone]

#1383770

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web