All of lore.kernel.org
 help / color / mirror / Atom feed
* imer_setup() is not compatible with PaX's RAP
@ 2017-11-11  8:09 Tom Li
  2017-11-11  8:16 ` Tom Li
  2017-11-12  2:50 ` Jason A. Donenfeld
  0 siblings, 2 replies; 14+ messages in thread
From: Tom Li @ 2017-11-11  8:09 UTC (permalink / raw)
  To: wireguard

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-11  8:09 imer_setup() is not compatible with PaX's RAP Tom Li
@ 2017-11-11  8:16 ` Tom Li
  2017-11-12  2:50 ` Jason A. Donenfeld
  1 sibling, 0 replies; 14+ messages in thread
From: Tom Li @ 2017-11-11  8:16 UTC (permalink / raw)
  To: wireguard

Oops, s/imer/timer, and missing reference links.

[1] https://github.com/torvalds/linux/commit/686fef928bba6be13cabe639f154af7d72b63120
[2] https://git.zx2c4.com/WireGuard/commit/?id=7be989478f2fa659b0b6b55dbd72f222b932a5ee

In addition, RAP design presentation.

[3] https://pax.grsecurity.net/docs/PaXTeam-H2HC15-RAP-RIP-ROP.pdf

Regards,
Tom Li

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-11  8:09 imer_setup() is not compatible with PaX's RAP Tom Li
  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
  1 sibling, 2 replies; 14+ messages in thread
From: Jason A. Donenfeld @ 2017-11-12  2:50 UTC (permalink / raw)
  To: Pipacs; +Cc: WireGuard mailing list

Hey Pipacs,

Looks like RAP is working correctly! That's heartening. If the below
didn't break, I suppose there'd be a problem. [See replied-to email
below.]

Upstream changed the argument of timer callbacks to `struct timer_list
*` instead of `unsigned long`. Since I'm trying to get my stuff
upstream, I generally code against the newest kernel, and then have
this disgusting file called compat.h [1] that abuses the preprocessor
to rewrite code based on which kernel I'm compiling against; I've got
it working for 3.10+.  If you woke up with good vision this morning,
I'm sorry, since your eyeballs are doubtlessly bleeding after glancing
at compat.h. In order to support old kernels, I just cast the
functions with argument `struct timer_list *` to functions with
argument `unsigned long`, and call it a day.

A grsecurity user has complained about this not working. I assume when
you guys move to 4.14, this will go away, but I also assume that the
person who emailed me below isn't actually a paying customer and
instead is using the old patches. So, I can't just say, "go to
grsecurity.net and get the latest." Yet, I'd like to continue
supporting PaX in my compat.h layer for as long as possible.

I'm wondering if you can think of any macro hackery I could surround
in `#ifdef CONFIG_PAX_RAP` inside compat.h, and have this issue go
away. Perhaps some kind of special rap_hash attributes? Or other
tricks?

Thanks,
Jason

[1] https://git.zx2c4.com/WireGuard/tree/src/compat/compat.h


On Sat, Nov 11, 2017 at 5:09 PM, Tom Li <tomli@tomli.me> wrote:
> 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
>
> [1] https://github.com/torvalds/linux/commit/686fef928bba6be13cabe639f154af7d72b63120
> [2] https://git.zx2c4.com/WireGuard/commit/?id=7be989478f2fa659b0b6b55dbd72f222b932a5ee

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-12  2:50 ` Jason A. Donenfeld
@ 2017-11-12  3:13   ` Jason A. Donenfeld
  2017-11-13  1:39   ` PaX Team
  1 sibling, 0 replies; 14+ messages in thread
From: Jason A. Donenfeld @ 2017-11-12  3:13 UTC (permalink / raw)
  To: Pipacs; +Cc: WireGuard mailing list

In other words, can I do better than this?

https://git.zx2c4.com/WireGuard/commit/?id=01eebded60497528249962d03582a163a90d3b22

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  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
  1 sibling, 1 reply; 14+ messages in thread
From: PaX Team @ 2017-11-13  1:39 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

On 12 Nov 2017 at 11:50, Jason A. Donenfeld wrote:

> Hey Pipacs,
> 
> Looks like RAP is working correctly! That's heartening. If the below
> didn't break, I suppose there'd be a problem. [See replied-to email
> below.]

indeed, this is one kind of problem that RAP's supposed to catch ;).

> A grsecurity user has complained about this not working. I assume when
> you guys move to 4.14, this will go away,

not really... while there're no in-tree users of timer_setup in 4.14
and thus in-tree timer functions can be called without triggering RAP,
your wireguard callbacks have the wrong prototype and will be caught.
you can only cheat the compiler at the assignment by that fptr cast,
at fptr dereference time RAP will still see the type hash mismatch.

> I'm wondering if you can think of any macro hackery I could surround
> in `#ifdef CONFIG_PAX_RAP` inside compat.h, and have this issue go
> away. Perhaps some kind of special rap_hash attributes? Or other
> tricks?

