linux-kernel - Re: [PATCH RESEND 1/4] sched: /proc/sched_stat fails on very very large machines.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 16 Jan 2013 13:53:52 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Nathan Zimmer <nzimmer@....com>
Cc:	mingo@...hat.com, peterz@...radead.org, tglx@...utronix.de,
	johnstul@...ibm.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND 1/4] sched: /proc/sched_stat fails on very very
 large machines.

On Tue, 15 Jan 2013 15:46:09 -0600
Nathan Zimmer <nzimmer@....com> wrote:

> On systems with 4096 cores doing a cat /proc/sched_stat fails.
> We are trying to push all the data into a single kmalloc buffer.
> The issue is on these very large machines all the data will not fit in 4mb.
> 
> A better solution is to not us the single_open mechanism but to provide
> our own seq_operations.
> 
> The output should be identical to previous version and thus not need the
> version number.
> 
> ...
>
> index 903ffa9..33a85c9 100644
> --- a/kernel/sched/stats.c
> +++ b/kernel/sched/stats.c
> @@ -21,9 +21,13 @@ static int show_schedstat(struct seq_file *seq, void *v)
>  	if (mask_str == NULL)
>  		return -ENOMEM;
>  
> -	seq_printf(seq, "version %d\n", SCHEDSTAT_VERSION);
> -	seq_printf(seq, "timestamp %lu\n", jiffies);
> -	for_each_online_cpu(cpu) {
> +	if (v == (void *)1) {

The magic-numbers-in-pointers at least need comments, please.  Or nice
and meaningful #defines.

> +		seq_printf(seq, "version %d\n", SCHEDSTAT_VERSION);
> +		seq_printf(seq, "timestamp %lu\n", jiffies);

The code leaks the memory at mask_str here.

> +	} else {
> +
> +		cpu = (unsigned long)(v - 2);
> +
>  		struct rq *rq = cpu_rq(cpu);
>  #ifdef CONFIG_SMP
>  		struct sched_domain *sd;
> @@ -72,35 +76,64 @@ static int show_schedstat(struct seq_file *seq, void *v)
>  		}
>  		rcu_read_unlock();
>  #endif
> +		kfree(mask_str);
>  	}
> -	kfree(mask_str);
>  	return 0;
>  }

Undoing this change will fix the leak.

The schedstats code (both the original and after the patch) appears to
be racy against cpu hotplug?  What prevents the rq from vanishing while
we're playing with it?


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