Subject: kdb: fix stack overflow for large NR_CPUS count
In auditing the 2.6.27 source I found a *big* problem in kdb. The stack in
kdb_bp will overflow with NR_CPUS=4096 ... the stack is 31744 bytes!
====== Stack (-l 500)
1 - x86_64_default_128 (NR_CPUS=128)
2 - x86_64_default (NR_CPUS=4096)
.1. .2. ..final..
1088 +31744 32832 +2917% kdb_bp
0 +2632 2632 . smp_call_function_mask
0 +2216 2216 . __build_sched_domains
0 +1544 1544 . tick_handle_oneshot_broadcast
The problem looks like the static array in kdb_bp (template):
static int
kdb_bp(int argc, const char **argv)
{
...
kdb_bp_t template = {0};
Which has a field sized by NR_CPUS:
kdbhard_bp_t *bp_hard[NR_CPUS]; /* Hardware breakpoint structure
*/
When NR_CPUS=4096, the above struct will overflow the 16k stack.
The fix I implemented is to move the template to static memory and
protect it with a spinlock. The spinlock most likely is not required
as it's highly unlikely that two bp commands can be entered into kdb
at the exact same time, but this definitely prevents that.
Some other minor tweaks to reduce stack size also implemented:
kdb_cpu_callback: use set_cpus_allowed_ptr() to prevent passing cpumask_t
on the stack.
kdb_per_cpu: use cpus_clear which prevents creating an empty cpumask_t
(CPU_MASK_NONE) on the stack.
For the next release, any arrays should be allocated using nr_cpu_ids.
Based on linux-2.6.27.
Signed-of-by: Mike Travis <[EMAIL PROTECTED]>
---
kdb/kdb_bp.c | 40 ++++++++++++++++++++++++++++------------
kdb/kdbmain.c | 12 ++++++------
2 files changed, 34 insertions(+), 18 deletions(-)
--- linux-2.6.27.orig/kdb/kdb_bp.c
+++ linux-2.6.27/kdb/kdb_bp.c
@@ -275,17 +275,20 @@ kdb_printbp(kdb_bp_t *bp, int i)
* bpha Set breakpoint on all cpus, force hardware register
*/
+/* protect template */
+static DEFINE_SPINLOCK(kdb_bp_inuse);
+static kdb_bp_t template;
+
static int
kdb_bp(int argc, const char **argv)
{
int i, bpno;
kdb_bp_t *bp, *bp_check;
- int diag;
+ int diag = 0;
int free;
char *symname = NULL;
long offset = 0ul;
int nextarg;
- kdb_bp_t template = {0};
if (argc == 0) {
/*
@@ -300,6 +303,14 @@ kdb_bp(int argc, const char **argv)
return 0;
}
+
+ /* highly unlikely that two bp cmds can be entered simultaneously */
+ if (!spin_trylock(&kdb_bp_inuse)) {
+ kdb_printf("Illegal second bp command!\n");
+ return KDB_BADMODE;
+ }
+ memset(&template, 0, sizeof(template));
+
template.bp_global = ((strcmp(argv[0], "bpa") == 0)
|| (strcmp(argv[0], "bpha") == 0));
template.bp_forcehw = ((strcmp(argv[0], "bph") == 0)
@@ -313,9 +324,11 @@ kdb_bp(int argc, const char **argv)
diag = kdbgetaddrarg(argc, argv, &nextarg, &template.bp_addr,
&offset, &symname);
if (diag)
- return diag;
- if (!template.bp_addr)
- return KDB_BADINT;
+ goto out;
+ if (!template.bp_addr) {
+ diag = KDB_BADINT;
+ goto out;
+ }
/*
* Find an empty bp structure, to allocate
@@ -327,16 +340,17 @@ kdb_bp(int argc, const char **argv)
}
}
- if (bpno == KDB_MAXBPT)
- return KDB_TOOMANYBPT;
+ if (bpno == KDB_MAXBPT) {
+ diag = KDB_TOOMANYBPT;
+ goto out;
+ }
/*
* Handle architecture dependent parsing
*/
diag = kdba_parsebp(argc, argv, &nextarg, &template);
- if (diag) {
- return diag;
- }
+ if (diag)
+ goto out;
/*
@@ -380,13 +394,15 @@ kdb_bp(int argc, const char **argv)
bp->bp_enabled = 0;
bp->bp_hardtype = 0;
kdba_free_hwbp(bp);
- return diag;
+ goto out;
}
}
kdb_printbp(bp, bpno);
- return 0;
+out:
+ spin_unlock(&kdb_bp_inuse);
+ return diag;
}
/*
--- linux-2.6.27.orig/kdb/kdbmain.c
+++ linux-2.6.27/kdb/kdbmain.c
@@ -3715,13 +3715,14 @@ kdb_per_cpu(int argc, const char **argv)
{
char buf[256], fmtstr[64];
kdb_symtab_t symtab;
- cpumask_t suppress = CPU_MASK_NONE;
+ cpumask_t suppress;
int cpu, diag;
unsigned long addr, val, bytesperword = 0, whichcpu = ~0UL;
if (argc < 1 || argc > 3)
return KDB_ARGCOUNT;
+ cpus_clear(suppress);
snprintf(buf, sizeof(buf), "per_cpu__%s", argv[1]);
if (!kdbgetsymval(buf, &symtab)) {
kdb_printf("%s is not a per_cpu variable\n", argv[1]);
@@ -3779,7 +3780,7 @@ kdb_per_cpu(int argc, const char **argv)
if (cpus_weight(suppress) == 0)
return 0;
kdb_printf("Zero suppressed cpu(s):");
- for (cpu = first_cpu(suppress); cpu < NR_CPUS; cpu = next_cpu(cpu,
suppress)) {
+ for_each_cpu_mask(cpu, suppress) {
kdb_printf(" %d", cpu);
if (cpu == NR_CPUS-1 || next_cpu(cpu, suppress) != cpu + 1)
continue;
@@ -4234,10 +4235,9 @@ kdb_cpu_callback(struct notifier_block *
if (action == CPU_ONLINE) {
int cpu =(unsigned long)hcpu;
cpumask_t save_cpus_allowed = current->cpus_allowed;
- cpumask_t new_cpus_allowed = cpumask_of_cpu(cpu);
- set_cpus_allowed(current, new_cpus_allowed);
- kdb(KDB_REASON_CPU_UP, 0, NULL); /* do kdb setup on this
cpu */
- set_cpus_allowed(current, save_cpus_allowed);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
+ kdb(KDB_REASON_CPU_UP, 0, NULL); /* do kdb setup on this cpu */
+ set_cpus_allowed_ptr(current, &save_cpus_allowed);
}
return NOTIFY_OK;
}
_______________________________________________
kdb mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/kdb