there's no way to satisfy RAP other than getting your types right.
the problem isn't in timer_setup per se, but at the actual indirect
calls to the timer functions and those will always trigger RAP unless
the timer functions have the correct prototype. now you could add
wrapper functions if you don't want to uglyfy the code headed upstream
but then you cannot generate them from the timer_setup macro since
that's invoked inside a function and C would at most let you declare
those wrapper functions there but not define them.

another approach could be to depend on TIMER_DATA_TYPE instead and use
it in your callback prototypes. as a sidenote, if you want to depend
on plugin related features, it's better to depend on RAP_PLUGIN and
similar (as you do with CONSTIFY_PLUGIN already) and not the actual
kernel config option since the latter can be true even if the given
compiler doesn't support plugins or the config could cover more than
the plugin and could work without it even (the STACKLEAK feature used
to be an example of that).

speaking of PaX support, you recently added some __ro_after_init wrapper
to wireguard which breaks under KERNEXEC when it's used on ops structs
(my __read_only has different semantics) so i have to revert it here but
it'd be nicer if you didn't define it when KERNEXEC is active.

cheers,
 PaX Team

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-13  1:39   ` PaX Team
@ 2017-11-13 19:34     ` Jason A. Donenfeld
  2017-11-14  0:15       ` PaX Team
  0 siblings, 1 reply; 14+ messages in thread
From: Jason A. Donenfeld @ 2017-11-13 19:34 UTC (permalink / raw)
  To: pageexec; +Cc: WireGuard mailing list

Hey Pipacs,

On Mon, Nov 13, 2017 at 2:39 AM, PaX Team <pageexec@freemail.hu> wrote:
> at fptr dereference time RAP will still see the type hash mismatch.
> another approach could be to depend on TIMER_DATA_TYPE instead and use
> it in your callback prototypes. as a sidenote, if you want to depend
> on plugin related features, it's better to depend on RAP_PLUGIN

I've fixed this all up here:
https://git.zx2c4.com/WireGuard/commit/?id=e4bf02b833f99f4dcc2ab685d92517ccf8cc4766

I think it _should_ work now. Thanks for the suggestions. I just
monkey patched the signatures of each of those functions. Ugly, but it
works.

>
> speaking of PaX support, you recently added some __ro_after_init wrapper
> to wireguard which breaks under KERNEXEC when it's used on ops structs
> (my __read_only has different semantics) so i have to revert it here but
> it'd be nicer if you didn't define it when KERNEXEC is active.

I've attempted to fix this in the same commit. Does that look like it will work?

By the way, if you ever find yourself having to revert things to run
WireGuard, don't hesitate to send a patch or just poke me, and I'll
fix things. I'd definitely like to support PaX for as long as I can
manage to do so.

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-13 19:34     ` Jason A. Donenfeld
@ 2017-11-14  0:15       ` PaX Team
  2017-11-14  9:29         ` Jason A. Donenfeld
  0 siblings, 1 reply; 14+ messages in thread
From: PaX Team @ 2017-11-14  0:15 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

On 13 Nov 2017 at 20:34, Jason A. Donenfeld wrote:

