2007-10-25 13:41:36

by Sami Farin

[permalink] [raw]
Subject: x86: randomize brk() and RLIMIT_DATA

Would be neat if randomized brk and setrlimit(RLIMIT_DATA, ...)
worked in a predictable way:

$ gcc brk.c -fPIC -pie -m64;./a.out;./a.out;./a.out
sbrk=0x7f721b815000 main=0x7f721af04860
sbrk succeeded (brk=0x7f721b909240)
sbrk=0x7fc3d77e2000 main=0x7fc3d66fa860
sbrk failed: Cannot allocate memory (brk=0x7fc3d77e2000)
sbrk=0x7f05b0615000 main=0x7f05af76b860
sbrk failed: Cannot allocate memory (brk=0x7f05b0615000)

--
Do what you love because life is too short for anything else.


Attachments:
(No filename) (484.00 B)
brk.c (692.00 B)
Download all attachments

2007-10-25 14:17:09

by Arjan van de Ven

[permalink] [raw]
Subject: Re: x86: randomize brk() and RLIMIT_DATA

On Thu, 25 Oct 2007 16:41:24 +0300
Sami Farin <[email protected]> wrote:

> Would be neat if randomized brk and setrlimit(RLIMIT_DATA, ...)
> worked in a predictable way:

this isn't a valid case afaics; even on "traditional x86" (before we
changed the address space layout, or even today if you have an
unlimited stack rlimit) this isn't going to work. applications really
shouldn't use (s)brk() but malloc(); you have to be able to fall back
to mmap regardless of what you do.

>
> $ gcc brk.c -fPIC -pie -m64;./a.out;./a.out;./a.out
> sbrk=0x7f721b815000 main=0x7f721af04860
> sbrk succeeded (brk=0x7f721b909240)
> sbrk=0x7fc3d77e2000 main=0x7fc3d66fa860
> sbrk failed: Cannot allocate memory (brk=0x7fc3d77e2000)
> sbrk=0x7f05b0615000 main=0x7f05af76b860
> sbrk failed: Cannot allocate memory (brk=0x7f05b0615000)
>


--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2007-10-25 14:46:44

by Jiri Kosina

[permalink] [raw]
Subject: Re: x86: randomize brk() and RLIMIT_DATA

On Thu, 25 Oct 2007, Arjan van de Ven wrote:

> > Would be neat if randomized brk and setrlimit(RLIMIT_DATA, ...)
> > worked in a predictable way:
> this isn't a valid case afaics; even on "traditional x86" (before we
> changed the address space layout, or even today if you have an unlimited
> stack rlimit) this isn't going to work. applications really shouldn't
> use (s)brk() but malloc(); you have to be able to fall back to mmap
> regardless of what you do.

I tend to agree here with Arjan. However it probably would make no harm to
make at least a little bit consisten behavior of setrlimit(), though it
has a little use in such cases.

Sami, does the patch below work for you?

diff --git a/mm/mmap.c b/mm/mmap.c
index facc1a7..c7ade18 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -251,7 +251,8 @@ asmlinkage unsigned long sys_brk(unsigned long brk)
* not page aligned -Ram Gupta
*/
rlim = current->signal->rlim[RLIMIT_DATA].rlim_cur;
- if (rlim < RLIM_INFINITY && brk - mm->start_data > rlim)
+ if (rlim < RLIM_INFINITY && brk - mm->start_data -
+ (mm->start_brk - mm->end_data) > rlim)
goto out;

newbrk = PAGE_ALIGN(brk);

2007-10-25 17:19:55

by Sami Farin

[permalink] [raw]
Subject: Re: x86: randomize brk() and RLIMIT_DATA

On Thu, Oct 25, 2007 at 16:46:26 +0200, Jiri Kosina wrote:
> On Thu, 25 Oct 2007, Arjan van de Ven wrote:
>
> > > Would be neat if randomized brk and setrlimit(RLIMIT_DATA, ...)
> > > worked in a predictable way:
> > this isn't a valid case afaics; even on "traditional x86" (before we
> > changed the address space layout, or even today if you have an unlimited
> > stack rlimit) this isn't going to work. applications really shouldn't
> > use (s)brk() but malloc(); you have to be able to fall back to mmap
> > regardless of what you do.
>
> I tend to agree here with Arjan. However it probably would make no harm to
> make at least a little bit consisten behavior of setrlimit(), though it
> has a little use in such cases.
>
> Sami, does the patch below work for you?

Thanks, Jiri, now RLIMIT_DATA works as expected.

Using only RLIMIT_AS to limit processes' memory usage is not
very easy. It includes also libraries mapped read-only,
I have to check/modify the limits when I update/add
libraries,...

Amazingly, I found a patch which seems to be just what I need:
http://marc.info/?l=linux-mm&m=118402827803338&w=4
Seems like that is not going to -mm or mainstream...

--
Do what you love because life is too short for anything else.

2007-10-26 10:44:59

by Jiri Kosina

[permalink] [raw]
Subject: [PATCH] [RFC] brk randomization: compute RLIMIT_DATA properly (was Re: x86: randomize brk() and RLIMIT_DATA)

