2010-06-01 19:00:06

by David Howells

[permalink] [raw]
Subject: [PATCH] NOMMU: Add '[stack]' label to /proc/pid/maps output

From: Mike Frysinger <[email protected]>

Add support to the NOMMU /proc/pid/maps file to show which mapping is the stack
of the original thread after execve. This is largely based on the MMU code.
Subsidiary thread stacks are not indicated.

For FDPIC, we now get:

root:/> cat /proc/self/maps
02064000-02067ccc rw-p 0004d000 00:01 22 /bin/busybox
0206e000-0206f35c rw-p 00006000 00:01 295 /lib/ld-uClibc.so.0
025f0000-025f6f0c r-xs 00000000 00:01 295 /lib/ld-uClibc.so.0
02680000-026ba6b0 r-xs 00000000 00:01 297 /lib/libc.so.0
02700000-0274d384 r-xs 00000000 00:01 22 /bin/busybox
02816000-02817000 rw-p 00000000 00:00 0
02848000-0284c0d8 rw-p 00000000 00:00 0
02860000-02880000 rw-p 00000000 00:00 0 [stack]

The semi-downside here is that for FLAT, we get:

root:/> cat /proc/155/maps
029f0000-029f9000 rwxp 00000000 00:00 0 [stack]

The reason being that FLAT combines a whole lot of stuff into one map
(including the stack). But this isn't any worse than the current output (which
is nothing), so screw it.

Signed-off-by: Mike Frysinger <[email protected]>
Signed-off-by: David Howells <[email protected]>
---

fs/proc/task_nommu.c | 20 ++++++++++++++++----
1 files changed, 16 insertions(+), 4 deletions(-)


diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index 46d4b5d..cb6306e 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -122,11 +122,20 @@ int task_statm(struct mm_struct *mm, int *shared, int *text,
return size;
}

+static void pad_len_spaces(struct seq_file *m, int len)
+{
+ len = 25 + sizeof(void*) * 6 - len;
+ if (len < 1)
+ len = 1;
+ seq_printf(m, "%*c", len, ' ');
+}
+
/*
* display a single VMA to a sequenced file
*/
static int nommu_vma_show(struct seq_file *m, struct vm_area_struct *vma)
{
+ struct mm_struct *mm = vma->vm_mm;
unsigned long ino = 0;
struct file *file;
dev_t dev = 0;
@@ -155,11 +164,14 @@ static int nommu_vma_show(struct seq_file *m, struct vm_area_struct *vma)
MAJOR(dev), MINOR(dev), ino, &len);

if (file) {
- len = 25 + sizeof(void *) * 6 - len;
- if (len < 1)
- len = 1;
- seq_printf(m, "%*c", len, ' ');
+ pad_len_spaces(m, len);
seq_path(m, &file->f_path, "");
+ } else if (mm) {
+ if (vma->vm_start <= mm->start_stack &&
+ vma->vm_end >= mm->start_stack) {
+ pad_len_spaces(m, len);
+ seq_puts(m, "[stack]");
+ }
}

seq_putc(m, '\n');


2010-06-01 19:12:27

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] NOMMU: Add '[stack]' label to /proc/pid/maps output



On Tue, 1 Jun 2010, David Howells wrote:
>
> From: Mike Frysinger <[email protected]>
>
> Add support to the NOMMU /proc/pid/maps file to show which mapping is the stack
> of the original thread after execve. This is largely based on the MMU code.

Umm? The MMU code that we _reverted_?

It turns out to be totally useless, since people put stacks in various
different places, and the values the kernel does see end up not even being
the "real" stack top.

See commit 34441427aab4bdb3069a4ffcda69a99357abcb2e.

Just don't do it.

Linus

2010-06-01 20:57:15

by Mike Frysinger

[permalink] [raw]
Subject: Re: [Uclinux-dist-devel] [PATCH] NOMMU: Add '[stack]' label to /proc/pid/maps output

On Tue, Jun 1, 2010 at 15:07, Linus Torvalds wrote:
> On Tue, 1 Jun 2010, David Howells wrote:
>> Add support to the NOMMU /proc/pid/maps file to show which mapping is the stack
>> of the original thread after execve.  This is largely based on the MMU code.
>
> Umm? The MMU code that we _reverted_?

to be fair, this patch was written a few kernel revisions ago and
you're referring to commit merged a few weeks ago ...

> It turns out to be totally useless, since people put stacks in various
> different places, and the values the kernel does see end up not even being
> the "real" stack top.
>
> See commit 34441427aab4bdb3069a4ffcda69a99357abcb2e.
>
> Just don't do it.

but i dont think we're talking the same thing. that commit refers to
[threadstack]. afaict, [stack] is still in there.
-mike

2010-06-01 21:11:10

by Linus Torvalds

[permalink] [raw]
Subject: Re: [Uclinux-dist-devel] [PATCH] NOMMU: Add '[stack]' label to /proc/pid/maps output



On Tue, 1 Jun 2010, Mike Frysinger wrote:
>
> to be fair, this patch was written a few kernel revisions ago and
> you're referring to commit merged a few weeks ago ...

