fix RLIMIT_CORE accounting for sparse dumps

PATCH Superseded Omar Sandoval 2 participants 8 messages Feb 19, 2016 – Mar 26, 2016

Subsystems (5)

VFS PowerPC exec and binfmt Cell broadband engine architecture +1 more subsystems

lore

Share & tools

lore

Omar Sandoval

author

2016-02-19 00:33 UTC #0 lore raw

cover fix RLIMIT_CORE accounting for sparse dumps

From: Omar Sandoval <[email protected]>

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(-)

Signature · 1 line

2.7.1

Omar Sandoval

author

2016-02-19 00:33 UTC #1 lore raw

PATCH 1/2 coredump: get rid of coredump_params->written

From: Omar Sandoval <[email protected]>

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

Signed-off-by Omar Sandoval <[email protected]>

---
 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(-)

arch/powerpc/platforms/cell/spufs/coredump.c +3 -2

index be6212ddbf06..84fb984f29c1 100644
@@ -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);

fs/binfmt_elf.c +1 -1

index 051ea4809c14..db2c784bb676 100644
@@ -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;

fs/binfmt_elf_fdpic.c +1 -1

index b1adb92e69de..8cfde2482468 100644
@@ -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))

fs/coredump.c +3 -5

index 9ea87e9fdccf..dc9268bbd61b 100644
@@ -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;

include/linux/binfmts.h +0 -1

index 576e4639ca60..39c6d6e1234e 100644
@@ -64,7 +64,6 @@ struct coredump_params {
 	struct file *file;
 	unsigned long limit;
 	unsigned long mm_flags;
-	loff_t written;
 };
 
 /*

Signature · 1 line

2.7.1

Omar Sandoval

author

2016-02-19 00:33 UTC #2 lore raw

PATCH 2/2 coredump: only charge written data against RLIMIT_CORE

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

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 <[email protected]>

---
 fs/coredump.c           | 5 ++---
 include/linux/binfmts.h | 1 +
 2 files changed, 3 insertions(+), 3 deletions(-)

fs/coredump.c +2 -3

index dc9268bbd61b..8272bd511603 100644
@@ -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;

include/linux/binfmts.h +1 -0

index 39c6d6e1234e..576e4639ca60 100644
@@ -64,6 +64,7 @@ struct coredump_params {
 	struct file *file;
 	unsigned long limit;
 	unsigned long mm_flags;
+	loff_t written;
 };
 
 /*

Signature · 1 line

2.7.1

Omar Sandoval

author

2016-02-26 17:57 UTC #3 lore raw

In reply to Omar Sandoval

Quoted 8 lines · Omar Sandoval

From: Omar Sandoval <[email protected]>
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]

Quoted 5 lines

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

Quoted 9 lines

 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(-)

Signature · 1 line

2.7.1

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

Signature · 1 line

Omar

Omar Sandoval

author

2016-02-26 17:58 UTC #4 lore raw

Quoted 21 lines · Omar Sandoval

Quoted 8 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
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]
Quoted 11 lines
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(-)

Quoted 8 lines

Quoted 3 lines
Signature · 1 line
2.7.1
Ping, any change this can get in for -rc6?
Signature · 1 line
Omar

s/change/chance/

Signature · 1 line

Omar

Chris Mason

2016-02-26 18:31 UTC #5 lore raw

In reply to Omar Sandoval

Quoted 21 lines · Omar Sandoval

Quoted 8 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
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]
Quoted 11 lines
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(-)

Quoted 5 lines

Quoted 3 lines
Signature · 1 line
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

Omar Sandoval

author

2016-03-18 21:39 UTC #6 lore raw

Quoted 26 lines · Chris Mason

Quoted 25 lines · Omar Sandoval
Quoted 8 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
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]
Quoted 14 lines
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(-)
Signature · 1 line
2.7.1

Quoted 6 lines

Quoted 1 line
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?

Signature · 1 line

Omar

Omar Sandoval

author

2016-03-26 01:45 UTC #7 lore raw

Quoted 27 lines · Omar Sandoval

Quoted 26 lines · Chris Mason
Quoted 25 lines · Omar Sandoval
Quoted 8 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
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]
Quoted 14 lines
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(-)
Signature · 1 line
2.7.1

Quoted 11 lines

Quoted 6 lines
Quoted 1 line
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?
Signature · 1 line
Omar

Ping.

Signature · 1 line

Omar