2009-01-05 23:39:21

by Ying Han

[permalink] [raw]
Subject: [PATCH]Fix: 32bit binary has 64bit address of stack vma

From: Ying Han <[email protected]>

Fix 32bit binary get 64bit stack vma offset.

32bit binary running on 64bit system, the /proc/pid/maps shows for the
vma represents stack get a 64bit adress:
ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]

Signed-off-by: Ying Han <[email protected]>

fs/exec.c | 5 +-

diff --git a/fs/exec.c b/fs/exec.c
index 4e834f1..8c3eff4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -517,6 +517,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
unsigned long length = old_end - old_start;
unsigned long new_start = old_start - shift;
unsigned long new_end = old_end - shift;
+ unsigned long new_pgoff = new_start >> PAGE_SHIFT;
struct mmu_gather *tlb;

BUG_ON(new_start > new_end);
@@ -531,7 +532,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
/*
* cover the whole range: [new_start, old_end)
*/
- vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
+ vma_adjust(vma, new_start, old_end, new_pgoff, NULL);

/*
* move the page tables downwards, on failure we rely on
@@ -564,7 +565,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
/*
* shrink the vma to just the new range.
*/
- vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
+ vma_adjust(vma, new_start, new_end, new_pgoff, NULL);

return 0;
}


2009-01-09 02:40:32

by Ying Han

[permalink] [raw]
Subject: Re: [PATCH]Fix: 32bit binary has 64bit address of stack vma

On Mon, Jan 5, 2009 at 3:39 PM, Ying Han <[email protected]> wrote:
> From: Ying Han <[email protected]>
>
> Fix 32bit binary get 64bit stack vma offset.
>
> 32bit binary running on 64bit system, the /proc/pid/maps shows for the
> vma represents stack get a 64bit adress:
> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
>
> Signed-off-by: Ying Han <[email protected]>
>
> fs/exec.c | 5 +-
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 4e834f1..8c3eff4 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -517,6 +517,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
> unsigned long length = old_end - old_start;
> unsigned long new_start = old_start - shift;
> unsigned long new_end = old_end - shift;
> + unsigned long new_pgoff = new_start >> PAGE_SHIFT;
> struct mmu_gather *tlb;
>
> BUG_ON(new_start > new_end);
> @@ -531,7 +532,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
> /*
> * cover the whole range: [new_start, old_end)
> */
> - vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, old_end, new_pgoff, NULL);
>
> /*
> * move the page tables downwards, on failure we rely on
> @@ -564,7 +565,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
> /*
> * shrink the vma to just the new range.
> */
> - vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, new_end, new_pgoff, NULL);
>
> return 0;
> }
>

2009-01-10 00:27:48

by Ying Han

[permalink] [raw]
Subject: Re: [PATCH]Fix: 32bit binary has 64bit address of stack vma

friendly ping...

On Thu, Jan 8, 2009 at 6:40 PM, Ying Han <[email protected]> wrote:
> On Mon, Jan 5, 2009 at 3:39 PM, Ying Han <[email protected]> wrote:
>> From: Ying Han <[email protected]>
>>
>> Fix 32bit binary get 64bit stack vma offset.
>>
>> 32bit binary running on 64bit system, the /proc/pid/maps shows for the
>> vma represents stack get a 64bit adress:
>> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
>>
>> Signed-off-by: Ying Han <[email protected]>
>>
>> fs/exec.c | 5 +-
>>
>> diff --git a/fs/exec.c b/fs/exec.c
>> index 4e834f1..8c3eff4 100644
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -517,6 +517,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
>> unsigned long length = old_end - old_start;
>> unsigned long new_start = old_start - shift;
>> unsigned long new_end = old_end - shift;
>> + unsigned long new_pgoff = new_start >> PAGE_SHIFT;
>> struct mmu_gather *tlb;
>>
>> BUG_ON(new_start > new_end);
>> @@ -531,7 +532,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
>> /*
>> * cover the whole range: [new_start, old_end)
>> */
>> - vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
>> + vma_adjust(vma, new_start, old_end, new_pgoff, NULL);
>>
>> /*
>> * move the page tables downwards, on failure we rely on
>> @@ -564,7 +565,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
>> /*
>> * shrink the vma to just the new range.
>> */
>> - vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
>> + vma_adjust(vma, new_start, new_end, new_pgoff, NULL);
>>
>> return 0;
>> }
>>
>

2009-01-10 00:38:19

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH]Fix: 32bit binary has 64bit address of stack vma

