linux-kernel - Re: [PATCH] [v3] warn on performance-impacting configs aka. TAINT

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]

Message-ID: <53F75B91.2040100@sr71.net>
Date:	Fri, 22 Aug 2014 08:02:41 -0700
From:	Dave Hansen <dave@...1.net>
To:	Ingo Molnar <mingo@...nel.org>
CC:	linux-kernel@...r.kernel.org, dave.hansen@...ux.intel.com,
	peterz@...radead.org, mingo@...hat.com, ak@...ux.intel.com,
	tim.c.chen@...ux.intel.com, akpm@...ux-foundation.org,
	cl@...ux.com, penberg@...nel.org, linux-mm@...ck.org,
	kirill@...temov.name, lauraa@...eaurora.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] [v3] warn on performance-impacting configs aka. TAINT_PERFORMANCE

On 08/22/2014 12:20 AM, Ingo Molnar wrote:
> Essentially all DEBUG_OBJECTS_* options are expensive, assuming 
> they are enabled, i.e. DEBUG_OBJECTS_ENABLE_DEFAULT=y.
> 
> Otherwise they should only be warned about if the debugobjects 
> boot option got enabled.
> 
> I.e. you'll need a bit of a runtime check for this one.

At that point, what do we print, and when do we print it?  We're not
saying that the config option should be disabled because it's really the
boot option plus the config option that is causing the problem.

I'll just put the DEBUG_OBJECTS_ENABLE_DEFAULT in here which is
analogous to what we're doing with SLUB_DEBUG_ON.

>> +static ssize_t performance_taint_read(struct file *file, char __user *user_buf,
>> +			size_t count, loff_t *ppos)
>> +{
>> +	int i;
>> +	int ret;
>> +	char *buf;
>> +	size_t buf_written = 0;
>> +	size_t buf_left;
>> +	size_t buf_len;
>> +
>> +	if (!ARRAY_SIZE(perfomance_killing_configs))
>> +		return 0;
>> +
>> +	buf_len = 1;
>> +	for (i = 0; i < ARRAY_SIZE(perfomance_killing_configs); i++)
>> +		buf_len += strlen(config_prefix) +
>> +			   strlen(perfomance_killing_configs[i]);
>> +	/* Add a byte for for each entry in the array for a \n */
>> +	buf_len += ARRAY_SIZE(perfomance_killing_configs);
>> +
>> +	buf = kmalloc(buf_len, GFP_KERNEL);
>> +	if (!buf)
>> +		return -ENOMEM;
>> +
>> +	buf_left = buf_len;
>> +	for (i = 0; i < ARRAY_SIZE(perfomance_killing_configs); i++) {
>> +		buf_written += snprintf(buf + buf_written, buf_left,
>> +					"%s%s\n", config_prefix,
>> +					perfomance_killing_configs[i]);
>> +		buf_left = buf_len - buf_written;
> 
> So, ARRAY_SIZE(performance_killing_configs) is written out four 
> times, a temporary variable would be in order I suspect.

If one of them had gone over 80 chars, I probably would have. :)  I put
one in anyway.

> Also, do you want to check buf_left and break out early from 
> the loop if it goes non-positive?

You're slowly inflating my patch for no practical gain. :)

Oh well, I put that in too.

I think I got the rest of the things addressed too.  v4 coming soon.
--
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/