Working on change for: genetlink: make policy common to family

Working on change for: genetlink: make policy common to family

[Bruno Wolff III]

Wireguard isn't building on 5.2 right now because of commit:
3b0f31f2b8c9fb348e4530b88f6b64f9621f83d6 genetlink: make policy common to family

I've got Wireguard building, but need to do basic testing, then add a kernel 
version test in and do some other testing. If that all goes OK I'll submit 
a signed off patch to the list.

It looks to be a very simple change to netlink.c, but I could have easily 
missed something subtle.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Bruno Wolff III]

On Mon, May 13, 2019 at 14:52:13 -0500,
  Bruno Wolff III <bruno@wolff.to> wrote:
>Wireguard isn't building on 5.2 right now because of commit:
>3b0f31f2b8c9fb348e4530b88f6b64f9621f83d6 genetlink: make policy common to family
>
>I've got Wireguard building, but need to do basic testing, then add a 
>kernel version test in and do some other testing. If that all goes OK 
>I'll submit a signed off patch to the list.
>
>It looks to be a very simple change to netlink.c, but I could have 
>easily missed something subtle.

wg (the config tool) doesn't work with my change, so there probably is more 
needed than just moving .policy to the family structure in the kernel. I'll 
continue looking at it, but it might need someone better than me to look at it 
eventually.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Bruno Wolff III]

On Mon, May 13, 2019 at 15:24:53 -0500,
  Bruno Wolff III <bruno@wolff.to> wrote:
>On Mon, May 13, 2019 at 14:52:13 -0500,
> Bruno Wolff III <bruno@wolff.to> wrote:
>>Wireguard isn't building on 5.2 right now because of commit:
>>3b0f31f2b8c9fb348e4530b88f6b64f9621f83d6 genetlink: make policy common to family
>>
>>I've got Wireguard building, but need to do basic testing, then add 
>>a kernel version test in and do some other testing. If that all goes 
>>OK I'll submit a signed off patch to the list.
>>
>>It looks to be a very simple change to netlink.c, but I could have 
>>easily missed something subtle.
>
>wg (the config tool) doesn't work with my change, so there probably is 
>more needed than just moving .policy to the family structure in the 
>kernel. I'll continue looking at it, but it might need someone better 
>than me to look at it eventually.

There is also a small chance that there are multiple issues. I may need 
to test before and after the commit I identified to see if this is the 
case.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Bruno Wolff III]

On Mon, May 13, 2019 at 16:21:10 -0500,
  Bruno Wolff III <bruno@wolff.to> wrote:
>On Mon, May 13, 2019 at 15:24:53 -0500,
> Bruno Wolff III <bruno@wolff.to> wrote:
>>On Mon, May 13, 2019 at 14:52:13 -0500,
>>Bruno Wolff III <bruno@wolff.to> wrote:
>>>Wireguard isn't building on 5.2 right now because of commit:
>>>3b0f31f2b8c9fb348e4530b88f6b64f9621f83d6 genetlink: make policy common to family
>>>

I'm slowly trying to work on this, but if someone who knows what they are 
doing wants to just get it done, feel free.

The commit that breaks things was developed on an old enough kernel that 
it was a problem with the compat stuff being out of sync. I'm trying various 
merge points to try to find a good place to test if there is a separate 
problem or if my change for that one is bad. The builds take a while so 
this hasn't been happening quickly.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Bruno Wolff III]

I think 8cb081746c031fb164089322e2336a0bf5b3070c netlink: make validation 
more configurable for future strictness, might be the other commit causing 
problems. Some nla functions have changed. It looks like renamed, 
deprecated versions of the functions will exist for a while. So it should 
be easy for me to test this today. In the long using the deprecared 
functions will not be desired.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Bruno Wolff III]

On Wed, May 15, 2019 at 05:50:14 -0500,
  Bruno Wolff III <bruno@wolff.to> wrote:
>I think 8cb081746c031fb164089322e2336a0bf5b3070c netlink: make 
>validation more configurable for future strictness, might be the other 
>commit causing problems. Some nla functions have changed. It looks 
>like renamed, deprecated versions of the functions will exist for a 
>while. So it should be easy for me to test this today. In the long 
>using the deprecared functions will not be desired.

Wireguard built with the deprecated versions of nlmsg_parse and 
nla_parse_nested (and .policy moved to genl_family), but I'm still getting:
Unable to modify interface: Invalid argument
When running:
wg setconf wg0 /etc/wireguard/config

So I still don't know if I'm doing something wrong or missing yet another 
change.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Bruno Wolff III]

Now I'm looking at: f6ad55a6a184ebdf3d98a90eab0895f73ce9797e Merge branch 
'nla_nest_start', which looks like it might also cause a problem.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Bruno Wolff III]

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]

On Wed, May 15, 2019 at 06:18:30 -0500,
  Bruno Wolff III <bruno@wolff.to> wrote:
>Now I'm looking at: f6ad55a6a184ebdf3d98a90eab0895f73ce9797e Merge 
>branch 'nla_nest_start', which looks like it might also cause a 
>problem.