On Thu, 25 Oct 2007, Sami Farin wrote:

> > > > Would be neat if randomized brk and setrlimit(RLIMIT_DATA, ...)
> > > > worked in a predictable way:
> > > this isn't a valid case afaics; even on "traditional x86" (before we
> > > changed the address space layout, or even today if you have an
> > > unlimited stack rlimit) this isn't going to work. applications
> > > really shouldn't use (s)brk() but malloc(); you have to be able to
> > > fall back to mmap regardless of what you do.
> > I tend to agree here with Arjan. However it probably would make no
> > harm to make at least a little bit consisten behavior of setrlimit(),
> > though it has a little use in such cases.
> > Sami, does the patch below work for you?
> Thanks, Jiri, now RLIMIT_DATA works as expected. Using only RLIMIT_AS to
> limit processes' memory usage is not very easy. It includes also
> libraries mapped read-only, I have to check/modify the limits when I
> update/add libraries,...

This patch is [RFC] as I am not sure whether it is worth it (see Arjan's
comment above).


From: Jiri Kosina <[email protected]>

brk randomization: compute RLIMIT_DATA properly

In cases of heap area placed at randomly-generated offset from
mm->end_data (arch_randomize_brk()), we need to subtract the value of the
offset for setrlimit(RLIMIT_DATA) to work properly -- otherwise we count
the unoccupied memory between mm->end_data and mm->start_brk as occupied.

Signed-off-by: Jiri Kosina <[email protected]>

diff --git a/mm/mmap.c b/mm/mmap.c
index facc1a7..c7ade18 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -251,7 +251,8 @@ asmlinkage unsigned long sys_brk(unsigned long brk)
* not page aligned -Ram Gupta
*/
rlim = current->signal->rlim[RLIMIT_DATA].rlim_cur;
- if (rlim < RLIM_INFINITY && brk - mm->start_data > rlim)
+ if (rlim < RLIM_INFINITY && brk - mm->start_data -
+ (mm->start_brk - mm->end_data) > rlim)
goto out;

newbrk = PAGE_ALIGN(brk);

2007-10-26 12:04:14

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH] [RFC] brk randomization: compute RLIMIT_DATA properly (was Re: x86: randomize brk() and RLIMIT_DATA)

On Fri, 26 Oct 2007, Jiri Kosina wrote:
> On Thu, 25 Oct 2007, Sami Farin wrote:
> > > > > Would be neat if randomized brk and setrlimit(RLIMIT_DATA, ...)
> > > > > worked in a predictable way:
> > > > this isn't a valid case afaics; even on "traditional x86" (before we
> > > > changed the address space layout, or even today if you have an
> > > > unlimited stack rlimit) this isn't going to work. applications
> > > > really shouldn't use (s)brk() but malloc(); you have to be able to
> > > > fall back to mmap regardless of what you do.
> > > I tend to agree here with Arjan. However it probably would make no
> > > harm to make at least a little bit consisten behavior of setrlimit(),
> > > though it has a little use in such cases.
> > > Sami, does the patch below work for you?
> > Thanks, Jiri, now RLIMIT_DATA works as expected. Using only RLIMIT_AS to
> > limit processes' memory usage is not very easy. It includes also
> > libraries mapped read-only, I have to check/modify the limits when I
> > update/add libraries,...
>
> This patch is [RFC] as I am not sure whether it is worth it (see Arjan's
> comment above).

It is worth it.

Arjan's right that RLIMIT_DATA has difficulty making sense, but
you shouldn't be randomizing its breakage further with your patch.

>
>
> From: Jiri Kosina <[email protected]>
>
> brk randomization: compute RLIMIT_DATA properly
>
> In cases of heap area placed at randomly-generated offset from
> mm->end_data (arch_randomize_brk()), we need to subtract the value of the
> offset for setrlimit(RLIMIT_DATA) to work properly -- otherwise we count
> the unoccupied memory between mm->end_data and mm->start_brk as occupied.
>
> Signed-off-by: Jiri Kosina <[email protected]>
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index facc1a7..c7ade18 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -251,7 +251,8 @@ asmlinkage unsigned long sys_brk(unsigned long brk)
> * not page aligned -Ram Gupta
> */
> rlim = current->signal->rlim[RLIMIT_DATA].rlim_cur;
> - if (rlim < RLIM_INFINITY && brk - mm->start_data > rlim)
> + if (rlim < RLIM_INFINITY && brk - mm->start_data -
> + (mm->start_brk - mm->end_data) > rlim)
> goto out;
>
> newbrk = PAGE_ALIGN(brk);

I find the order in that test mysterious. Others might ask you to use
an intermediate variable, I don't care about that, but I would find

if (rlim < RLIM_INFINITY &&
(brk - mm->start_brk) + (mm->end_data - mm->start_data) > rlim)

much easier to understand.

Hugh

2007-10-26 12:16:55

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH] [RFC] brk randomization: compute RLIMIT_DATA properly (was Re: x86: randomize brk() and RLIMIT_DATA)

On Fri, 26 Oct 2007, Hugh Dickins wrote:

