2009-11-26 15:01:05

by Jan Engelhardt

[permalink] [raw]
Subject: Programs die when max_map_count is too large

Hi,


setting max_map_count to a value large enough results in programs dying
at first try.
This is on 2.6.31.6.

15:59 borg:/proc/sys/vm # echo $[1<<31-1] >max_map_count
15:59 borg:/proc/sys/vm # cat max_map_count
1073741824
15:59 borg:/proc/sys/vm # echo $[1<<31] >max_map_count
15:59 borg:/proc/sys/vm # cat max_map_count
Killed


thanks,
Jan


2009-11-26 16:15:39

by Cong Wang

[permalink] [raw]
Subject: Re: Programs die when max_map_count is too large

On Thu, Nov 26, 2009 at 04:01:08PM +0100, Jan Engelhardt wrote:
>Hi,
>
>
>setting max_map_count to a value large enough results in programs dying
>at first try.
>This is on 2.6.31.6.
>
>15:59 borg:/proc/sys/vm # echo $[1<<31-1] >max_map_count
>15:59 borg:/proc/sys/vm # cat max_map_count
>1073741824
>15:59 borg:/proc/sys/vm # echo $[1<<31] >max_map_count
>15:59 borg:/proc/sys/vm # cat max_map_count
>Killed
>

Hmm, a quick patch from me is below.

IMO, the problem is 'sysctl_max_map_count' is actually signed int,
while when writing to it, the value is treated as unsigned.

A better fix which I could imagine is to fix sysctl to understand
signed int's.

Just have a try, totally untested.


--------------->



Signed-off-by: WANG Cong <[email protected]>

---
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 84a524a..4161887 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -214,7 +214,7 @@ struct mm_struct {
pgd_t * pgd;
atomic_t mm_users; /* How many users with user space? */
atomic_t mm_count; /* How many references to "struct mm_struct" (users count as 1) */
- int map_count; /* number of VMAs */
+ unsigned int map_count; /* number of VMAs */
struct rw_semaphore mmap_sem;
spinlock_t page_table_lock; /* Protects page tables and some counters */

diff --git a/mm/mmap.c b/mm/mmap.c
index 73f5e4b..3adf2e7 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -85,7 +85,7 @@ EXPORT_SYMBOL(vm_get_page_prot);

int sysctl_overcommit_memory = OVERCOMMIT_GUESS; /* heuristic overcommit */
int sysctl_overcommit_ratio = 50; /* default is 50% */
-int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
+unsigned int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
struct percpu_counter vm_committed_as;

/*
diff --git a/mm/nommu.c b/mm/nommu.c
index 9876fa0..bf9ae62 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -65,7 +65,7 @@ unsigned long highest_memmap_pfn;
struct percpu_counter vm_committed_as;
int sysctl_overcommit_memory = OVERCOMMIT_GUESS; /* heuristic overcommit */
int sysctl_overcommit_ratio = 50; /* default is 50% */
-int sysctl_max_map_count = DEFAULT_MAX_MAP_COUNT;
+unsigned int sysctl_max_map_count = DEFAULT_MAX_MAP_COUNT;
int sysctl_nr_trim_pages = CONFIG_NOMMU_INITIAL_TRIM_EXCESS;
int heap_stack_gap = 0;

2009-11-26 16:48:41

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: Programs die when max_map_count is too large

> IMO, the problem is 'sysctl_max_map_count' is actually signed int,
> while when writing to it, the value is treated as unsigned.

The problem is the loop in simple_strtoul(), some people were so afraid
to touch so they even came up with strict_* variants.

> A better fix which I could imagine is to fix sysctl to understand
> signed int's.

- int map_count;
+ unsigned int map_count;

This is incomplete at least.

2009-11-27 06:30:21

by Cong Wang

[permalink] [raw]
Subject: Re: Programs die when max_map_count is too large

On Fri, Nov 27, 2009 at 12:48 AM, Alexey Dobriyan <[email protected]> wrote:
>> IMO, the problem is 'sysctl_max_map_count' is actually signed int,
>> while when writing to it, the value is treated as unsigned.
>
> The problem is the loop in simple_strtoul(), some people were so afraid
> to touch so they even came up with strict_* variants.
>
>> A better fix which I could imagine is to fix sysctl to understand
>> signed int's.
>
>        -       int map_count;
>        +       unsigned int map_count;
>
> This is incomplete at least.
>

Yeah, perhaps.

I got a better fix, but only for max_map_count. Please check it below.

(Sorr for attatching it, gmail's IMAP sucks.)

----------------->

Make negative values for 'max_map_count' invalid, they are meaningless.
And avoid overflow.

I already tested this.

Signed-off-by: WANG Cong <[email protected]>
-------


Attachments:
kernel-sysctl-max-map-count-overflow.diff (392.00 B)