linux-kernel - Re: [PATCH percpu/for-3.11 1/2] percpu-refcount: add __must_check to percpu_ref_init() and don't use ACCESS_ONCE() in percpu_ref_kill

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:	Thu, 20 Jun 2013 10:29:51 +0930
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Tejun Heo <tj@...nel.org>
Cc:	Kent Overstreet <koverstreet@...gle.com>,
	linux-kernel@...r.kernel.org, Oleg Nesterov <oleg@...hat.com>,
	Christoph Lameter <cl@...ux-foundation.org>
Subject: Re: [PATCH percpu/for-3.11 1/2] percpu-refcount: add __must_check to percpu_ref_init() and don't use ACCESS_ONCE() in percpu_ref_kill_rcu()

Tejun Heo <tj@...nel.org> writes:
> On Wed, Jun 19, 2013 at 12:25:14PM +0930, Rusty Russell wrote:
>> But it's quite OK to ignore OOM errors in builtin init functions.
>
> I think it'd be cleaner to let those use cases use BUG_ON() around it.
> We really want most users to be checking its return value.

Yeah, but it's an admission of API design failure.

__attribute__((warn_unused_result)) is a bad implementation of a poorly
conceived idea.  It was originally designed to catch realloc misuse,
which is presumably why casting to (void) doesn't suppress it.
Protecting realloc properly would mean the GCC understanding that the
pointer arg handed to realloc was no longer valid which would catch many
more cases, but compilers are hard, so we got the hacky attribute.

Now seems to get abused by lazy coders who blame users for their own
broken APIs.  And Ubuntu, who turn it on by default in their gcc when
optimizing.  Yeah, it's a sore point :)

So I end up writing code like this (to quote from ccan):

        /* Gcc's warn_unused_result is fascist bullshit. */
        #define doesnt_matter()
...
        	if (system(command))
        		doesnt_matter();

>> It would be neatest to have it fail into slow mode, of course, but it's
>> probably not worth the pain.
>
> percpu allocation is always GFP_KERNEL, so it can't get any slower
> without deadlocking.

Sorry, I was unclear.  If you fail the percpu allocation, you have a
counter which is always in atomic mode.

This saves everyone a headache.  init doesn't fail, no poorly-tested
failure paths, no whining Rusty.

Rant over,
Rusty.
--
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/