> > - if (rlim < RLIM_INFINITY && brk - mm->start_data > rlim)
> > + if (rlim < RLIM_INFINITY && brk - mm->start_data -
> > + (mm->start_brk - mm->end_data) > rlim)
> I find the order in that test mysterious.

I agree that the order you proposed is easier to understand on the first
sight.


From: Jiri Kosina <[email protected]>

brk randomization: compute RLIMIT_DATA properly

In cases of heap area placed at randomly-generated offset from
mm->end_data (arch_randomize_brk()), we need to subtract the value of the
offset for setrlimit(RLIMIT_DATA) to work properly -- otherwise we count
the unoccupied memory between mm->end_data and mm->start_brk as occupied.

Signed-off-by: Jiri Kosina <[email protected]>

diff --git a/mm/mmap.c b/mm/mmap.c
index facc1a7..673ac7d 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -251,7 +251,8 @@ asmlinkage unsigned long sys_brk(unsigned long brk)
* not page aligned -Ram Gupta
*/
rlim = current->signal->rlim[RLIMIT_DATA].rlim_cur;
- if (rlim < RLIM_INFINITY && brk - mm->start_data > rlim)
+ if (rlim < RLIM_INFINITY && (brk - mm->start_brk) +
+ (mm->end_data - mm->start_data) > rlim)
goto out;

newbrk = PAGE_ALIGN(brk);

2007-10-26 12:32:24

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH] [RFC] brk randomization: compute RLIMIT_DATA properly (was Re: x86: randomize brk() and RLIMIT_DATA)

On Fri, 26 Oct 2007, Jiri Kosina wrote:
> On Fri, 26 Oct 2007, Hugh Dickins wrote:
>
> > > - if (rlim < RLIM_INFINITY && brk - mm->start_data > rlim)
> > > + if (rlim < RLIM_INFINITY && brk - mm->start_data -
> > > + (mm->start_brk - mm->end_data) > rlim)
> > I find the order in that test mysterious.
>
> I agree that the order you proposed is easier to understand on the first
> sight.
>
>
> From: Jiri Kosina <[email protected]>
>
> brk randomization: compute RLIMIT_DATA properly
>
> In cases of heap area placed at randomly-generated offset from
> mm->end_data (arch_randomize_brk()), we need to subtract the value of the
> offset for setrlimit(RLIMIT_DATA) to work properly -- otherwise we count
> the unoccupied memory between mm->end_data and mm->start_brk as occupied.
>
> Signed-off-by: Jiri Kosina <[email protected]>

Acked-by: Hugh Dickins <[email protected]>

>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index facc1a7..673ac7d 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -251,7 +251,8 @@ asmlinkage unsigned long sys_brk(unsigned long brk)
> * not page aligned -Ram Gupta
> */
> rlim = current->signal->rlim[RLIMIT_DATA].rlim_cur;
> - if (rlim < RLIM_INFINITY && brk - mm->start_data > rlim)
> + if (rlim < RLIM_INFINITY && (brk - mm->start_brk) +
> + (mm->end_data - mm->start_data) > rlim)
> goto out;
>
> newbrk = PAGE_ALIGN(brk);

2007-10-26 12:33:23

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] [RFC] brk randomization: compute RLIMIT_DATA properly (was Re: x86: randomize brk() and RLIMIT_DATA)


On Oct 26 2007 13:30, Hugh Dickins wrote:
>> @@ -251,7 +251,8 @@ asmlinkage unsigned long sys_brk(unsigned long brk)
>> * not page aligned -Ram Gupta
>> */
>> rlim = current->signal->rlim[RLIMIT_DATA].rlim_cur;
>> - if (rlim < RLIM_INFINITY && brk - mm->start_data > rlim)
>> + if (rlim < RLIM_INFINITY && (brk - mm->start_brk) +
>> + (mm->end_data - mm->start_data) > rlim)
>> goto out;
>>
>> newbrk = PAGE_ALIGN(brk);

Parentheses around (brk - mm->start_brk) + (mm->end_data - mm->start_data)
not strictly necessary.

2007-10-26 13:25:48

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH] [RFC] brk randomization: compute RLIMIT_DATA properly (was Re: x86: randomize brk() and RLIMIT_DATA)

On Fri, 26 Oct 2007, Jan Engelhardt wrote:
> On Oct 26 2007 13:30, Hugh Dickins wrote:
> >> @@ -251,7 +251,8 @@ asmlinkage unsigned long sys_brk(unsigned long brk)
> >> * not page aligned -Ram Gupta
> >> */
> >> rlim = current->signal->rlim[RLIMIT_DATA].rlim_cur;
> >> - if (rlim < RLIM_INFINITY && brk - mm->start_data > rlim)
> >> + if (rlim < RLIM_INFINITY && (brk - mm->start_brk) +
> >> + (mm->end_data - mm->start_data) > rlim)
> >> goto out;
> >>
> >> newbrk = PAGE_ALIGN(brk);
>
> Parentheses around (brk - mm->start_brk) + (mm->end_data - mm->start_data)
> not strictly necessary.

Yes, but don't you find they make it easier to understand the expression?

Hugh