Introduce the capability to dynamically configure the maximum file
note size for ELF core dumps via sysctl. This enhancement removes
the previous static limit of 4MB, allowing system administrators to
adjust the size based on system-specific requirements or constraints.
- Remove hardcoded `MAX_FILE_NOTE_SIZE` from `fs/binfmt_elf.c`.
- Define `max_file_note_size` in `fs/coredump.c` with an initial value set to 4MB.
- Declare `max_file_note_size` as an external variable in `include/linux/coredump.h`.
- Add a new sysctl entry in `kernel/sysctl.c` to manage this setting at runtime.
$ sysctl -a | grep max_file_note_size
kernel.max_file_note_size = 4194304
$ sysctl -n kernel.max_file_note_size
4194304
$echo 519304 > /proc/sys/kernel/max_file_note_size
$sysctl -n kernel.max_file_note_size
519304
Signed-off-by: Vijay Nag <[email protected]>
Signed-off-by: Allen Pais <[email protected]>
---
fs/binfmt_elf.c | 3 +--
fs/coredump.c | 3 +++
include/linux/coredump.h | 1 +
kernel/sysctl.c | 8 ++++++++
4 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 5397b552fbeb..5fc7baa9ebf2 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1564,7 +1564,6 @@ static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
}
-#define MAX_FILE_NOTE_SIZE (4*1024*1024)
/*
* Format of NT_FILE note:
*
@@ -1592,7 +1591,7 @@ static int fill_files_note(struct memelfnote *note, struct coredump_params *cprm
names_ofs = (2 + 3 * count) * sizeof(data[0]);
alloc:
- if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */
+ if (size >= max_file_note_size) /* paranoia check */
return -EINVAL;
size = round_up(size, PAGE_SIZE);
/*
diff --git a/fs/coredump.c b/fs/coredump.c
index be6403b4b14b..a83c6cc893fc 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -56,10 +56,13 @@
static bool dump_vma_snapshot(struct coredump_params *cprm);
static void free_vma_snapshot(struct coredump_params *cprm);
+#define MAX_FILE_NOTE_SIZE (4*1024*1024)
+
static int core_uses_pid;
static unsigned int core_pipe_limit;
static char core_pattern[CORENAME_MAX_SIZE] = "core";
static int core_name_size = CORENAME_MAX_SIZE;
+unsigned int max_file_note_size = MAX_FILE_NOTE_SIZE;
struct core_name {
char *corename;
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index d3eba4360150..e1ae7ab33d76 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -46,6 +46,7 @@ static inline void do_coredump(const kernel_siginfo_t *siginfo) {}
#endif
#if defined(CONFIG_COREDUMP) && defined(CONFIG_SYSCTL)
+extern unsigned int max_file_note_size;
extern void validate_coredump_safety(void);
#else
static inline void validate_coredump_safety(void) {}
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 81cc974913bb..80cdc37f2fa2 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -63,6 +63,7 @@
#include <linux/mount.h>
#include <linux/userfaultfd_k.h>
#include <linux/pid.h>
+#include <linux/coredump.h>
#include "../lib/kstrtox.h"
@@ -1623,6 +1624,13 @@ static struct ctl_table kern_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "max_file_note_size",
+ .data = &max_file_note_size,
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
#ifdef CONFIG_PROC_SYSCTL
{
.procname = "tainted",
--
2.17.1
On Mon, Apr 29, 2024 at 05:21:28PM +0000, Allen Pais wrote:
> Introduce the capability to dynamically configure the maximum file
> note size for ELF core dumps via sysctl. This enhancement removes
> the previous static limit of 4MB, allowing system administrators to
> adjust the size based on system-specific requirements or constraints.
>
> - Remove hardcoded `MAX_FILE_NOTE_SIZE` from `fs/binfmt_elf.c`.
> - Define `max_file_note_size` in `fs/coredump.c` with an initial value set to 4MB.
> - Declare `max_file_note_size` as an external variable in `include/linux/coredump.h`.
> - Add a new sysctl entry in `kernel/sysctl.c` to manage this setting at runtime.
>
> $ sysctl -a | grep max_file_note_size
> kernel.max_file_note_size = 4194304
>
> $ sysctl -n kernel.max_file_note_size
> 4194304
>
> $echo 519304 > /proc/sys/kernel/max_file_note_size
>
> $sysctl -n kernel.max_file_note_size
> 519304
This doesn't highlight anything about *why*. So in practice you must've
hit a use case where ELF notes are huge, can you give an example of
that? The commit should also describe that this is only used in the path
of a coredump on ELF binaries via elf_core_dump().
More below.
> Signed-off-by: Vijay Nag <[email protected]>
> Signed-off-by: Allen Pais <[email protected]>
> ---
> fs/binfmt_elf.c | 3 +--
> fs/coredump.c | 3 +++
> include/linux/coredump.h | 1 +
> kernel/sysctl.c | 8 ++++++++
> 4 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 5397b552fbeb..5fc7baa9ebf2 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1564,7 +1564,6 @@ static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
> fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
> }
>
> -#define MAX_FILE_NOTE_SIZE (4*1024*1024)
> /*
> * Format of NT_FILE note:
> *
> @@ -1592,7 +1591,7 @@ static int fill_files_note(struct memelfnote *note, struct coredump_params *cprm
>
> names_ofs = (2 + 3 * count) * sizeof(data[0]);
> alloc:
> - if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */
> + if (size >= max_file_note_size) /* paranoia check */
> return -EINVAL;
> size = round_up(size, PAGE_SIZE);
> /*
> diff --git a/fs/coredump.c b/fs/coredump.c
> index be6403b4b14b..a83c6cc893fc 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -56,10 +56,13 @@
> static bool dump_vma_snapshot(struct coredump_params *cprm);
> static void free_vma_snapshot(struct coredump_params *cprm);
>
> +#define MAX_FILE_NOTE_SIZE (4*1024*1024)
> +
> static int core_uses_pid;
> static unsigned int core_pipe_limit;
> static char core_pattern[CORENAME_MAX_SIZE] = "core";
> static int core_name_size = CORENAME_MAX_SIZE;
> +unsigned int max_file_note_size = MAX_FILE_NOTE_SIZE;
>
> struct core_name {
> char *corename;
> diff --git a/include/linux/coredump.h b/include/linux/coredump.h
> index d3eba4360150..e1ae7ab33d76 100644
> --- a/include/linux/coredump.h
> +++ b/include/linux/coredump.h
> @@ -46,6 +46,7 @@ static inline void do_coredump(const kernel_siginfo_t *siginfo) {}
> #endif
>
> #if defined(CONFIG_COREDUMP) && defined(CONFIG_SYSCTL)
> +extern unsigned int max_file_note_size;
> extern void validate_coredump_safety(void);
> #else
> static inline void validate_coredump_safety(void) {}
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 81cc974913bb..80cdc37f2fa2 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -63,6 +63,7 @@
> #include <linux/mount.h>
> #include <linux/userfaultfd_k.h>
> #include <linux/pid.h>
> +#include <linux/coredump.h>
>
> #include "../lib/kstrtox.h"
>
> @@ -1623,6 +1624,13 @@ static struct ctl_table kern_table[] = {
> .mode = 0644,
> .proc_handler = proc_dointvec,
> },
> + {
> + .procname = "max_file_note_size",
> + .data = &max_file_note_size,
> + .maxlen = sizeof(unsigned int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec,
> + },
> #ifdef CONFIG_PROC_SYSCTL
No, please move this to coredump_sysctls in fs/coredump.c. And there is
no point in supporting int, this is unisgned int right? So use the right
proc handler for it.
If we're gonna do this, it makes sense to document the ELF note binary
limiations. Then, consider a defense too, what if a specially crafted
binary with a huge elf note are core dumped many times, what then?
Lifting to 4 MiB puts in a situation where abuse can lead to many silly
insane kvmalloc()s. Is that what we want? Why?
Luis
On Mon, Apr 29, 2024 at 05:21:28PM +0000, Allen Pais wrote:
> Introduce the capability to dynamically configure the maximum file
> note size for ELF core dumps via sysctl. This enhancement removes
> the previous static limit of 4MB, allowing system administrators to
> adjust the size based on system-specific requirements or constraints.
Under what conditions is this actually needed?
> [...]
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 81cc974913bb..80cdc37f2fa2 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -63,6 +63,7 @@
> #include <linux/mount.h>
> #include <linux/userfaultfd_k.h>
> #include <linux/pid.h>
> +#include <linux/coredump.h>
>
> #include "../lib/kstrtox.h"
>
> @@ -1623,6 +1624,13 @@ static struct ctl_table kern_table[] = {
> .mode = 0644,
> .proc_handler = proc_dointvec,
> },
> + {
> + .procname = "max_file_note_size",
> + .data = &max_file_note_size,
> + .maxlen = sizeof(unsigned int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec,
> + },
Please don't add new sysctls to kernel/sysctl.c. Put this in fs/coredump.c
instead, and name it "core_file_note_size_max". (A "max" suffix is more
common than prefixes, and I'd like it clarified that it relates to the
coredumper with the "core" prefix that match the other coredump sysctls.
-Kees
--
Kees Cook
On Mon, Apr 29, 2024 at 11:38 AM Luis Chamberlain <[email protected]> wrote:
>
> On Mon, Apr 29, 2024 at 05:21:28PM +0000, Allen Pais wrote:
> > Introduce the capability to dynamically configure the maximum file
> > note size for ELF core dumps via sysctl. This enhancement removes
> > the previous static limit of 4MB, allowing system administrators to
> > adjust the size based on system-specific requirements or constraints.
> >
> > - Remove hardcoded `MAX_FILE_NOTE_SIZE` from `fs/binfmt_elf.c`.
> > - Define `max_file_note_size` in `fs/coredump.c` with an initial value set to 4MB.
> > - Declare `max_file_note_size` as an external variable in `include/linux/coredump.h`.
> > - Add a new sysctl entry in `kernel/sysctl.c` to manage this setting at runtime.
> >
> > $ sysctl -a | grep max_file_note_size
> > kernel.max_file_note_size = 4194304
> >
> > $ sysctl -n kernel.max_file_note_size
> > 4194304
> >
> > $echo 519304 > /proc/sys/kernel/max_file_note_size
> >
> > $sysctl -n kernel.max_file_note_size
> > 519304
>
> This doesn't highlight anything about *why*. So in practice you must've
> hit a use case where ELF notes are huge, can you give an example of
> that? The commit should also describe that this is only used in the path
> of a coredump on ELF binaries via elf_core_dump().
>
Yes, I should have captured it. We have observed that during a crash
when there are more than 65k mmaps in memory, the existing fixed limit on the
size of the ELF notes section becomes a bottleneck. The notes section quickly
reaches its capacity, leading to incomplete memory segment information in the
resulting coredump. This truncation compromises the utility of the coredumps,
as crucial information about the memory state at the time of the crash
might be omitted.
I will add the above to the commit message. Hope that addresses your concern.
> More below.
>
> > Signed-off-by: Vijay Nag <[email protected]>
> > Signed-off-by: Allen Pais <[email protected]>
> > ---
> > fs/binfmt_elf.c | 3 +--
> > fs/coredump.c | 3 +++
> > include/linux/coredump.h | 1 +
> > kernel/sysctl.c | 8 ++++++++
> > 4 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> > index 5397b552fbeb..5fc7baa9ebf2 100644
> > --- a/fs/binfmt_elf.c
> > +++ b/fs/binfmt_elf.c
> > @@ -1564,7 +1564,6 @@ static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
> > fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
> > }
> >
> > -#define MAX_FILE_NOTE_SIZE (4*1024*1024)
> > /*
> > * Format of NT_FILE note:
> > *
> > @@ -1592,7 +1591,7 @@ static int fill_files_note(struct memelfnote *note, struct coredump_params *cprm
> >
> > names_ofs = (2 + 3 * count) * sizeof(data[0]);
> > alloc:
> > - if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */
> > + if (size >= max_file_note_size) /* paranoia check */
> > return -EINVAL;
> > size = round_up(size, PAGE_SIZE);
> > /*
> > diff --git a/fs/coredump.c b/fs/coredump.c
> > index be6403b4b14b..a83c6cc893fc 100644
> > --- a/fs/coredump.c
> > +++ b/fs/coredump.c
> > @@ -56,10 +56,13 @@
> > static bool dump_vma_snapshot(struct coredump_params *cprm);
> > static void free_vma_snapshot(struct coredump_params *cprm);
> >
> > +#define MAX_FILE_NOTE_SIZE (4*1024*1024)
> > +
> > static int core_uses_pid;
> > static unsigned int core_pipe_limit;
> > static char core_pattern[CORENAME_MAX_SIZE] = "core";
> > static int core_name_size = CORENAME_MAX_SIZE;
> > +unsigned int max_file_note_size = MAX_FILE_NOTE_SIZE;
> >
> > struct core_name {
> > char *corename;
> > diff --git a/include/linux/coredump.h b/include/linux/coredump.h
> > index d3eba4360150..e1ae7ab33d76 100644
> > --- a/include/linux/coredump.h
> > +++ b/include/linux/coredump.h
> > @@ -46,6 +46,7 @@ static inline void do_coredump(const kernel_siginfo_t *siginfo) {}
> > #endif
> >
> > #if defined(CONFIG_COREDUMP) && defined(CONFIG_SYSCTL)
> > +extern unsigned int max_file_note_size;
> > extern void validate_coredump_safety(void);
> > #else
> > static inline void validate_coredump_safety(void) {}
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index 81cc974913bb..80cdc37f2fa2 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -63,6 +63,7 @@
> > #include <linux/mount.h>
> > #include <linux/userfaultfd_k.h>
> > #include <linux/pid.h>
> > +#include <linux/coredump.h>
> >
> > #include "../lib/kstrtox.h"
> >
> > @@ -1623,6 +1624,13 @@ static struct ctl_table kern_table[] = {
> > .mode = 0644,
> > .proc_handler = proc_dointvec,
> > },
> > + {
> > + .procname = "max_file_note_size",
> > + .data = &max_file_note_size,
> > + .maxlen = sizeof(unsigned int),
> > + .mode = 0644,
> > + .proc_handler = proc_dointvec,
> > + },
> > #ifdef CONFIG_PROC_SYSCTL
>
> No, please move this to coredump_sysctls in fs/coredump.c. And there is
> no point in supporting int, this is unisgned int right? So use the right
> proc handler for it.
>
Will address it in v2.
> If we're gonna do this, it makes sense to document the ELF note binary
> limiations. Then, consider a defense too, what if a specially crafted
> binary with a huge elf note are core dumped many times, what then?
> Lifting to 4 MiB puts in a situation where abuse can lead to many silly
> insane kvmalloc()s. Is that what we want? Why?
>
You raise a good point. I need to see how we can safely handle this case.
Thanks,
Allen
> Luis
--
- Allen
On Mon, Apr 29, 2024 at 12:49 PM Kees Cook <[email protected]> wrote:
>
> On Mon, Apr 29, 2024 at 05:21:28PM +0000, Allen Pais wrote:
> > Introduce the capability to dynamically configure the maximum file
> > note size for ELF core dumps via sysctl. This enhancement removes
> > the previous static limit of 4MB, allowing system administrators to
> > adjust the size based on system-specific requirements or constraints.
>
> Under what conditions is this actually needed?
I addressed this in the email I sent out before this.
>
> > [...]
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index 81cc974913bb..80cdc37f2fa2 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -63,6 +63,7 @@
> > #include <linux/mount.h>
> > #include <linux/userfaultfd_k.h>
> > #include <linux/pid.h>
> > +#include <linux/coredump.h>
> >
> > #include "../lib/kstrtox.h"
> >
> > @@ -1623,6 +1624,13 @@ static struct ctl_table kern_table[] = {
> > .mode = 0644,
> > .proc_handler = proc_dointvec,
> > },
> > + {
> > + .procname = "max_file_note_size",
> > + .data = &max_file_note_size,
> > + .maxlen = sizeof(unsigned int),
> > + .mode = 0644,
> > + .proc_handler = proc_dointvec,
> > + },
>
> Please don't add new sysctls to kernel/sysctl.c. Put this in fs/coredump.c
> instead, and name it "core_file_note_size_max". (A "max" suffix is more
> common than prefixes, and I'd like it clarified that it relates to the
> coredumper with the "core" prefix that match the other coredump sysctls.
>
> -Kees
Makes sense. Let me know if the below looks fine,
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 5397b552fbeb..6aebd062b92b 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1564,7 +1564,6 @@ static void fill_siginfo_note(struct memelfnote
*note, user_siginfo_t *csigdata,
fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
}
-#define MAX_FILE_NOTE_SIZE (4*1024*1024)
/*
* Format of NT_FILE note:
*
@@ -1592,7 +1591,7 @@ static int fill_files_note(struct memelfnote
*note, struct coredump_params *cprm
names_ofs = (2 + 3 * count) * sizeof(data[0]);
alloc:
- if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */
+ if (size >= core_file_note_size_max) /* paranoia check */
return -EINVAL;
size = round_up(size, PAGE_SIZE);
/*
diff --git a/fs/coredump.c b/fs/coredump.c
index be6403b4b14b..2108eb93acb9 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -56,10 +56,13 @@
static bool dump_vma_snapshot(struct coredump_params *cprm);
static void free_vma_snapshot(struct coredump_params *cprm);
+#define MAX_FILE_NOTE_SIZE (4*1024*1024)
+
static int core_uses_pid;
static unsigned int core_pipe_limit;
static char core_pattern[CORENAME_MAX_SIZE] = "core";
static int core_name_size = CORENAME_MAX_SIZE;
+unsigned int core_file_note_size_max = MAX_FILE_NOTE_SIZE;
struct core_name {
char *corename;
@@ -1020,6 +1023,13 @@ static struct ctl_table coredump_sysctls[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "core_file_note_size_max",
+ .data = &core_file_note_size_max,
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_douintvec,
+ },
};
static int __init init_fs_coredump_sysctls(void)
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index d3eba4360150..14c057643e7f 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -46,6 +46,7 @@ static inline void do_coredump(const
kernel_siginfo_t *siginfo) {}
#endif
#if defined(CONFIG_COREDUMP) && defined(CONFIG_SYSCTL)
+extern unsigned int core_file_note_size_max;
extern void validate_coredump_safety(void);
#else
static inline void validate_coredump_safety(void) {}
Thanks,
Allen
On Mon, Apr 29, 2024 at 03:35:01PM -0700, Allen wrote:
> On Mon, Apr 29, 2024 at 12:49 PM Kees Cook <[email protected]> wrote:
> >
> > On Mon, Apr 29, 2024 at 05:21:28PM +0000, Allen Pais wrote:
> > > Introduce the capability to dynamically configure the maximum file
> > > note size for ELF core dumps via sysctl. This enhancement removes
> > > the previous static limit of 4MB, allowing system administrators to
> > > adjust the size based on system-specific requirements or constraints.
> >
> > Under what conditions is this actually needed?
>
> I addressed this in the email I sent out before this.
>
> >
> > > [...]
> > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > > index 81cc974913bb..80cdc37f2fa2 100644
> > > --- a/kernel/sysctl.c
> > > +++ b/kernel/sysctl.c
> > > @@ -63,6 +63,7 @@
> > > #include <linux/mount.h>
> > > #include <linux/userfaultfd_k.h>
> > > #include <linux/pid.h>
> > > +#include <linux/coredump.h>
> > >
> > > #include "../lib/kstrtox.h"
> > >
> > > @@ -1623,6 +1624,13 @@ static struct ctl_table kern_table[] = {
> > > .mode = 0644,
> > > .proc_handler = proc_dointvec,
> > > },
> > > + {
> > > + .procname = "max_file_note_size",
> > > + .data = &max_file_note_size,
> > > + .maxlen = sizeof(unsigned int),
> > > + .mode = 0644,
> > > + .proc_handler = proc_dointvec,
> > > + },
> >
> > Please don't add new sysctls to kernel/sysctl.c. Put this in fs/coredump.c
> > instead, and name it "core_file_note_size_max". (A "max" suffix is more
> > common than prefixes, and I'd like it clarified that it relates to the
> > coredumper with the "core" prefix that match the other coredump sysctls.
> >
> > -Kees
>
> Makes sense. Let me know if the below looks fine,
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 5397b552fbeb..6aebd062b92b 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1564,7 +1564,6 @@ static void fill_siginfo_note(struct memelfnote
> *note, user_siginfo_t *csigdata,
> fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
> }
>
> -#define MAX_FILE_NOTE_SIZE (4*1024*1024)
> /*
> * Format of NT_FILE note:
> *
> @@ -1592,7 +1591,7 @@ static int fill_files_note(struct memelfnote
> *note, struct coredump_params *cprm
>
> names_ofs = (2 + 3 * count) * sizeof(data[0]);
> alloc:
> - if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */
> + if (size >= core_file_note_size_max) /* paranoia check */
> return -EINVAL;
> size = round_up(size, PAGE_SIZE);
> /*
> diff --git a/fs/coredump.c b/fs/coredump.c
> index be6403b4b14b..2108eb93acb9 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -56,10 +56,13 @@
> static bool dump_vma_snapshot(struct coredump_params *cprm);
> static void free_vma_snapshot(struct coredump_params *cprm);
>
> +#define MAX_FILE_NOTE_SIZE (4*1024*1024)
> +
> static int core_uses_pid;
> static unsigned int core_pipe_limit;
> static char core_pattern[CORENAME_MAX_SIZE] = "core";
> static int core_name_size = CORENAME_MAX_SIZE;
> +unsigned int core_file_note_size_max = MAX_FILE_NOTE_SIZE;
>
> struct core_name {
> char *corename;
> @@ -1020,6 +1023,13 @@ static struct ctl_table coredump_sysctls[] = {
> .mode = 0644,
> .proc_handler = proc_dointvec,
> },
> + {
> + .procname = "core_file_note_size_max",
> + .data = &core_file_note_size_max,
> + .maxlen = sizeof(unsigned int),
> + .mode = 0644,
> + .proc_handler = proc_douintvec,
> + },
> };
>
> static int __init init_fs_coredump_sysctls(void)
> diff --git a/include/linux/coredump.h b/include/linux/coredump.h
> index d3eba4360150..14c057643e7f 100644
> --- a/include/linux/coredump.h
> +++ b/include/linux/coredump.h
> @@ -46,6 +46,7 @@ static inline void do_coredump(const
> kernel_siginfo_t *siginfo) {}
> #endif
>
> #if defined(CONFIG_COREDUMP) && defined(CONFIG_SYSCTL)
> +extern unsigned int core_file_note_size_max;
> extern void validate_coredump_safety(void);
> #else
> static inline void validate_coredump_safety(void) {}
Yes; thank you! I like this naming (and sysctl location) now. :)
-Kees
--
Kees Cook
> Will address it in v2.
>
> > If we're gonna do this, it makes sense to document the ELF note binary
> > limiations. Then, consider a defense too, what if a specially crafted
> > binary with a huge elf note are core dumped many times, what then?
> > Lifting to 4 MiB puts in a situation where abuse can lead to many silly
> > insane kvmalloc()s. Is that what we want? Why?
> >
> You raise a good point. I need to see how we can safely handle this case.
>
Luis,
Here's a rough idea that caps the max allowable size for the note section.
I am using 16MB as the max value.
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -56,10 +56,14 @@
static bool dump_vma_snapshot(struct coredump_params *cprm);
static void free_vma_snapshot(struct coredump_params *cprm);
+#define MAX_FILE_NOTE_SIZE (4*1024*1024)
+#define MAX_ALLOWED_NOTE_SIZE (16*1024*1024)
+
static int core_uses_pid;
static unsigned int core_pipe_limit;
static char core_pattern[CORENAME_MAX_SIZE] = "core";
static int core_name_size = CORENAME_MAX_SIZE;
+unsigned int core_file_note_size_max = MAX_FILE_NOTE_SIZE;
struct core_name {
char *corename;
@@ -1060,12 +1064,22 @@ static struct ctl_table coredump_sysctls[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "core_file_note_size_max",
+ .data = &core_file_note_size_max,
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_core_file_note_size_max,
+ },
};
+int proc_core_file_note_size_max(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp, loff_t *ppos) {
+ int error = proc_douintvec(table, write, buffer, lenp, ppos);
+ if (write && (core_file_note_size_max < MAX_FILE_NOTE_SIZE
+ || core_file_note_size_max > MAX_ALLOWED_NOTE_SIZE))
+. /* Revert to default if out of bounds */
+ core_file_note_size_max = MAX_FILE_NOTE_SIZE;
+ return error;
+}
Let me know what you think.
Thanks,
- Allen