Development discussion of WireGuard
 help / color / mirror / Atom feed
* [PATCH] Fixed null pointer exception when user namespace is empty
@ 2021-10-16 20:59 Aaron Avery
  2021-10-17  0:52 ` Jason A. Donenfeld
  0 siblings, 1 reply; 4+ messages in thread
From: Aaron Avery @ 2021-10-16 20:59 UTC (permalink / raw)
  To: wireguard; +Cc: Aaron Avery

---
I compiled the Wireguard kernel module for my QNAP NAS running
version 4.14.24. When creating the network device, it got a null pointer
exception. I figured out that the user namespace is null on this system
and was being passed into ns_capable as-is, crashing the kernel (somewhat).
After applying this change, I finally have Wireguard up and running
after years of wishing I had it available instead of OpenVPN.

I'm not a Linux expert so if there's a better way to handle this
situation (such as checking for root instead of CAP_NET_ADMIN when
user_ns doesn't exist), let me know and I can try it and submit
a different patch.
Otherwise, it seems like this could be applied to both
wireguard-linux-compat and wireguard-linux for maximum system
compatibility going forward.

 src/netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/netlink.c b/src/netlink.c
index ef239ab..688e41f 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -513,7 +513,7 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 		struct net *net;
 		rcu_read_lock();
 		net = rcu_dereference(wg->creating_net);
-		ret = !net || !ns_capable(net->user_ns, CAP_NET_ADMIN) ? -EPERM : 0;
+		ret = !net || (net->user_ns && !ns_capable(net->user_ns, CAP_NET_ADMIN)) ? -EPERM : 0;
 		rcu_read_unlock();
 		if (ret)
 			goto out;
-- 
2.33.0


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

* Re: [PATCH] Fixed null pointer exception when user namespace is empty
  2021-10-16 20:59 [PATCH] Fixed null pointer exception when user namespace is empty Aaron Avery
@ 2021-10-17  0:52 ` Jason A. Donenfeld
  2021-10-17 20:27   ` Aaron Avery
  0 siblings, 1 reply; 4+ messages in thread
From: Jason A. Donenfeld @ 2021-10-17  0:52 UTC (permalink / raw)
  To: Aaron Avery; +Cc: WireGuard mailing list

Hi Aaron,

That patch is missing your Signed-off-by line, so I won't be able to
take it as-is.

However, I also wonder if it makes sense. I just grepped the entire
kernel and I couldn't find any other instances of net->user_ns being
NULL checked. Is it possible that there's a bug in QNAP's kernel
somewhere? Or you're compiling against the wrong .config so the struct
offsets are wrong? Or something else? When should net->user_ns be
NULL?

Thanks,
Jason

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

* Re: [PATCH] Fixed null pointer exception when user namespace is empty
  2021-10-17  0:52 ` Jason A. Donenfeld
@ 2021-10-17 20:27   ` Aaron Avery
  2021-10-18  1:04     ` David Kerr
  0 siblings, 1 reply; 4+ messages in thread
From: Aaron Avery @ 2021-10-17 20:27 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

Thank you, Jason. Sorry I missed the Signed-off-by. As I said, I'm not that familiar with Linux development.
I'm quite sure I have the correct .config for my QNAP's kernel, and the WG kernel module I built works perfectly, with this patch. All of the *_NS entries in their config file are enabled.

After more research, I agree with you completely that this could easily be a QNAP-specific bug so shouldn't be merged. As a NAS, they probably have some custom network driver code and may have failed to initialize user_ns in this situation. The only somewhat-relevant kernel commit I could find for rtnetlink.c is https://github.com/torvalds/linux/commit/f428fe4a04cc339166c8bbd489789760de3a0cee and version 4.14.24 is more recent than that.

A lot of people like me use a QNAP NAS as their always-on "home server". Hopefully Google will crawl this message list so that tech-savvy people searching for Wireguard QNAP will find this patch and have a chance to use WG to remotely access their home network.

- Aaron

From: Jason A. Donenfeld <Jason@zx2c4.com>
Sent: Saturday, October 16, 2021 7:52 PM
To: Aaron Avery <aavery77@hotmail.com>
Cc: WireGuard mailing list <wireguard@lists.zx2c4.com>
Subject: Re: [PATCH] Fixed null pointer exception when user namespace is empty 
 
Hi Aaron,

That patch is missing your Signed-off-by line, so I won't be able to
take it as-is.

However, I also wonder if it makes sense. I just grepped the entire
kernel and I couldn't find any other instances of net->user_ns being
NULL checked. Is it possible that there's a bug in QNAP's kernel
somewhere? Or you're compiling against the wrong .config so the struct
offsets are wrong? Or something else? When should net->user_ns be
NULL?

Thanks,
Jason

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

* Re: [PATCH] Fixed null pointer exception when user namespace is empty
  2021-10-17 20:27   ` Aaron Avery
@ 2021-10-18  1:04     ` David Kerr
  0 siblings, 0 replies; 4+ messages in thread
From: David Kerr @ 2021-10-18  1:04 UTC (permalink / raw)
  To: wireguard

My QNAP system just recently did a system update to QTS version 5
which now includes Wireguard as part of their QVPN service.  I have
not tried it, but it is there along with the usual suspects of
OpenVPN, IPSec, PPTP, etc.

DAK.


On Sun, Oct 17, 2021 at 4:30 PM Aaron Avery <aavery77@hotmail.com> wrote:
>
> Thank you, Jason. Sorry I missed the Signed-off-by. As I said, I'm not that familiar with Linux development.
> I'm quite sure I have the correct .config for my QNAP's kernel, and the WG kernel module I built works perfectly, with this patch. All of the *_NS entries in their config file are enabled.
>
> After more research, I agree with you completely that this could easily be a QNAP-specific bug so shouldn't be merged. As a NAS, they probably have some custom network driver code and may have failed to initialize user_ns in this situation. The only somewhat-relevant kernel commit I could find for rtnetlink.c is https://github.com/torvalds/linux/commit/f428fe4a04cc339166c8bbd489789760de3a0cee and version 4.14.24 is more recent than that.
>
> A lot of people like me use a QNAP NAS as their always-on "home server". Hopefully Google will crawl this message list so that tech-savvy people searching for Wireguard QNAP will find this patch and have a chance to use WG to remotely access their home network.
>
> - Aaron
>
> From: Jason A. Donenfeld <Jason@zx2c4.com>
> Sent: Saturday, October 16, 2021 7:52 PM
> To: Aaron Avery <aavery77@hotmail.com>
> Cc: WireGuard mailing list <wireguard@lists.zx2c4.com>
> Subject: Re: [PATCH] Fixed null pointer exception when user namespace is empty
>
> Hi Aaron,
>
> That patch is missing your Signed-off-by line, so I won't be able to
> take it as-is.
>
> However, I also wonder if it makes sense. I just grepped the entire
> kernel and I couldn't find any other instances of net->user_ns being
> NULL checked. Is it possible that there's a bug in QNAP's kernel
> somewhere? Or you're compiling against the wrong .config so the struct
> offsets are wrong? Or something else? When should net->user_ns be
> NULL?
>
> Thanks,
> Jason

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

end of thread, other threads:[~2021-10-18  1:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-16 20:59 [PATCH] Fixed null pointer exception when user namespace is empty Aaron Avery
2021-10-17  0:52 ` Jason A. Donenfeld
2021-10-17 20:27   ` Aaron Avery
2021-10-18  1:04     ` David Kerr

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).