Changing nla_nest_start to nla_nest_start_noflag didn't seem to help.

In case anyone else is working on getting wireguard to work with 5.2, 
I'm attaching my latest test diff.

[-- Attachment #2: test.diff --]
[-- Type: text/plain, Size: 3014 bytes --]

diff --git a/src/netlink.c b/src/netlink.c
index b179b3184725..dd46487e0888 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -74,7 +74,7 @@ static int get_allowedips(struct sk_buff *skb, const u8 *ip, u8 cidr,
 {
 	struct nlattr *allowedip_nest;
 
-	allowedip_nest = nla_nest_start(skb, 0);
+	allowedip_nest = nla_nest_start_noflag(skb, 0);
 	if (!allowedip_nest)
 		return -EMSGSIZE;
 
@@ -94,7 +94,7 @@ static int
 get_peer(struct wg_peer *peer, struct allowedips_node **next_allowedips_node,
 	 u64 *allowedips_seq, struct sk_buff *skb)
 {
-	struct nlattr *allowedips_nest, *peer_nest = nla_nest_start(skb, 0);
+	struct nlattr *allowedips_nest, *peer_nest = nla_nest_start_noflag(skb, 0);
 	struct allowedips_node *allowedips_node = *next_allowedips_node;
 	bool fail;
 
@@ -156,7 +156,7 @@ get_peer(struct wg_peer *peer, struct allowedips_node **next_allowedips_node,
 	else if (*allowedips_seq != peer->device->peer_allowedips.seq)
 		goto no_allowedips;
 
-	allowedips_nest = nla_nest_start(skb, WGPEER_A_ALLOWEDIPS);
+	allowedips_nest = nla_nest_start_noflag(skb, WGPEER_A_ALLOWEDIPS);
 	if (!allowedips_nest)
 		goto err;
 
@@ -190,7 +190,7 @@ static int wg_get_device_start(struct netlink_callback *cb)
 	struct wg_device *wg;
 	int ret;
 
-	ret = nlmsg_parse(cb->nlh, GENL_HDRLEN + genl_family.hdrsize, attrs,
+	ret = nlmsg_parse_deprecated(cb->nlh, GENL_HDRLEN + genl_family.hdrsize, attrs,
 			  genl_family.maxattr, device_policy, NULL);
 	if (ret < 0)
 		return ret;
@@ -247,7 +247,7 @@ static int wg_get_device_dump(struct sk_buff *skb, struct netlink_callback *cb)
 		up_read(&wg->static_identity.lock);
 	}
 
-	peers_nest = nla_nest_start(skb, WGDEVICE_A_PEERS);
+	peers_nest = nla_nest_start_noflag(skb, WGDEVICE_A_PEERS);
 	if (!peers_nest)
 		goto out;
 	ret = 0;
@@ -450,7 +450,7 @@ static int set_peer(struct wg_device *wg, struct nlattr **attrs)
 		int rem;
 
 		nla_for_each_nested(attr, attrs[WGPEER_A_ALLOWEDIPS], rem) {
-			ret = nla_parse_nested(allowedip, WGALLOWEDIP_A_MAX,
+			ret = nla_parse_nested_deprecated(allowedip, WGALLOWEDIP_A_MAX,
 					       attr, allowedip_policy, NULL);
 			if (ret < 0)
 				goto out;
@@ -561,7 +561,7 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 		int rem;
 
 		nla_for_each_nested(attr, info->attrs[WGDEVICE_A_PEERS], rem) {
-			ret = nla_parse_nested(peer, WGPEER_A_MAX, attr,
+			ret = nla_parse_nested_deprecated(peer, WGPEER_A_MAX, attr,
 					       peer_policy, NULL);
 			if (ret < 0)
 				goto out;
@@ -596,12 +596,10 @@ struct genl_ops genl_ops[] = {
 #endif
 		.dumpit = wg_get_device_dump,
 		.done = wg_get_device_done,
-		.policy = device_policy,
 		.flags = GENL_UNS_ADMIN_PERM
 	}, {
 		.cmd = WG_CMD_SET_DEVICE,
 		.doit = wg_set_device,
-		.policy = device_policy,
 		.flags = GENL_UNS_ADMIN_PERM
 	}
 };
@@ -617,6 +615,7 @@ __ro_after_init = {
 	.name = WG_GENL_NAME,
 	.version = WG_GENL_VERSION,
 	.maxattr = WGDEVICE_A_MAX,
+	.policy = device_policy,
 	.module = THIS_MODULE,
 	.netnsok = true
 };

[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Jason A. Donenfeld]

Thanks for getting this started. This commit should take care of it:

https://git.zx2c4.com/WireGuard/commit/?id=7a83d1e6da8aa27da8fd4d06e6b7d11198c7c049
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Bruno Wolff III]

On Fri, May 17, 2019 at 13:12:07 +0200,
  "Jason A. Donenfeld" <Jason@zx2c4.com> wrote:
>Thanks for getting this started. This commit should take care of it:
>
>https://git.zx2c4.com/WireGuard/commit/?id=7a83d1e6da8aa27da8fd4d06e6b7d11198c7c049

Thanks for the fix. I'm using it with Fedora's 5.2.0-0.rc0.git8.1.fc31.x86_64 
kernel successfully. (Note not all arches built successfully with that 
kernel, but it was the latest for x86_64 I could get right now for testing 
a 5.2 kernel with the new fix.)
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Working on change for: genetlink: make policy common to family

[Robin Kauffman]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 2943 bytes --]

Hi-	I'm having difficulty building the current WireGuard Git master
(c563bb275181a753853447c9c9c3a6767a6a1554) against a slightly older
checkout of Linus' Git master
(72cf0b07418a9c8349aa9137194b1ccba6e54a9d).  The error I get is: 
CC      net/wireguard/main.oIn file included from <command-
line>:././net/wireguard/compat/compat.h:828:21: error: redeclaration of
enumerator ‘NLA_UNSPEC’ #define NLA_MIN_LEN
NLA_UNSPEC                     ^~~~~~~~~~./include/net/netlink.h:186:2:
note: in expansion of macro ‘NLA_MIN_LEN’  NLA_MIN_LEN,  ^~~~~~~~~~~In
file included from ./include/net/rtnetlink.h:6,                 from
./include/net/sch_generic.h:17,                 from
./include/linux/filter.h:25,                 from
./include/net/sock.h:64,                 from
./include/linux/tcp.h:23,                 from
./include/linux/ipv6.h:87,                 from
./include/net/ipv6.h:16,                 from
././net/wireguard/compat/compat.h:843,                 from <command-
line>:./include/net/netlink.h:166:2: note: previous definition of
‘NLA_UNSPEC’ was here  NLA_UNSPEC, 
^~~~~~~~~~scripts/Makefile.build:278: recipe for target
'net/wireguard/main.o' failedmake[2]: *** [net/wireguard/main.o] Error
1scripts/Makefile.build:489: recipe for target 'net/wireguard'
failedmake[1]: *** [net/wireguard] Error 2Makefile:1073: recipe for
target 'net' failedmake: *** [net] Error 2
	The full build log (GNU LZMA-compressed) is attached.
		-Robin K.
On Fri, 2019-05-17 at 08:36 -0500, Bruno Wolff III wrote:
> On Fri, May 17, 2019 at 13:12:07 +0200,  "Jason A. Donenfeld" <
> Jason@zx2c4.com> wrote:
> > Thanks for getting this started. This commit should take care of
> > it:
> > https://u9444849.ct.sendgrid.net/wf/click?upn=hlnDB5p2SCqcHpy-2Bu-2FDu8d88SXVo27bsPhiBQuz8ptM0jOis4sOiwp3Pc-2BAwmdItagQXBksDjcwFAI58qh6deyT-2Bny-2FQBJ5h4fuS0zgp1fObf8vTKOP9MwZ48K84-2Fa8V_HW4IRDX4SDmDQg1X-2FvoSrLWdDgphijQ89nXdFgsrfVzCVKmUOYslTHsHH18vguEGf3S5bA73VraTmcttvDzqk-2Bh2XO-2F8tYKS7WXf-2BeawAdESewRgDJuTQCwmE51AWByFqTKQBNCrA27VZdFyZvf-2FQUnGvUKmPwIIb-2FFgG4e-2BOcg59gDnq-2FDtrSYxGBFMZtf2iz68jHSEDM3EfbdiZtJby2ZOFOjKiCh3Mwg6jm1hgu0-3D
> 
> Thanks for the fix. I'm using it with Fedora's 5.2.0-
> 0.rc0.git8.1.fc31.x86_64 kernel successfully. (Note not all arches
> built successfully with that kernel, but it was the latest for x86_64
> I could get right now for testing a 5.2 kernel with the new
> fix.)_______________________________________________WireGuard mailing
> listWireGuard@lists.zx2c4.com
> https://u9444849.ct.sendgrid.net/wf/click?upn=hlnDB5p2SCqcHpy-2Bu-2FDu8cojZAgJwd1eyzYQhFJNTVr6b-2FwB3KjUFMppgiJU-2B1EKJ2WXDtyNFABUqm2tNEA2bg-3D-3D_HW4IRDX4SDmDQg1X-2FvoSrLWdDgphijQ89nXdFgsrfVzCVKmUOYslTHsHH18vguEGf3S5bA73VraTmcttvDzqk-2Fw8mTLMCgr2KW4laq8Os42fkcjf7GhleHzKrVPUA00meG0-2FDCTfnQ22mZj6uTQ-2BTLoa9RAZ-2FyHxvEeEG27asIVxIcaXqHM8QA5MfIT8Ox-2FTn8YS-2BWsMGh5vfyM7VNmYLDDumh8Eptc8Ydloro7OSlU-3D

[-- Attachment #1.1.1.2: Type: text/html, Size: 6709 bytes --]

[-- Attachment #1.1.2: kernel_build_log_2019051801.txt.xz --]
[-- Type: application/x-xz, Size: 33884 bytes --]

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 148 bytes --]

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard