Mailing List Archive: fs/binfmt_elf.c:maydump()

fs/binfmt_elf.c:maydump()

davem at davemloft

Apr 6, 2006, 2:03 PM

Post #1 of 7 (1065 views)

I sort of understand the idea behind this check in maydump():

/* If it hasn't been written to, don't write it out */
if (!vma->anon_vma)
return 0;

but it causes real problems for debugging. In fact a GDB testcase
breaks because of this check.

In the GDB testcase, the application mmap()'s a file with some
text in it. It then calls abort() to dump core. Then GDB loads
up the application again using that core file, and it tries to
look at the mmap()'d file, and that doesn't work. We don't dump
the file contents because of the above check so GDB has no idea
how to reproduce the application state at the time of the core
dump.

Furthermore, it is vitally important to dump such areas to handle the
case where the file contents change after the core dump occurs.
So even if we had some way to tell GDB the full pathname of the
file which was mapped at that location, we should still dump the
contents and not try to elide them via this check in maydump().

Yes, this means we might hit the core dump limits quicker but we
shouldn't be doing anything which makes less debugging information
than necessary available. Software development is hard enough as
it is right? :)

I also have a strange feeling that the VM_SHARED/i_nlink==0 check
might cause similar problems, but I won't touch that for now.

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 537893a..7fea878 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1167,10 +1167,6 @@ static int maydump(struct vm_area_struct
if (vma->vm_flags & VM_SHARED)
return vma->vm_file->f_dentry->d_inode->i_nlink == 0;

- /* If it hasn't been written to, don't write it out */
- if (!vma->anon_vma)
- return 0;
-
return 1;
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: fs/binfmt_elf.c:maydump() [ In reply to ]

Apr 6, 2006, 3:15 PM

Post #2 of 7 (1050 views)

On Thu, Apr 06, 2006 at 02:03:57PM -0700, David S. Miller wrote:
> Yes, this means we might hit the core dump limits quicker but we
> shouldn't be doing anything which makes less debugging information
> than necessary available. Software development is hard enough as
> it is right? :)

> - /* If it hasn't been written to, don't write it out */
> - if (!vma->anon_vma)
> - return 0;
> -

Isn't this, um, a little more extreme than what you really want?
What goes into coredumps with this patch applied? I bet it includes
the complete text segments of every executable and shared library
involved in the link. You're going to need those if you want to debug,
anyway.

--
Daniel Jacobowitz
CodeSourcery
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: fs/binfmt_elf.c:maydump() [ In reply to ]

davem at davemloft

Apr 6, 2006, 3:35 PM

Post #3 of 7 (1057 views)

From: Daniel Jacobowitz <dan@debian.org>
Date: Thu, 6 Apr 2006 18:15:19 -0400

> On Thu, Apr 06, 2006 at 02:03:57PM -0700, David S. Miller wrote:
> > Yes, this means we might hit the core dump limits quicker but we
> > shouldn't be doing anything which makes less debugging information
> > than necessary available. Software development is hard enough as
> > it is right? :)
>
> > - /* If it hasn't been written to, don't write it out */
> > - if (!vma->anon_vma)
> > - return 0;
> > -
>
> Isn't this, um, a little more extreme than what you really want?
> What goes into coredumps with this patch applied? I bet it includes
> the complete text segments of every executable and shared library
> involved in the link. You're going to need those if you want to debug,
> anyway.

With this patch applied, yes, it includes the complete text segments of
every executable and shared library mapped into the program which is
dumping core.

What's a good check to avoid shared libraries and executables? And do
we really want to avoid including them? What if a new version of one
of the shared libraries is installed on the system after the core file
is generated? Wouldn't you want the complete original image so that
the program could be debugged accurately in the face of such changes?

Anyways a possible check would be if the object was mapped with
execute permission, so a test on VM_EXEC being set in vma->vm_flags.

But like the comment above maydump() seems to suggest, I'm of the
opinion that we should include as much as possible into the core
file image.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: fs/binfmt_elf.c:maydump() [ In reply to ]

davem at davemloft

Apr 6, 2006, 10:18 PM

Post #4 of 7 (1045 views)

From: "David S. Miller" <davem@davemloft.net>
Date: Thu, 06 Apr 2006 15:35:18 -0700 (PDT)

> With this patch applied, yes, it includes the complete text segments of
> every executable and shared library mapped into the program which is
> dumping core.

I did some more research and the 2.4.x kernel currently does
the test like this:

