linux-kernel - Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

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: <87si36kx4m.fsf@rasmusvillemoes.dk>
Date:	Sun, 13 Dec 2015 19:43:21 +0100
From:	Rasmus Villemoes <linux@...musvillemoes.dk>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Neil Brown <neilb@...e.com>
Subject: Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Sat, Dec 12 2015, Al Viro <viro@...IV.linux.org.uk> wrote:

> On Fri, Dec 11, 2015 at 11:16:24PM +0000, Al Viro wrote:
>
>> #define set_delayed_type(call, f, arg) \
>> 	sizeof(f(arg),0), \
>> 	__set_delayed_type(call, __closure_##f, (void *)arg)
>> 
>> That could be reused for timers with typechecking - we have a lot of timer
>> callbacks that start with casting the argument (unsigned long, not void *,
>> but that's not a big deal) to whatever it is that callback really wants,
>> with setup_timer() callers explicitly casting that whatever the callback
>> really wants to unsigned long.  Which, of course, defeats the typechecking
>> by both cc(1) and sparse(1)...
>> 
>> I still hope for better solution, though...  Comments?
>
> Hmm...
>
> #define set_delayed_type(call, f, arg)					\
>  	(sizeof(f(arg),0),						\
> 	__set_delayed_type(call, ({					\
> 		void _(void *__) {f((typeof(arg))(unsigned long)__);}	\
> 		_;}), (void *)arg))
>
> woult do nicely, but it's a gccism, and the one clang doesn't support
> ;-/

Careful. While this may appear to work, I'm pretty sure it would break
horribly when f is not the actual name of a function but a function
pointer. Not that it won't compile, but it would presumably make gcc
emit what they call a 'trampoline', a small piece of executable code
located on the stack, allowing the stub to access the containing
function's local variables (namely, f). I don't know if kernel stacks
are even executable, but even then it's a bad idea, once the stack frame
containing the trampoline is gone. All of this could easily happen down
the line when someone sees a bunch of similar code and decides to factor
that into a helper(), and then it works whenever gcc decides to inline
the helper to both call sites and otherwise, as the gcc docs put it,
"all hell breaks loose". IOWs, an accident waiting to happen.

One could probably ensure that the macro is always used with a
compile/link-time constant by putting in some trick like

static void *link_time_constant_please = f;

but even then I wouldn't be completely sure that gcc would always emit a
stub as simple as the one you showed.

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