On Fri, 9 Jan 2009 16:27:07 -0800
Ying Han <[email protected]> wrote:

> friendly ping...

We'll get there. We're in the merge window now, so I tend to defer
non-serious bugfixes until things are a bit quieter.

> On Thu, Jan 8, 2009 at 6:40 PM, Ying Han <[email protected]> wrote:
> > On Mon, Jan 5, 2009 at 3:39 PM, Ying Han <[email protected]> wrote:
> >> From: Ying Han <[email protected]>
> >>
> >> Fix 32bit binary get 64bit stack vma offset.
> >>
> >> 32bit binary running on 64bit system, the /proc/pid/maps shows for the
> >> vma represents stack get a 64bit adress:
> >> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]

That changelog hurts my brain.

> >> Signed-off-by: Ying Han <[email protected]>

2009-01-10 01:33:16

by Ying Han

[permalink] [raw]
Subject: Re: [PATCH]Fix: 32bit binary has 64bit address of stack vma

On Fri, Jan 9, 2009 at 4:37 PM, Andrew Morton <[email protected]> wrote:
> On Fri, 9 Jan 2009 16:27:07 -0800
> Ying Han <[email protected]> wrote:
>
>> friendly ping...
>
> We'll get there. We're in the merge window now, so I tend to defer
> non-serious bugfixes until things are a bit quieter.
Thank you Andrew .

>
>> On Thu, Jan 8, 2009 at 6:40 PM, Ying Han <[email protected]> wrote:
>> > On Mon, Jan 5, 2009 at 3:39 PM, Ying Han <[email protected]> wrote:
>> >> From: Ying Han <[email protected]>
>> >>
>> >> Fix 32bit binary get 64bit stack vma offset.
>> >>
>> >> 32bit binary running on 64bit system, the /proc/pid/maps shows for the
>> >> vma represents stack get a 64bit adress:
>> >> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
>
> That changelog hurts my brain.
hm, i will change it for better reading.
>
>> >> Signed-off-by: Ying Han <[email protected]>
>

2009-01-12 23:00:44

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH]Fix: 32bit binary has 64bit address of stack vma

On Mon, 5 Jan 2009 15:39:07 -0800
Ying Han <[email protected]> wrote:

> From: Ying Han <[email protected]>
>
> Fix 32bit binary get 64bit stack vma offset.
>
> 32bit binary running on 64bit system, the /proc/pid/maps shows for the
> vma represents stack get a 64bit adress:
> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
>
> Signed-off-by: Ying Han <[email protected]>
>
> fs/exec.c | 5 +-
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 4e834f1..8c3eff4 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -517,6 +517,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
> unsigned long length = old_end - old_start;
> unsigned long new_start = old_start - shift;
> unsigned long new_end = old_end - shift;
> + unsigned long new_pgoff = new_start >> PAGE_SHIFT;
> struct mmu_gather *tlb;
>
> BUG_ON(new_start > new_end);
> @@ -531,7 +532,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
> /*
> * cover the whole range: [new_start, old_end)
> */
> - vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, old_end, new_pgoff, NULL);
>
> /*
> * move the page tables downwards, on failure we rely on
> @@ -564,7 +565,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
> /*
> * shrink the vma to just the new range.
> */
> - vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, new_end, new_pgoff, NULL);
>
> return 0;

I rewrote the chagnelog as below. Please confirm that it makes sense?


Subject: fs/exec.c: fix value of vma->vm_pgoff for the stack VMA of 32-bit processes
From: Ying Han <[email protected]>

With a 32 bit binary running on a 64 bit system, the /proc/pid/maps for
the [stack] VMA displays a 64-bit address:

ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]

This is because vma->vm_pgoff for that VMA is incorrectly being stored in
units of offset-in-bytes. It should be stored in units of offset-in-pages.

Signed-off-by: Ying Han <[email protected]>
Cc: Mike Waychison <[email protected]>
Cc: Hugh Dickins <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

fs/exec.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff -puN fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes fs/exec.c
--- a/fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes
+++ a/fs/exec.c
@@ -509,6 +509,7 @@ static int shift_arg_pages(struct vm_are
unsigned long length = old_end - old_start;
unsigned long new_start = old_start - shift;
unsigned long new_end = old_end - shift;
+ unsigned long new_pgoff = new_start >> PAGE_SHIFT;
struct mmu_gather *tlb;

BUG_ON(new_start > new_end);
@@ -523,7 +524,7 @@ static int shift_arg_pages(struct vm_are
/*
* cover the whole range: [new_start, old_end)
*/
- vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
+ vma_adjust(vma, new_start, old_end, new_pgoff, NULL);

/*
* move the page tables downwards, on failure we rely on
@@ -556,7 +557,7 @@ static int shift_arg_pages(struct vm_are
/*
* shrink the vma to just the new range.
*/
- vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
+ vma_adjust(vma, new_start, new_end, new_pgoff, NULL);

return 0;
}
_

2009-01-12 23:11:20

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH]Fix: 32bit binary has 64bit address of stack vma