Yeah, but it got sent to me today, which is why I reacted. But:

> but i dont think we're talking the same thing. that commit refers to
> [threadstack]. afaict, [stack] is still in there.

Oooh, and yes I did miss that entirely. The "[stack]" thing has been part
of /proc/pid/maps since 2005 (v2.6.12-rc1 according to the historical
logs), so it never even entered my mind that it would be about that
ancient thing.

But right you are, and I withdraw any objections. Sorry for the noise,

Linus

2010-06-04 21:30:45

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] NOMMU: Add '[stack]' label to /proc/pid/maps output

On Tue, 01 Jun 2010 19:59:37 +0100
David Howells <[email protected]> wrote:

> From: Mike Frysinger <[email protected]>
>
> Add support to the NOMMU /proc/pid/maps file to show which mapping is the stack
> of the original thread after execve. This is largely based on the MMU code.
> Subsidiary thread stacks are not indicated.
>
> For FDPIC, we now get:
>
> root:/> cat /proc/self/maps
> 02064000-02067ccc rw-p 0004d000 00:01 22 /bin/busybox
> 0206e000-0206f35c rw-p 00006000 00:01 295 /lib/ld-uClibc.so.0
> 025f0000-025f6f0c r-xs 00000000 00:01 295 /lib/ld-uClibc.so.0
> 02680000-026ba6b0 r-xs 00000000 00:01 297 /lib/libc.so.0
> 02700000-0274d384 r-xs 00000000 00:01 22 /bin/busybox
> 02816000-02817000 rw-p 00000000 00:00 0
> 02848000-0284c0d8 rw-p 00000000 00:00 0
> 02860000-02880000 rw-p 00000000 00:00 0 [stack]
>
> The semi-downside here is that for FLAT, we get:
>
> root:/> cat /proc/155/maps
> 029f0000-029f9000 rwxp 00000000 00:00 0 [stack]
>
> The reason being that FLAT combines a whole lot of stuff into one map
> (including the stack). But this isn't any worse than the current output (which
> is nothing), so screw it.
>

So it's a non-back-compatible change which can a) break nommu-only
userspace and b) unbreak mmu-tested userspace which gets run on nommu.

ho hum, we suck. They can send us the bill.

> --- a/fs/proc/task_nommu.c
> +++ b/fs/proc/task_nommu.c
> @@ -122,11 +122,20 @@ int task_statm(struct mm_struct *mm, int *shared, int *text,
> return size;
> }
>
> +static void pad_len_spaces(struct seq_file *m, int len)
> +{
> + len = 25 + sizeof(void*) * 6 - len;
> + if (len < 1)
> + len = 1;
> + seq_printf(m, "%*c", len, ' ');
> +}

hm, copy-n-paste alert.

2010-06-04 23:00:44

by Mike Frysinger

[permalink] [raw]
Subject: Re: [Uclinux-dist-devel] [PATCH] NOMMU: Add '[stack]' label to /proc/pid/maps output

On Fri, Jun 4, 2010 at 16:48, Andrew Morton wrote:
> On Tue, 01 Jun 2010 19:59:37 +0100 David Howells wrote:
>> From: Mike Frysinger <[email protected]>
>>
>> Add support to the NOMMU /proc/pid/maps file to show which mapping is the stack
>> of the original thread after execve.  This is largely based on the MMU code.
>> Subsidiary thread stacks are not indicated.
>>
>> For FDPIC, we now get:
>>
>>       root:/> cat /proc/self/maps
>>       02064000-02067ccc rw-p 0004d000 00:01 22         /bin/busybox
>>       0206e000-0206f35c rw-p 00006000 00:01 295        /lib/ld-uClibc.so.0
>>       025f0000-025f6f0c r-xs 00000000 00:01 295        /lib/ld-uClibc.so.0
>>       02680000-026ba6b0 r-xs 00000000 00:01 297        /lib/libc.so.0
>>       02700000-0274d384 r-xs 00000000 00:01 22         /bin/busybox
>>       02816000-02817000 rw-p 00000000 00:00 0
>>       02848000-0284c0d8 rw-p 00000000 00:00 0
>>       02860000-02880000 rw-p 00000000 00:00 0          [stack]
>>
>> The semi-downside here is that for FLAT, we get:
>>
>>       root:/> cat /proc/155/maps
>>       029f0000-029f9000 rwxp 00000000 00:00 0          [stack]
>>
>> The reason being that FLAT combines a whole lot of stuff into one map
>> (including the stack).  But this isn't any worse than the current output (which
>> is nothing), so screw it.
>
> So it's a non-back-compatible change which can a) break nommu-only
> userspace and b) unbreak mmu-tested userspace which gets run on nommu.

i dont see how it breaks anything really. any code that parses the
maps file has to account for an optional label in the maps output.

i guess if someone wrote some code that does something special with
"[stack]", then behavior would change with FLAT files, but that's
pretty unlikely i would think. plus, FLAT is not ELF and as such, its
layout in memory is highly unlike any existing ELF layout. so someone
would already have to be writing a custom handler for FLAT files which
means they account for the "one blob is everything" layout.
-mike