static inline int maydump(struct vm_area_struct *vma)
{
/*
* If we may not read the contents, don't allow us to dump
* them either. "dump_write()" can't handle it anyway.
*/
if (!(vma->vm_flags & VM_READ))
return 0;

/* Do not dump I/O mapped devices! -DaveM */
if (vma->vm_flags & VM_IO)
return 0;
#if 1
if (vma->vm_flags & (VM_WRITE|VM_GROWSUP|VM_GROWSDOWN))
return 1;
if (vma->vm_flags & (VM_READ|VM_EXEC|VM_EXECUTABLE|VM_SHARED))
return 0;
#endif
return 1;
}

Ok, if it's not readable don't put it into the core file, fine.
If it's an I/O mapping, skip it too, also makes sense.

The next check forces dumping of any wriable or stack segments.

The function always terminates at the next check, because VM_READ
is guarenteed to be set if we get here. The first thing we
checked is if VM_READ was clear at the very top of the function.

Yikes...

Anyways, so what's exactly happening in 2.6.x right now?

static int maydump(struct vm_area_struct *vma)
{
/* Do not dump I/O mapped devices or special mappings */
if (vma->vm_flags & (VM_IO | VM_RESERVED))
return 0;

/* Dump shared memory only if mapped from an anonymous file. */
if (vma->vm_flags & VM_SHARED)
return vma->vm_file->f_dentry->d_inode->i_nlink == 0;

/* If it hasn't been written to, don't write it out */
if (!vma->anon_vma)
return 0;

return 1;
}

Skip reserved or I/O mappings, ok.

Else skip shared mappings of non-anonymous files. I'm not so sure
about this check.

Else skip any mapping not written to yet. I still think at this
point the logic is wrong.

How about something like the following patch? If it's executable
and not written to, skip it. This would skip the main executable
image and all text segments of the shared libraries mapped in.

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 537893a..9ec5c2b 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1167,8 +1167,10 @@ static int maydump(struct vm_area_struct
if (vma->vm_flags & VM_SHARED)
return vma->vm_file->f_dentry->d_inode->i_nlink == 0;

- /* If it hasn't been written to, don't write it out */
- if (!vma->anon_vma)
+ /* If it is executable and hasn't been written to,
+ * don't write it out.
+ */
+ if ((vma->vm_flags & VM_EXEC) && !vma->anon_vma)
return 0;

return 1;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: fs/binfmt_elf.c:maydump() [ In reply to ]

Apr 7, 2006, 11:02 AM

Post #5 of 7 (1025 views)

On Thu, Apr 06, 2006 at 10:18:07PM -0700, David S. Miller wrote:
> How about something like the following patch? If it's executable
> and not written to, skip it. This would skip the main executable
> image and all text segments of the shared libraries mapped in.

Will this dump text segments that have been COW'd for the purposes of
inserting a breakpoint?

It's just a question of goals, I guess. We could dump code, but it's
rarely useful, so historically we didn't. Similarly, we could dump
mapped data from shared memory, but it can be huge and is rarely
useful, so generally we don't.

--
Daniel Jacobowitz
CodeSourcery
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: fs/binfmt_elf.c:maydump() [ In reply to ]

davem at davemloft

Apr 7, 2006, 1:27 PM

Post #6 of 7 (1029 views)

From: Daniel Jacobowitz <dan@debian.org>
Date: Fri, 7 Apr 2006 14:02:43 -0400

> On Thu, Apr 06, 2006 at 10:18:07PM -0700, David S. Miller wrote:
> > How about something like the following patch? If it's executable
> > and not written to, skip it. This would skip the main executable
> > image and all text segments of the shared libraries mapped in.
>
> Will this dump text segments that have been COW'd for the purposes of
> inserting a breakpoint?

Yes, and it would also dump text segments that get written
by the dynamic linker such as the .plt, which we definitely
do want.

It would also dump impure text segment cases as well.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: fs/binfmt_elf.c:maydump() [ In reply to ]

Apr 10, 2006, 6:01 AM

Post #7 of 7 (1045 views)

On Fri, Apr 07, 2006 at 01:27:53PM -0700, David S. Miller wrote:
> Yes, and it would also dump text segments that get written
> by the dynamic linker such as the .plt, which we definitely
> do want.
>
> It would also dump impure text segment cases as well.

Well, I'm OK with this, upon reflection. Might as well merge it and
see if anyone else is appalled :-)

--
Daniel Jacobowitz
CodeSourcery
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/