On Mon, 12 Jan 2009 15:05:51 -0800
Mike Waychison <[email protected]> wrote:

> > Subject: fs/exec.c: fix value of vma->vm_pgoff for the stack VMA of 32-bit processes
> > From: Ying Han <[email protected]>
> >
> > With a 32 bit binary running on a 64 bit system, the /proc/pid/maps for
> > the [stack] VMA displays a 64-bit address:
> >
> > ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
> >
> > This is because vma->vm_pgoff for that VMA is incorrectly being stored in
> > units of offset-in-bytes. It should be stored in units of offset-in-pages.
> >
>
> The problem is that the offset was stored without taking into account
> the shift.

Sigh. Is it efficient to have me sitting here reverse-engineering the
code, writing your changelog?



From: Ying Han <[email protected]>

With a 32 bit binary running on a 64 bit system, the /proc/pid/maps for
the [stack] VMA displays a 64-bit address:

ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]

This is because vma->vm_pgoff for that VMA is not being updated for the
shift of the stack VMA.

Signed-off-by: Ying Han <[email protected]>
Cc: Mike Waychison <[email protected]>
Cc: Hugh Dickins <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

fs/exec.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff -puN fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes fs/exec.c
--- a/fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes
+++ a/fs/exec.c
@@ -509,6 +509,7 @@ static int shift_arg_pages(struct vm_are
unsigned long length = old_end - old_start;
unsigned long new_start = old_start - shift;
unsigned long new_end = old_end - shift;
+ unsigned long new_pgoff = new_start >> PAGE_SHIFT;
struct mmu_gather *tlb;

BUG_ON(new_start > new_end);
@@ -523,7 +524,7 @@ static int shift_arg_pages(struct vm_are
/*
* cover the whole range: [new_start, old_end)
*/
- vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
+ vma_adjust(vma, new_start, old_end, new_pgoff, NULL);

/*
* move the page tables downwards, on failure we rely on
@@ -556,7 +557,7 @@ static int shift_arg_pages(struct vm_are
/*
* shrink the vma to just the new range.
*/
- vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
+ vma_adjust(vma, new_start, new_end, new_pgoff, NULL);

return 0;
}
_

2009-01-12 23:20:33

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH]Fix: 32bit binary has 64bit address of stack vma

Andrew Morton wrote:
> On Mon, 5 Jan 2009 15:39:07 -0800
> Ying Han <[email protected]> wrote:
>
>> From: Ying Han <[email protected]>
>>
>> Fix 32bit binary get 64bit stack vma offset.
>>
>> 32bit binary running on 64bit system, the /proc/pid/maps shows for the
>> vma represents stack get a 64bit adress:
>> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
>>
>> Signed-off-by: Ying Han <[email protected]>
>>
>> fs/exec.c | 5 +-
>>
>> diff --git a/fs/exec.c b/fs/exec.c
>> index 4e834f1..8c3eff4 100644
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -517,6 +517,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
>> unsigned long length = old_end - old_start;
>> unsigned long new_start = old_start - shift;
>> unsigned long new_end = old_end - shift;
>> + unsigned long new_pgoff = new_start >> PAGE_SHIFT;
>> struct mmu_gather *tlb;
>>
>> BUG_ON(new_start > new_end);
>> @@ -531,7 +532,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
>> /*
>> * cover the whole range: [new_start, old_end)
>> */
>> - vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
>> + vma_adjust(vma, new_start, old_end, new_pgoff, NULL);
>>
>> /*
>> * move the page tables downwards, on failure we rely on
>> @@ -564,7 +565,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
>> /*
>> * shrink the vma to just the new range.
>> */
>> - vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
>> + vma_adjust(vma, new_start, new_end, new_pgoff, NULL);
>>
>> return 0;
>
> I rewrote the chagnelog as below. Please confirm that it makes sense?
>
>
> Subject: fs/exec.c: fix value of vma->vm_pgoff for the stack VMA of 32-bit processes
> From: Ying Han <[email protected]>
>
> With a 32 bit binary running on a 64 bit system, the /proc/pid/maps for
> the [stack] VMA displays a 64-bit address:
>
> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
>
> This is because vma->vm_pgoff for that VMA is incorrectly being stored in
> units of offset-in-bytes. It should be stored in units of offset-in-pages.
>

