linux-kernel - Re: [PATCH 2/5] mm: remove unlikly NULL from kfree

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: <20090326161019.GT28946@ZenIV.linux.org.uk>
Date:	Thu, 26 Mar 2009 16:10:19 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Pekka Enberg <penberg@...helsinki.fi>
Cc:	Hua Zhong <hzhong@...il.com>, Steven Rostedt <rostedt@...dmis.org>,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Roland McGrath <roland@...hat.com>,
	Nick Piggin <nickpiggin@...oo.com.au>,
	Steven Rostedt <srostedt@...hat.com>,
	Christoph Lameter <cl@...ux-foundation.org>,
	Matt Mackall <mpm@...enic.com>
Subject: Re: [PATCH 2/5] mm: remove unlikly NULL from kfree

On Wed, Mar 25, 2009 at 03:51:49PM +0200, Pekka Enberg wrote:
> OK, so according to Steven, audit_syscall_exit() is one such call-site
> that shows up in the traces. I don't really understand what's going on
> there but if it is sane, maybe that warrants the removal of unlikely()
> from kfree(). Hmm?

*Shrug*

We certainly can add explicit check, but that'll keep asking for
patches "removing useless check".  The same applies to other places,
really.

And making kfree(NULL) break will keep reintroducing bugs, since people
will expect the behaviour of free(3)...

I don't have any serious objections to adding a check there; indeed,
the normal case there (
                if (context->state != AUDIT_RECORD_CONTEXT) {
                        kfree(context->filterkey);
                        context->filterkey = NULL;
                }
) is context->state != AUDIT_RECORD_CONTEXT, context->filterkey == NULL,
so it might be worth turning into
		if (unlikely(context->filterkey)) {
			if (context->state != AUDIT_RECORD_CONTEXT) {
				kfree(context->filterkey);
				context->filterkey = NULL;
			}
		}
anyway.  Just dubious about the goal in general...
--
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/