linux-kernel - Re: [PATCH] include/linux/slab.h: new KFREE() macro.

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: <1168239852.6202.15.camel@dsl081-166-245.sea1.dsl.speakeasy.net>
Date:	Sun, 07 Jan 2007 23:04:12 -0800
From:	Vadim Lobanov <vlobanov@...akeasy.net>
To:	Amit Choudhary <amit2030@...oo.com>
Cc:	Christoph Hellwig <hch@...radead.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] include/linux/slab.h: new KFREE() macro.

On Sun, 2007-01-07 at 20:09 -0800, Amit Choudhary wrote:
> I have already explained it earlier. I will try again. You will not need free_2: and free_1: with
> KFREE(). You will only need one free: with KFREE.

So, to rephrase, your stated goal is to get rid of any non-singular goto
labels in function error handling paths? Aside from sounding trippy in a
non-conformist kind of way, what benefits will this give to the kernel?

I ask this because there's already one easy-to-spot downside: you'll end
up calling kfree(NULL) a bunch of times that can be, and should be,
avoided. Whereas turning my computer into a better space-heater using
noops (like repeated kfree(NULL) calls) may be a noble goal, I'd much
rather not waste this planet's limited resources unnecessarily.

> Also, let's say that count is different for each array? Then how do you propose that memory be
> allocated in one pass?

The parameters to a '+' operator need not be equivalent.

> I have scanned the whole kernel to check whether people are checking for return values of kmalloc,
> I found that at many places they don't and have sent patches for them. Now, this too is brain
> damaged code. And during the scan I saw examples of what I described earlier.

These cases should be fixed independently of any particular KFREE()
agenda.

> KFREE() can fix some of those cases.

I am curious as to how a KFREE() macro can fix cases where people don't
check the kmalloc() return value.

> Below are some examples where people are doing KFREE() kind of stuff:

I glanced at one instance, and...

> arch/i386/kernel/cpu/cpufreq/acpi-cpufreq.c:				kfree(acpi_perf_data[j]);
> arch/i386/kernel/cpu/cpufreq/acpi-cpufreq.c-				acpi_perf_data[j] = NULL;

'acpi_perf_data' is a global and persistent data structure, where a NULL
value actually has a specific and distinct meaning (as in
acpi_cpufreq_cpu_init()). How you think this helps your argument with
setting a local pointer to NULL after free is beyond me.

-- Vadim Lobanov

-
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/