The problem is that the offset was stored without taking into account
the shift.

> Signed-off-by: Ying Han <[email protected]>
> Cc: Mike Waychison <[email protected]>
> Cc: Hugh Dickins <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> fs/exec.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff -puN fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes fs/exec.c
> --- a/fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes
> +++ a/fs/exec.c
> @@ -509,6 +509,7 @@ static int shift_arg_pages(struct vm_are
> unsigned long length = old_end - old_start;
> unsigned long new_start = old_start - shift;
> unsigned long new_end = old_end - shift;
> + unsigned long new_pgoff = new_start >> PAGE_SHIFT;
> struct mmu_gather *tlb;
>
> BUG_ON(new_start > new_end);
> @@ -523,7 +524,7 @@ static int shift_arg_pages(struct vm_are
> /*
> * cover the whole range: [new_start, old_end)
> */
> - vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, old_end, new_pgoff, NULL);
>
> /*
> * move the page tables downwards, on failure we rely on
> @@ -556,7 +557,7 @@ static int shift_arg_pages(struct vm_are
> /*
> * shrink the vma to just the new range.
> */
> - vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, new_end, new_pgoff, NULL);
>
> return 0;
> }
> _
>

2009-01-12 23:23:42

by Ying Han

[permalink] [raw]
Subject: Re: [PATCH]Fix: 32bit binary has 64bit address of stack vma

Thanks Andrew.

On Mon, Jan 12, 2009 at 2:59 PM, Andrew Morton
<[email protected]> wrote:
> On Mon, 5 Jan 2009 15:39:07 -0800
> Ying Han <[email protected]> wrote:
>
>> From: Ying Han <[email protected]>
>>
>> Fix 32bit binary get 64bit stack vma offset.
>>
>> 32bit binary running on 64bit system, the /proc/pid/maps shows for the
>> vma represents stack get a 64bit adress:
>> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
>>
>> Signed-off-by: Ying Han <[email protected]>
>>
>> fs/exec.c | 5 +-
>>
>> diff --git a/fs/exec.c b/fs/exec.c
>> index 4e834f1..8c3eff4 100644
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -517,6 +517,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
>> unsigned long length = old_end - old_start;
>> unsigned long new_start = old_start - shift;
>> unsigned long new_end = old_end - shift;
>> + unsigned long new_pgoff = new_start >> PAGE_SHIFT;
>> struct mmu_gather *tlb;
>>
>> BUG_ON(new_start > new_end);
>> @@ -531,7 +532,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
>> /*
>> * cover the whole range: [new_start, old_end)
>> */
>> - vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
>> + vma_adjust(vma, new_start, old_end, new_pgoff, NULL);
>>
>> /*
>> * move the page tables downwards, on failure we rely on
>> @@ -564,7 +565,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, uns
>> /*
>> * shrink the vma to just the new range.
>> */
>> - vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
>> + vma_adjust(vma, new_start, new_end, new_pgoff, NULL);
>>
>> return 0;
>
> I rewrote the chagnelog as below. Please confirm that it makes sense?
>
>
> Subject: fs/exec.c: fix value of vma->vm_pgoff for the stack VMA of 32-bit processes
> From: Ying Han <[email protected]>
>
> With a 32 bit binary running on a 64 bit system, the /proc/pid/maps for
> the [stack] VMA displays a 64-bit address:
>
> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
looks good.
>
> This is because vma->vm_pgoff for that VMA is incorrectly being stored in
> units of offset-in-bytes. It should be stored in units of offset-in-pages.
that is not the problem. the real problem here is that the
vma->vm_pgoff is initialized as 64bit address. When
it is doing the shift_arg_pages, it supposed to be readjust to 32bit.
It did for newstart and newend, but still
used the old vma->start(64bit) for vma->pgoff. Here i make the change
to get the newpgoff based on the newstart.
> Signed-off-by: Ying Han <[email protected]>
> Cc: Mike Waychison <[email protected]>
> Cc: Hugh Dickins <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> fs/exec.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff -puN fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes fs/exec.c
> --- a/fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes
> +++ a/fs/exec.c
> @@ -509,6 +509,7 @@ static int shift_arg_pages(struct vm_are
> unsigned long length = old_end - old_start;
> unsigned long new_start = old_start - shift;
> unsigned long new_end = old_end - shift;
> + unsigned long new_pgoff = new_start >> PAGE_SHIFT;
> struct mmu_gather *tlb;
>
> BUG_ON(new_start > new_end);
> @@ -523,7 +524,7 @@ static int shift_arg_pages(struct vm_are
> /*
> * cover the whole range: [new_start, old_end)
> */
> - vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, old_end, new_pgoff, NULL);
>
> /*
> * move the page tables downwards, on failure we rely on
> @@ -556,7 +557,7 @@ static int shift_arg_pages(struct vm_are
> /*
> * shrink the vma to just the new range.
> */
> - vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, new_end, new_pgoff, NULL);
>
> return 0;
> }
> _
>
>

