Development discussion of WireGuard
 help / color / mirror / Atom feed
From: Tom Li <tomli@tomli.me>
To: wireguard@lists.zx2c4.com
Subject: imer_setup() is not compatible with PaX's RAP
Date: Sat, 11 Nov 2017 16:09:20 +0800	[thread overview]
Message-ID: <20171111080920.GA5705@localhost.localdomain> (raw)

After upgraded to 0.0.20171101, I found my 4.9 PaX/grsecurity kernel
will panic soon if wireguard has been started, because of a RAP function
point type violation.

PAX: RAP hash violation for function pointer: peer_remove_all+0x450/0x930 [wireguard]
PAX: overwritten function pointer detected: 0000 [#1] PREEMPT SMP
RIP: 0010:[<ffffffff8115f670>]  [<ffffffff8115f670>] call_timer_fn+0x210/0x230

Basically it says call_timer_fn() was going to call a function pointer led to peer_remove_all(),
but the type of the function pointer doesn't match the function.

A quick look to the kernel source code showed it was a side-effect of Kees Cook's
new timer API in commit 686fef928bba6be13cabe639f154af7d72b63120 [1], which introduced
new timer_setup() function.

static inline void timer_setup(struct timer_list *timer,
                               void (*callback)(struct timer_list *),
                               unsigned int flags)

WireGuard has switched to this new API to setup timers since
7be989478f2fa659b0b6b55dbd72f222b932a5ee [2], and provided a macro to be compatible
with older kernels.

#define timer_setup(a, b, c) setup_timer(a, ((void (*)(unsigned long))b), ((unsigned long)a))

But it has an unfortunately consequence. a function type must match function pointer type used
in the indirect call, although conversion in-between is allowed, but it has to be matched
when the pointer is called, otherwise it's an undefined behavior.

Normally, it works on most compilers, but it triggers PaX's RAP as a breach of Control-Flow
Integrity. I don't think there were any RAP violations in the previous version, and as user
it would be the best to get it fixed in a way that doesn't involve incompatible function pointer
types to comfort RAP, but I don't know if WireGuard is still going to support 4.9 PaX kernel...

P.S: Now the kernel is doing essentially the same thing, but Cook said after the conversion is
finished, pointer cast will be removed. But on existing kernels it'll always be an issue...

Anyway, It's hard to say who break who... Probably it's going to work for me if I revert
7be989478f2fa659b0b6b55dbd72f222b932a5ee on my computer as for now...

At least it has been documented in the mailing list now...

Regards,
Tom Li

             reply	other threads:[~2017-11-11  8:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-11  8:09 Tom Li [this message]
2017-11-11  8:16 ` Tom Li
2017-11-12  2:50 ` Jason A. Donenfeld
2017-11-12  3:13   ` Jason A. Donenfeld
2017-11-13  1:39   ` PaX Team
2017-11-13 19:34     ` Jason A. Donenfeld
2017-11-14  0:15       ` PaX Team
2017-11-14  9:29         ` Jason A. Donenfeld
2017-11-14 10:29           ` Jason A. Donenfeld
2017-11-14 11:12           ` PaX Team
2017-11-28 12:20             ` Jason A. Donenfeld
2017-11-28 12:32               ` PaX Team
2017-11-28 12:36                 ` Jason A. Donenfeld
2017-11-28 12:50                   ` PaX Team

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171111080920.GA5705@localhost.localdomain \
    --to=tomli@tomli.me \
    --cc=wireguard@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).