> I've fixed this all up here:
> https://git.zx2c4.com/WireGuard/commit/?id=e4bf02b833f99f4dcc2ab685d92517ccf8cc4766
> 
> I think it _should_ work now. Thanks for the suggestions. I just
> monkey patched the signatures of each of those functions. Ugly, but it
> works.

oh boy, can't disagree with ugly ;). it needed a few tweaks:

--- WireGuard-0.0.20171111.orig/src/compat/compat.h       2017-11-11 04:35:06.000000000 +0100
+++ WireGuard-0.0.20171111/src/compat/compat.h    2017-11-13 23:21:17.967716768 +0100
@@ -562,4 +562,23 @@ static inline void new_icmpv6_send(struc
 #define __read_mostly
 #endif

+#ifdef RAP_PLUGIN
+#include <linux/timer.h>
+#ifndef TIMER_DATA_TYPE
+#define TIMER_DATA_TYPE unsigned long
+#endif
+
+#define expired_retransmit_handshake(a) expired_retransmit_handshake(TIMER_DATA_TYPE timer)
+#define expired_send_keepalive(a) expired_send_keepalive(TIMER_DATA_TYPE timer)
+#define expired_new_handshake(a) expired_new_handshake(TIMER_DATA_TYPE timer)
+#define expired_zero_key_material(a) expired_zero_key_material(TIMER_DATA_TYPE timer)
+#define expired_send_persistent_keepalive(a) expired_send_persistent_keepalive(TIMER_DATA_TYPE timer)
+
+#undef timer_setup
+#define timer_setup(a, b, c) setup_timer(a, ((void (*)(TIMER_DATA_TYPE))b), ((TIMER_DATA_TYPE)a))
+
+#undef from_timer
+#define from_timer(var, callback_timer, timer_fieldname) container_of((struct timer_list *)callback_timer, typeof(*var), timer_fieldname)
+#endif
+
 #endif /* _WG_COMPAT_H */

the KERNEXEC block isn't needed as it was removing KERNEXEC's own define and
pre 4.14 kernels don't define TIMER_DATA_TYPE so it has to be defined for them.
also this whole block should probably depend on 4.15 if it ends up converting
all old prototypes and removes TIMER_DATA_TYPE itself.

> By the way, if you ever find yourself having to revert things to run
> WireGuard, don't hesitate to send a patch or just poke me, and I'll
> fix things. I'd definitely like to support PaX for as long as I can
> manage to do so.

thanks, will keep you posted if i see anything.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  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
  0 siblings, 2 replies; 14+ messages in thread
From: Jason A. Donenfeld @ 2017-11-14  9:29 UTC (permalink / raw)
  To: pageexec; +Cc: WireGuard mailing list

On Tue, Nov 14, 2017 at 1:15 AM, PaX Team <pageexec@freemail.hu> wrote:
> oh boy, can't disagree with ugly ;)

The goal is the highest possible density of filth.

>
> --- WireGuard-0.0.20171111.orig/src/compat/compat.h       2017-11-11 04:35:06.000000000 +0100
> +++ WireGuard-0.0.20171111/src/compat/compat.h    2017-11-13 23:21:17.967716768 +0100

I fixed things up here:
https://git.zx2c4.com/WireGuard/commit/?id=df318d1f0526663a2d92439376379e32ebcfef1a

> the KERNEXEC block isn't needed as it was removing KERNEXEC's own define

Wait, but earlier you wrote:

> speaking of PaX support, you recently added some __ro_after_init wrapper
> to wireguard which breaks under KERNEXEC when it's used on ops structs
> (my __read_only has different semantics) so i have to revert it here but
> it'd be nicer if you didn't define it when KERNEXEC is active.

So what exactly should I be fixing? I think in that last patch I
forgot to redefine it to be empty. Would this do what you have in
mind:

#ifdef CONFIG_PAX_KERNEXEC
#include <linux/cache.h>
#undef __ro_after_init
#define __ro_after_init
#endif

Or is there something else?

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-14  9:29         ` Jason A. Donenfeld
@ 2017-11-14 10:29           ` Jason A. Donenfeld
  2017-11-14 11:12           ` PaX Team
  1 sibling, 0 replies; 14+ messages in thread
From: Jason A. Donenfeld @ 2017-11-14 10:29 UTC (permalink / raw)
  To: pageexec; +Cc: WireGuard mailing list

On Tue, Nov 14, 2017 at 10:29 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Or is there something else?

With the latest git master (that commit in my last email), I just
tried compiling and running this with some sketchtastic 4.9 "forward
ports", and things seem to be working. If things are broken for you
with >4.9, happy to accept patches.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  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
  1 sibling, 1 reply; 14+ messages in thread
From: PaX Team @ 2017-11-14 11:12 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

On 14 Nov 2017 at 10:29, Jason A. Donenfeld wrote:

> I fixed things up here:
> https://git.zx2c4.com/WireGuard/commit/?id=df318d1f0526663a2d92439376379e32ebcfef1a

works fine, thanks.

> > speaking of PaX support, you recently added some __ro_after_init wrapper
> > to wireguard which breaks under KERNEXEC when it's used on ops structs
> > (my __read_only has different semantics) so i have to revert it here but
> > it'd be nicer if you didn't define it when KERNEXEC is active.
> 
> So what exactly should I be fixing? I think in that last patch I
> forgot to redefine it to be empty. Would this do what you have in
> mind:

i said 'don't define it', not 'redefine it' ;). the difference is that the
latter removes the definition provided under KERNEXEC.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-14 11:12           ` PaX Team
@ 2017-11-28 12:20             ` Jason A. Donenfeld
  2017-11-28 12:32               ` PaX Team
  0 siblings, 1 reply; 14+ messages in thread
From: Jason A. Donenfeld @ 2017-11-28 12:20 UTC (permalink / raw)
  To: pageexec; +Cc: WireGuard mailing list

Hey Pipacs,

A user on IRC found another issue. This time it looks like it might be
a RAP bug, though. Below is the error output. So far as I can see, the
function pointer type and the function declaration match fine.
However, the function itself is implemented in assembly, which means
it can't itself be instrumented by RAP. What to do?

https://git.zx2c4.com/WireGuard/tree/src/crypto/chacha20poly1305.c#n567

Jason

[31312.560886] wireguard: loading out-of-tree module taints kernel.
[31312.561438] wireguard: WireGuard 0.0.20171127 loaded. See
www.wireguard.com for information.
[31312.561439] wireguard: Copyright (C) 2015-2017 Jason A. Donenfeld
<Jason@zx2c4.com>. All Rights Reserved.
[31322.008508] PAX: RAP hash violation for function pointer:
poly1305_blocks_x86_64+0x0/0x100 [wireguard]
[31322.018515] PAX: overwritten function pointer detected: 0000 [#1] SMP
[31322.028152] Modules linked in: wireguard(O) ip6_udp_tunnel
udp_tunnel tun 8021q mrp ipt_MASQUERADE nf_nat_masquerade_ipv4 xt_nat
iptable_nat nf_nat_ipv4 ipt_REJECT nf_reject_ipv4 nf_log_ipv4
nf_conntrack_ipv4 nf_defrag_ipv4 xt_multiport iptable_filter
iptable_mangle ip_tables xt_TCPMSS xt_comment xt_tcpudp
ip6table_mangle ip6t_REJECT nf_reject_ipv6 nf_log_ipv6 nf_log_common
xt_LOG xt_limit xt_conntrack ip6table_filter ip6t_MASQUERADE
nf_nat_masquerade_ipv6 ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6
nf_nat_ipv6 nf_nat nf_conntrack ip6t_rpfilter ip6table_raw ip6_tables
x_tables ext4 crc16 jbd2 mbcache xfs libcrc32c exportfs af_packet ipv6
virtio_console psmouse serio_raw pcspkr igb ptp pps_core hwmon dca
i2c_algo_bit shpchp input_leds evdev joydev mousedev qemu_fw_cfg
floppy parport_pc parport tpm_tis
[31322.099742]  tpm_tis_core tpm dm_mod hid_generic usbhid hid uas
fbcon bitblit fbcon_rotate fbcon_ccw fbcon_ud fbcon_cw softcursor font
tileblit virtio_balloon button xhci_pci xhci_hcd uhci_hcd bochs_drm
ttm drm_kms_helper drm fb_sys_fops syscopyarea sysfillrect sysimgblt
ata_generic pata_acpi ata_piix libata virtio_pci virtio_ring virtio
i2c_piix4 i2c_core intel_agp intel_gtt agpgart loop crc32c_generic
btrfs xor raid6_pq usb_storage sd_mod scsi_mod
[31322.157368] CPU: 0 PID: 3344 Comm: kworker/0:2 Tainted: G
O    4.9.65-1-hardened #2-Alpine
[31322.173073] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
[31322.205545] Workqueue: wg-kex-lansecure
packet_handshake_receive_worker [wireguard]
[31322.222496] task: ffff8800b93f4bc0 task.stack: ffffc90003074000
[31322.239610] RIP: 0010:[<ffffffffa01ee17e>]  [<ffffffffa01ee17e>]
poly1305_update+0x18e/0x1b0 [wireguard]
[31322.257251] RSP: 0000:ffffc90003077998  EFLAGS: 00000216
[31322.274886] RAX: ffffffffa01f93c0 RBX: 0000000000000020 RCX: ffffffffa01ef7b3
[31322.292663] RDX: 0000000000000020 RSI: ffffc90003077ce7 RDI: ffffc90003077a70
[31322.310467] RBP: ffffc900030779d0 R08: ffffffffa01f93c0 R09: 0000000000000000
[31322.328276] R10: ffffc90003077b78 R11: 58e1b4d500000000 R12: ffffc90003077a70
[31322.346218] R13: 0000000000000000 R14: ffffc90003077ce7 R15: 0000000000000020
[31322.364271] FS:  0000000000000000(0000) GS:ffff8800bfa00000(0000)
knlGS:0000000000000000
[31322.382681] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[31322.401247] CR2: 0000000000000000 CR3: 00000000bae07000 CR4: 00000000000006b0
[31322.420111] Stack:
[31322.438912]  799bb7f3dc82ba87 5ecf70918324bcec ffffc90003077c87
ffffc90003077bb8
[31322.458357]  ffffc90003077a20 0000000000000001 0000000000000020
ffffc90003077c40
[31322.477912]  ffffffffa01ef7da b3326dec53956add 4bd3dddee06cadf2
0000000000000020
[31322.497565] Call Trace:
[31322.517082]  [<ffffffffa01ef7da>]
chacha20poly1305_decrypt+0x21a/0x8e0 [wireguard]
[31322.537124]  [<ffffffff8130ebc2>] ? memzero_explicit+0x1f/0x40
[31322.557308]  [<ffffffffa01f93c0>] ? poly1305_init_x86_64+0x40/0x40
[wireguard]
[31322.577686]  [<ffffffffa01f94c0>] ?
poly1305_blocks_x86_64+0x100/0x100 [wireguard]
[31322.598313]  [<ffffffffa01d73c6>]
noise_handshake_consume_initiation+0x286/0x6e0 [wireguard]
[31322.619328]  [<ffffffff8130ebc2>] ? memzero_explicit+0x1f/0x40
[31322.640488]  [<ffffffffa01e3259>] ? compute_mac1+0xc9/0x120 [wireguard]
[31322.661930]  [<ffffffffa01dcbdb>]
packet_handshake_receive_worker+0x2cb/0x430 [wireguard]
[31322.683748]  [<ffffffff8109b918>] process_one_work+0x260/0x40e
[31322.705478]  [<ffffffff8109ca87>] worker_thread+0x3f1/0x586
[31322.727402]  [<ffffffff8109c696>] ? rescuer_thread+0x443/0x443
[31322.749450]  [<ffffffff810a34e1>] kthread+0x15e/0x170
[31322.771493]  [<ffffffff810a3383>] ? kthread_park+0xa4/0xa4
[31322.793619]  [<ffffffff810a3383>] ? kthread_park+0xa4/0xa4
[31322.815512]  [<ffffffff8167051b>] ret_from_fork+0x5b/0x70
[31322.837338] Code: 8d bc 2f d0 00 00 00 4c 01 eb eb 0b b4 0c 76 cd
ff ff ff ff cc cc cc e8 41 62 12 e1 49 89 9c 24 e0 00 00 00 e9 4b ff
ff ff cd 82 <cd> 82 cd 83 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 cc
cc cc
[31322.881717] RIP  [<ffffffffa01ee17e>] poly1305_update+0x18e/0x1b0 [wireguard]
[31322.903292]  RSP <ffffc90003077998>
[31322.961174] ---[ end trace f0299ab1621f48bd ]---

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-28 12:20             ` Jason A. Donenfeld
@ 2017-11-28 12:32               ` PaX Team
  2017-11-28 12:36                 ` Jason A. Donenfeld
  0 siblings, 1 reply; 14+ messages in thread
From: PaX Team @ 2017-11-28 12:32 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

On 28 Nov 2017 at 13:20, Jason A. Donenfeld wrote:

> Hey Pipacs,
> 
> A user on IRC found another issue. This time it looks like it might be
> a RAP bug, though. Below is the error output. So far as I can see, the
> function pointer type and the function declaration match fine.
> However, the function itself is implemented in assembly, which means
> it can't itself be instrumented by RAP. What to do?

targets of indirect calls must be marked by the RAP hash which the plugin
will do for code it sees but for asm you'll have to do it yourself, look at
the use of RAP_ENTRY to see how that works.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-28 12:32               ` PaX Team
@ 2017-11-28 12:36                 ` Jason A. Donenfeld
  2017-11-28 12:50                   ` PaX Team
  0 siblings, 1 reply; 14+ messages in thread
From: Jason A. Donenfeld @ 2017-11-28 12:36 UTC (permalink / raw)
  To: pageexec; +Cc: WireGuard mailing list

On Tue, Nov 28, 2017 at 1:32 PM, PaX Team <pageexec@freemail.hu> wrote:
> targets of indirect calls must be marked by the RAP hash which the plugin
> will do for code it sees but for asm you'll have to do it yourself, look at
> the use of RAP_ENTRY to see how that works.

Oh, terrific. So I can just do something horrible like:

#ifdef RAP_PLUGIN
#undef ENTRY
#define ENTRY RAP_ENTRY
#endif

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: imer_setup() is not compatible with PaX's RAP
  2017-11-28 12:36                 ` Jason A. Donenfeld
@ 2017-11-28 12:50                   ` PaX Team
  0 siblings, 0 replies; 14+ messages in thread
From: PaX Team @ 2017-11-28 12:50 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

On 28 Nov 2017 at 13:36, Jason A. Donenfeld wrote:

> On Tue, Nov 28, 2017 at 1:32 PM, PaX Team <pageexec@freemail.hu> wrote:
> > targets of indirect calls must be marked by the RAP hash which the plugin
> > will do for code it sees but for asm you'll have to do it yourself, look at
> > the use of RAP_ENTRY to see how that works.
> 
> Oh, terrific. So I can just do something horrible like:
> 
> #ifdef RAP_PLUGIN
> #undef ENTRY
> #define ENTRY RAP_ENTRY
> #endif

well, that would work but if not all asm entry points are meant to be called
indirectly then you're unnecessarily increasing the attack surface ;). better
would be something like:

1. use ENTRY/RAP_ENTRY in your asm as necessary. you can call it something
   more generic like CFI_ENTRY if you want to cover other CFI systems in the
   future, e.g., intel's CET will need its own entry point marker insn.

2. have this in your headers:

#ifdef RAP_PLUGIN
#define CFI_ENTRY RAP_ENTRY
#elif defined(...)
...
#else
#define CFI_ENTRY ENTRY
#endif

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-11-28 12:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-11  8:09 imer_setup() is not compatible with PaX's RAP Tom Li
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.