2009-01-13 18:00:24

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH]Fix: 32bit binary has 64bit address of stack vma

On Mon, 12 Jan 2009, Andrew Morton wrote:
> On Mon, 12 Jan 2009 15:05:51 -0800
> Mike Waychison <[email protected]> wrote:
>
> > > Subject: fs/exec.c: fix value of vma->vm_pgoff for the stack VMA of 32-bit processes
> > > From: Ying Han <[email protected]>
> > >
> > > With a 32 bit binary running on a 64 bit system, the /proc/pid/maps for
> > > the [stack] VMA displays a 64-bit address:
> > >
> > > ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
> > >
> > > This is because vma->vm_pgoff for that VMA is incorrectly being stored in
> > > units of offset-in-bytes. It should be stored in units of offset-in-pages.
> > >
> >
> > The problem is that the offset was stored without taking into account
> > the shift.
>
> Sigh. Is it efficient to have me sitting here reverse-engineering the
> code, writing your changelog?
>
>
>
> From: Ying Han <[email protected]>
>
> With a 32 bit binary running on a 64 bit system, the /proc/pid/maps for
> the [stack] VMA displays a 64-bit address:
>
> ff96c000-ff981000 rwxp 7ffffffea000 00:00 0 [stack]
>
> This is because vma->vm_pgoff for that VMA is not being updated for the
> shift of the stack VMA.
>
> Signed-off-by: Ying Han <[email protected]>
> Cc: Mike Waychison <[email protected]>
> Cc: Hugh Dickins <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---

Sorry for all the waste of your time, but
I'm afraid this is entirely bogus, as Kamezawa-san has kindly
discovered for us (in: Re: mmotm 2009-01-12-16-53 uploaded thread).

The code was correct before, there was no shifting issue, and you
cannot muck around with the vm_pgoff of a vma like this patch does.
Thank Nick for the CONFIG_DEBUG_VM BUG_ON which warns that this
patch has inadvertently made these stack pages unreclaimable
(because their indexes no longer match up with vm_pgoff).

All there is here is a cosmetic issue: we're used to seeing 8-hex-
digit offsets in the /proc/pid/maps of a 32-bit process. Though it
could perfectly well mmap() a large file from a very high offset and
show more hex digits there, couldn't it?

Because this 32-bit process has been started by a 64-bit kernel, it
started off working with the 64-bit stack position before moving into
place, and that happens to get reflected in the final vm_pgoff.

You prefer only to see 8-hex-digit offsets for anonymous vmas of 32-bit
processes? If so, then perhaps Ying Han could supply a cosmetic patch
for /proc/pid/maps to mask off the upper bits in that case.

Hugh

>
> fs/exec.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff -puN fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes fs/exec.c
> --- a/fs/exec.c~fs-execc-fix-value-of-vma-vm_pgoff-for-the-stack-vma-of-32-bit-processes
> +++ a/fs/exec.c
> @@ -509,6 +509,7 @@ static int shift_arg_pages(struct vm_are
> unsigned long length = old_end - old_start;
> unsigned long new_start = old_start - shift;
> unsigned long new_end = old_end - shift;
> + unsigned long new_pgoff = new_start >> PAGE_SHIFT;
> struct mmu_gather *tlb;
>
> BUG_ON(new_start > new_end);
> @@ -523,7 +524,7 @@ static int shift_arg_pages(struct vm_are
> /*
> * cover the whole range: [new_start, old_end)
> */
> - vma_adjust(vma, new_start, old_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, old_end, new_pgoff, NULL);
>
> /*
> * move the page tables downwards, on failure we rely on
> @@ -556,7 +557,7 @@ static int shift_arg_pages(struct vm_are
> /*
> * shrink the vma to just the new range.
> */
> - vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);
> + vma_adjust(vma, new_start, new_end, new_pgoff, NULL);
>
> return 0;
> }
> _