All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps
@ 2016-02-19  0:32 Omar Sandoval
  2016-02-19  0:32 ` [PATCH 1/2] coredump: get rid of coredump_params->written Omar Sandoval
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Omar Sandoval @ 2016-02-19  0:32 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, Omar Sandoval

From: Omar Sandoval <osandov@fb.com>

Hi,

Someone here reported that they were getting truncated core dumps even
when RLIMIT_CORE was larger than the physical memory of the machine. It
looks some cleanup patches back in v3.13 [1] changed the behaviour of
the limit to also charge for sparse areas of a file. Here's an example
in 4.5-rc4, where a.out is:

----
#include <signal.h>

int main(int argc, char **argv)
{
	raise(SIGQUIT);
	return 0;
}
----

Make sure that your sysctl kernel.core_pattern isn't piping anywhere,
and be aware that there's some stupidity about the units used for ulimit
-c in different shells (bash uses 1024-byte blocks, same as what's shown
by du by default).

----
# ulimit -c unlimited
# ./a.out
Quit (core dumped)
# du core.248
88      core.248
# du --apparent-size core.248
232     core.248
# ulimit -c 128
# ./a.out
Quit (core dumped)
# du core.252
64      core.252
# du --apparent-size core.252
72      core.252
----

These 2 patches restore the original behavior:

----
# ulimit -c 128
# ./a.out
Quit (core dumped)
# du core.245
88      core.245
# du --apparent-size core.245
232     core.245
----

Patch 1 gets rid of cprm->written, since as far as I could tell, it's
always going to be equal to cprm->file->f_pos. Patch 2 reintroduces
cprm->written as the number of bytes actually written to the file, not
including what we seek over.

This series is based on 4.5-rc4. Al, could you apply these?

Thanks!

1: http://lkml.iu.edu/hypermail/linux/kernel/1310.1/00758.html

Appendix A: a quick sanity test with the patches applied

----
# ulimit -c unlimited
# python -c 'import os, signal; l = [0] * 1024 * 1024; os.kill(os.getpid(), signal.SIGQUIT)'
Quit (core dumped)
# du core.262
12268   core.262
# du --apparent-size core.262
12628   core.262
# ulimit -c 1024
# python -c 'import os, signal; l = [0] * 1024 * 1024; os.kill(os.getpid(), signal.SIGQUIT)'
Quit (core dumped)
# du core.266
1024    core.266
# du --apparent-size core.266
1024    core.266
----

Omar Sandoval (2):
  coredump: get rid of coredump_params->written
  coredump: only charge written data against RLIMIT_CORE

 arch/powerpc/platforms/cell/spufs/coredump.c | 5 +++--
 fs/binfmt_elf.c                              | 2 +-
 fs/binfmt_elf_fdpic.c                        | 2 +-
 fs/coredump.c                                | 7 ++-----
 4 files changed, 7 insertions(+), 9 deletions(-)

-- 
2.7.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] coredump: get rid of coredump_params->written
  2016-02-19  0:32 [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps Omar Sandoval
@ 2016-02-19  0:32 ` Omar Sandoval
  2016-02-19  0:32 ` [PATCH 2/2] coredump: only charge written data against RLIMIT_CORE Omar Sandoval
  2016-02-26 17:57 ` [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps Omar Sandoval
  2 siblings, 0 replies; 8+ messages in thread
From: Omar Sandoval @ 2016-02-19  0:32 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, Omar Sandoval

From: Omar Sandoval <osandov@fb.com>

cprm->written is redundant with cprm->file->f_pos, so use that instead.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 arch/powerpc/platforms/cell/spufs/coredump.c | 5 +++--
 fs/binfmt_elf.c                              | 2 +-
 fs/binfmt_elf_fdpic.c                        | 2 +-
 fs/coredump.c                                | 8 +++-----
 include/linux/binfmts.h                      | 1 -
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
index be6212ddbf06..84fb984f29c1 100644
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -137,6 +137,7 @@ static int spufs_arch_write_note(struct spu_context *ctx, int i,
 	char *name;
 	char fullname[80], *buf;
 	struct elf_note en;
+	size_t skip;
 
 	buf = (void *)get_zeroed_page(GFP_KERNEL);
 	if (!buf)
@@ -171,8 +172,8 @@ static int spufs_arch_write_note(struct spu_context *ctx, int i,
 	if (rc < 0)
 		goto out;
 
-	if (!dump_skip(cprm,
-		       roundup(cprm->written - total + sz, 4) - cprm->written))
+	skip = roundup(cprm->file->f_pos - total + sz, 4) - cprm->file->f_pos;
+	if (!dump_skip(cprm, skip))
 		goto Eio;
 out:
 	free_page((unsigned long)buf);
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 051ea4809c14..db2c784bb676 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -2273,7 +2273,7 @@ static int elf_core_dump(struct coredump_params *cprm)
 		goto end_coredump;
 
 	/* Align to page */
-	if (!dump_skip(cprm, dataoff - cprm->written))
+	if (!dump_skip(cprm, dataoff - cprm->file->f_pos))
 		goto end_coredump;
 
 	for (i = 0, vma = first_vma(current, gate_vma); vma != NULL;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index b1adb92e69de..8cfde2482468 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1787,7 +1787,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 				goto end_coredump;
 	}
 
-	if (!dump_skip(cprm, dataoff - cprm->written))
+	if (!dump_skip(cprm, dataoff - cprm->file->f_pos))
 		goto end_coredump;
 
 	if (!elf_fdpic_dump_segments(cprm))
diff --git a/fs/coredump.c b/fs/coredump.c
index 9ea87e9fdccf..dc9268bbd61b 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -760,7 +760,7 @@ int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 	struct file *file = cprm->file;
 	loff_t pos = file->f_pos;
 	ssize_t n;
-	if (cprm->written + nr > cprm->limit)
+	if (pos + nr > cprm->limit)
 		return 0;
 	while (nr) {
 		if (dump_interrupted())
@@ -769,7 +769,6 @@ int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 		if (n <= 0)
 			return 0;
 		file->f_pos = pos;
-		cprm->written += n;
 		nr -= n;
 	}
 	return 1;
@@ -781,12 +780,11 @@ int dump_skip(struct coredump_params *cprm, size_t nr)
 	static char zeroes[PAGE_SIZE];
 	struct file *file = cprm->file;
 	if (file->f_op->llseek && file->f_op->llseek != no_llseek) {
-		if (cprm->written + nr > cprm->limit)
+		if (file->f_pos + nr > cprm->limit)
 			return 0;
 		if (dump_interrupted() ||
 		    file->f_op->llseek(file, nr, SEEK_CUR) < 0)
 			return 0;
-		cprm->written += nr;
 		return 1;
 	} else {
 		while (nr > PAGE_SIZE) {
@@ -801,7 +799,7 @@ EXPORT_SYMBOL(dump_skip);
 
 int dump_align(struct coredump_params *cprm, int align)
 {
-	unsigned mod = cprm->written & (align - 1);
+	unsigned mod = cprm->file->f_pos & (align - 1);
 	if (align & (align - 1))
 		return 0;
 	return mod ? dump_skip(cprm, align - mod) : 1;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 576e4639ca60..39c6d6e1234e 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -64,7 +64,6 @@ struct coredump_params {
 	struct file *file;
 	unsigned long limit;
 	unsigned long mm_flags;
-	loff_t written;
 };
 
 /*
-- 
2.7.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] coredump: only charge written data against RLIMIT_CORE
  2016-02-19  0:32 [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps Omar Sandoval
  2016-02-19  0:32 ` [PATCH 1/2] coredump: get rid of coredump_params->written Omar Sandoval
@ 2016-02-19  0:32 ` Omar Sandoval
  2016-02-26 17:57 ` [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps Omar Sandoval
  2 siblings, 0 replies; 8+ messages in thread
From: Omar Sandoval @ 2016-02-19  0:32 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, Omar Sandoval

From: Omar Sandoval <osandov@fb.com>

Commit 9b56d54380ad ("dump_skip(): dump_seek() replacement taking
coredump_params") introduced a regression with regard to RLIMIT_CORE.
Previously, when a core dump was sparse, only the data that was actually
written out would count against the limit. Now, the sparse ranges are
also included, which leads to truncated core dumps when the actual disk
usage is still well below the limit. Restore the old behavior by only
counting what gets emitted and ignoring what gets skipped.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/coredump.c           | 5 ++---
 include/linux/binfmts.h | 1 +
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index dc9268bbd61b..8272bd511603 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -760,7 +760,7 @@ int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 	struct file *file = cprm->file;
 	loff_t pos = file->f_pos;
 	ssize_t n;
-	if (pos + nr > cprm->limit)
+	if (cprm->written + nr > cprm->limit)
 		return 0;
 	while (nr) {
 		if (dump_interrupted())
@@ -769,6 +769,7 @@ int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 		if (n <= 0)
 			return 0;
 		file->f_pos = pos;
+		cprm->written += nr;
 		nr -= n;
 	}
 	return 1;
@@ -780,8 +781,6 @@ int dump_skip(struct coredump_params *cprm, size_t nr)
 	static char zeroes[PAGE_SIZE];
 	struct file *file = cprm->file;
 	if (file->f_op->llseek && file->f_op->llseek != no_llseek) {
-		if (file->f_pos + nr > cprm->limit)
-			return 0;
 		if (dump_interrupted() ||
 		    file->f_op->llseek(file, nr, SEEK_CUR) < 0)
 			return 0;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 39c6d6e1234e..576e4639ca60 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -64,6 +64,7 @@ struct coredump_params {
 	struct file *file;
 	unsigned long limit;
 	unsigned long mm_flags;
+	loff_t written;
 };
 
 /*
-- 
2.7.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps
  2016-02-19  0:32 [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps Omar Sandoval
  2016-02-19  0:32 ` [PATCH 1/2] coredump: get rid of coredump_params->written Omar Sandoval
  2016-02-19  0:32 ` [PATCH 2/2] coredump: only charge written data against RLIMIT_CORE Omar Sandoval
@ 2016-02-26 17:57 ` Omar Sandoval
  2016-02-26 17:58   ` Omar Sandoval
  2016-02-26 18:31   ` Chris Mason
  2 siblings, 2 replies; 8+ messages in thread
From: Omar Sandoval @ 2016-02-26 17:57 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, Omar Sandoval

On Thu, Feb 18, 2016 at 04:32:52PM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> Hi,
> 
> Someone here reported that they were getting truncated core dumps even
> when RLIMIT_CORE was larger than the physical memory of the machine. It
> looks some cleanup patches back in v3.13 [1] changed the behaviour of
> the limit to also charge for sparse areas of a file.
[snip]
> 
> Omar Sandoval (2):
>   coredump: get rid of coredump_params->written
>   coredump: only charge written data against RLIMIT_CORE
> 
>  arch/powerpc/platforms/cell/spufs/coredump.c | 5 +++--
>  fs/binfmt_elf.c                              | 2 +-
>  fs/binfmt_elf_fdpic.c                        | 2 +-
>  fs/coredump.c                                | 7 ++-----
>  4 files changed, 7 insertions(+), 9 deletions(-)
> 
> -- 
> 2.7.1
> 

Ping, any change this can get in for -rc6?

-- 
Omar

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps
  2016-02-26 17:57 ` [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps Omar Sandoval
@ 2016-02-26 17:58   ` Omar Sandoval
  2016-02-26 18:31   ` Chris Mason
  1 sibling, 0 replies; 8+ messages in thread
From: Omar Sandoval @ 2016-02-26 17:58 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, Omar Sandoval

On Fri, Feb 26, 2016 at 09:57:22AM -0800, Omar Sandoval wrote:
> On Thu, Feb 18, 2016 at 04:32:52PM -0800, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > Hi,
> > 
> > Someone here reported that they were getting truncated core dumps even
> > when RLIMIT_CORE was larger than the physical memory of the machine. It
> > looks some cleanup patches back in v3.13 [1] changed the behaviour of
> > the limit to also charge for sparse areas of a file.
> [snip]
> > 
> > Omar Sandoval (2):
> >   coredump: get rid of coredump_params->written
> >   coredump: only charge written data against RLIMIT_CORE
> > 
> >  arch/powerpc/platforms/cell/spufs/coredump.c | 5 +++--
> >  fs/binfmt_elf.c                              | 2 +-
> >  fs/binfmt_elf_fdpic.c                        | 2 +-
> >  fs/coredump.c                                | 7 ++-----
> >  4 files changed, 7 insertions(+), 9 deletions(-)
> > 
> > -- 
> > 2.7.1
> > 
> 
> Ping, any change this can get in for -rc6?
> 
> -- 
> Omar

s/change/chance/

-- 
Omar

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps
  2016-02-26 17:57 ` [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps Omar Sandoval
  2016-02-26 17:58   ` Omar Sandoval
@ 2016-02-26 18:31   ` Chris Mason
  2016-03-18 21:39     ` Omar Sandoval
  1 sibling, 1 reply; 8+ messages in thread
From: Chris Mason @ 2016-02-26 18:31 UTC (permalink / raw)
  To: Omar Sandoval; +Cc: Al Viro, linux-fsdevel, Omar Sandoval

On Fri, Feb 26, 2016 at 09:57:22AM -0800, Omar Sandoval wrote:
> On Thu, Feb 18, 2016 at 04:32:52PM -0800, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > Hi,
> > 
> > Someone here reported that they were getting truncated core dumps even
> > when RLIMIT_CORE was larger than the physical memory of the machine. It
> > looks some cleanup patches back in v3.13 [1] changed the behaviour of
> > the limit to also charge for sparse areas of a file.
> [snip]
> > 
> > Omar Sandoval (2):
> >   coredump: get rid of coredump_params->written
> >   coredump: only charge written data against RLIMIT_CORE
> > 
> >  arch/powerpc/platforms/cell/spufs/coredump.c | 5 +++--
> >  fs/binfmt_elf.c                              | 2 +-
> >  fs/binfmt_elf_fdpic.c                        | 2 +-
> >  fs/coredump.c                                | 7 ++-----
> >  4 files changed, 7 insertions(+), 9 deletions(-)
> > 
> > -- 
> > 2.7.1
> > 
> 
> Ping, any change this can get in for -rc6?

Since this isn't a recent regression, I'd wait until the next merge
window instead of rc6.

-chris

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps
  2016-02-26 18:31   ` Chris Mason
@ 2016-03-18 21:39     ` Omar Sandoval
  2016-03-26  1:45       ` Omar Sandoval
  0 siblings, 1 reply; 8+ messages in thread
From: Omar Sandoval @ 2016-03-18 21:39 UTC (permalink / raw)
  To: Chris Mason, Al Viro, linux-fsdevel, Omar Sandoval

On Fri, Feb 26, 2016 at 01:31:08PM -0500, Chris Mason wrote:
> On Fri, Feb 26, 2016 at 09:57:22AM -0800, Omar Sandoval wrote:
> > On Thu, Feb 18, 2016 at 04:32:52PM -0800, Omar Sandoval wrote:
> > > From: Omar Sandoval <osandov@fb.com>
> > > 
> > > Hi,
> > > 
> > > Someone here reported that they were getting truncated core dumps even
> > > when RLIMIT_CORE was larger than the physical memory of the machine. It
> > > looks some cleanup patches back in v3.13 [1] changed the behaviour of
> > > the limit to also charge for sparse areas of a file.
> > [snip]
> > > 
> > > Omar Sandoval (2):
> > >   coredump: get rid of coredump_params->written
> > >   coredump: only charge written data against RLIMIT_CORE
> > > 
> > >  arch/powerpc/platforms/cell/spufs/coredump.c | 5 +++--
> > >  fs/binfmt_elf.c                              | 2 +-
> > >  fs/binfmt_elf_fdpic.c                        | 2 +-
> > >  fs/coredump.c                                | 7 ++-----
> > >  4 files changed, 7 insertions(+), 9 deletions(-)
> > > 
> > > -- 
> > > 2.7.1
> > > 
> > 
> > Ping, any change this can get in for -rc6?
> 
> Since this isn't a recent regression, I'd wait until the next merge
> window instead of rc6.
> 
> -chris

Al, does this look okay for the merge window?

-- 
Omar

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps
  2016-03-18 21:39     ` Omar Sandoval
@ 2016-03-26  1:45       ` Omar Sandoval
  0 siblings, 0 replies; 8+ messages in thread
From: Omar Sandoval @ 2016-03-26  1:45 UTC (permalink / raw)
  To: Chris Mason, Al Viro, linux-fsdevel, Omar Sandoval

On Fri, Mar 18, 2016 at 02:39:23PM -0700, Omar Sandoval wrote:
> On Fri, Feb 26, 2016 at 01:31:08PM -0500, Chris Mason wrote:
> > On Fri, Feb 26, 2016 at 09:57:22AM -0800, Omar Sandoval wrote:
> > > On Thu, Feb 18, 2016 at 04:32:52PM -0800, Omar Sandoval wrote:
> > > > From: Omar Sandoval <osandov@fb.com>
> > > > 
> > > > Hi,
> > > > 
> > > > Someone here reported that they were getting truncated core dumps even
> > > > when RLIMIT_CORE was larger than the physical memory of the machine. It
> > > > looks some cleanup patches back in v3.13 [1] changed the behaviour of
> > > > the limit to also charge for sparse areas of a file.
> > > [snip]
> > > > 
> > > > Omar Sandoval (2):
> > > >   coredump: get rid of coredump_params->written
> > > >   coredump: only charge written data against RLIMIT_CORE
> > > > 
> > > >  arch/powerpc/platforms/cell/spufs/coredump.c | 5 +++--
> > > >  fs/binfmt_elf.c                              | 2 +-
> > > >  fs/binfmt_elf_fdpic.c                        | 2 +-
> > > >  fs/coredump.c                                | 7 ++-----
> > > >  4 files changed, 7 insertions(+), 9 deletions(-)
> > > > 
> > > > -- 
> > > > 2.7.1
> > > > 
> > > 
> > > Ping, any change this can get in for -rc6?
> > 
> > Since this isn't a recent regression, I'd wait until the next merge
> > window instead of rc6.
> > 
> > -chris
> 
> Al, does this look okay for the merge window?
> 
> -- 
> Omar

Ping.

-- 
Omar

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-03-26  1:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-19  0:32 [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps Omar Sandoval
2016-02-19  0:32 ` [PATCH 1/2] coredump: get rid of coredump_params->written Omar Sandoval
2016-02-19  0:32 ` [PATCH 2/2] coredump: only charge written data against RLIMIT_CORE Omar Sandoval
2016-02-26 17:57 ` [PATCH 0/2] fix RLIMIT_CORE accounting for sparse dumps Omar Sandoval
2016-02-26 17:58   ` Omar Sandoval
2016-02-26 18:31   ` Chris Mason
2016-03-18 21:39     ` Omar Sandoval
2016-03-26  1:45       ` Omar Sandoval

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.