tipc-discussion — User questions & problem reports, project news, developer discussion

Re: [tipc-discussion] [PATCH] tipc: fix use-after-free in tipc_bcast_get_mode

[Jon M. <jm...@re...>]

On 8/25/20 11:52 PM, Hoang Huu Le wrote:
> Syzbot has reported those issues as:
>
> ==================================================================
> BUG: KASAN: use-after-free in tipc_bcast_get_mode+0x3ab/0x400 net/tipc/bcast.c:759
> Read of size 1 at addr ffff88805e6b3571 by task kworker/0:6/3850
>
> CPU: 0 PID: 3850 Comm: kworker/0:6 Not tainted 5.8.0-rc7-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Workqueue: events tipc_net_finalize_work
>
> Thread 1's call trace:
> [...]
>    kfree+0x103/0x2c0 mm/slab.c:3757 <- bcbase releasing
>    tipc_bcast_stop+0x1b0/0x2f0 net/tipc/bcast.c:721
>    tipc_exit_net+0x24/0x270 net/tipc/core.c:112
> [...]
>
> Thread 2's call trace:
> [...]
>    tipc_bcast_get_mode+0x3ab/0x400 net/tipc/bcast.c:759 <- bcbase
> has already been freed by Thread 1
>
>    tipc_node_broadcast+0x9e/0xcc0 net/tipc/node.c:1744
>    tipc_nametbl_publish+0x60b/0x970 net/tipc/name_table.c:752
>    tipc_net_finalize net/tipc/net.c:141 [inline]
>    tipc_net_finalize+0x1fa/0x310 net/tipc/net.c:131
>    tipc_net_finalize_work+0x55/0x80 net/tipc/net.c:150
> [...]
>
> ==================================================================
> BUG: KASAN: use-after-free in tipc_named_reinit+0xef/0x290 net/tipc/name_distr.c:344
> Read of size 8 at addr ffff888052ab2000 by task kworker/0:13/30628
> CPU: 0 PID: 30628 Comm: kworker/0:13 Not tainted 5.8.0-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Workqueue: events tipc_net_finalize_work
> Call Trace:
>   __dump_stack lib/dump_stack.c:77 [inline]
>   dump_stack+0x1f0/0x31e lib/dump_stack.c:118
>   print_address_description+0x66/0x5a0 mm/kasan/report.c:383
>   __kasan_report mm/kasan/report.c:513 [inline]
>   kasan_report+0x132/0x1d0 mm/kasan/report.c:530
>   tipc_named_reinit+0xef/0x290 net/tipc/name_distr.c:344
>   tipc_net_finalize+0x85/0xe0 net/tipc/net.c:138
>   tipc_net_finalize_work+0x50/0x70 net/tipc/net.c:150
>   process_one_work+0x789/0xfc0 kernel/workqueue.c:2269
>   worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
>   kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
>   ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
> [...]
> Freed by task 14058:
>   save_stack mm/kasan/common.c:48 [inline]
>   set_track mm/kasan/common.c:56 [inline]
>   kasan_set_free_info mm/kasan/common.c:316 [inline]
>   __kasan_slab_free+0x114/0x170 mm/kasan/common.c:455
>   __cache_free mm/slab.c:3426 [inline]
>   kfree+0x10a/0x220 mm/slab.c:3757
>   tipc_exit_net+0x29/0x50 net/tipc/core.c:113
>   ops_exit_list net/core/net_namespace.c:186 [inline]
>   cleanup_net+0x708/0xba0 net/core/net_namespace.c:603
>   process_one_work+0x789/0xfc0 kernel/workqueue.c:2269
>   worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
>   kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
>   ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
>
> Solution 1 (preferer):
> Fix it by calling flush_scheduled_work() to make sure the
> tipc_net_finalize_work() stopped before releasing bcbase object.
>
> Solution 2:
> Fix it by introducing a bit flag and returning if flag is zero
> (object had already been freed)
>
> Reported-by: syz...@sy...
> Reported-by: syz...@sy...
> Signed-off-by: Hoang Huu Le <hoa...@de...>
> ---
>   net/tipc/core.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/net/tipc/core.c b/net/tipc/core.c
> index 4f6dc74adf45..37d8695548cf 100644
> --- a/net/tipc/core.c
> +++ b/net/tipc/core.c
> @@ -109,6 +109,11 @@ static void __net_exit tipc_exit_net(struct net *net)
>   {
>   	tipc_detach_loopback(net);
>   	tipc_net_stop(net);
> +
> +	/* Make sure the tipc_net_finalize_work stopped
> +	 * before releasing the resources.
> +	 */
> +	flush_scheduled_work();
>   	tipc_bcast_stop(net);
>   	tipc_nametbl_stop(net);
>   	tipc_sk_rht_destroy(net);
Acked-by: Jon Maloy <jm...@re...>

Re: [tipc-discussion] [iproute2-next v2] tipc: support 128bit node identity for peer removing

[Jon M. <jm...@re...>]

On 8/24/20 6:00 AM, Hoang Huu Le wrote:
> From: Hoang Le <hoa...@de...>
>
> Problem:
> In kernel upstream, we add the support to set node identity with
> 128bit. However, we are still using legacy format in command tipc
> peer removing. Then, we got a problem when trying to remove
> offline node i.e:
>
> $ tipc node list
> Node Identity                    Hash     State
> d6babc1c1c6d                     1cbcd7ca down
>
> $ tipc peer remove address d6babc1c1c6d
> invalid network address, syntax: Z.C.N
> error: No such device or address
>
> Solution:
> We add the support to remove a specific node down with 128bit
> node identifier, as an alternative to legacy 32-bit node address.
>
> Signed-off-by: Hoang Le <hoa...@de...>
> Signed-off-by: Hoang Huu Le <hoa...@de...>
> ---
>   tipc/peer.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/tipc/peer.c b/tipc/peer.c
> index f6380777033d..f14ec35e6f71 100644
> --- a/tipc/peer.c
> +++ b/tipc/peer.c
> @@ -59,17 +59,68 @@ static int cmd_peer_rm_addr(struct nlmsghdr *nlh, const struct cmd *cmd,
>   	return msg_doit(nlh, NULL, NULL);
>   }
>   
> +static int cmd_peer_rm_nodeid(struct nlmsghdr *nlh, const struct cmd *cmd,
> +			      struct cmdl *cmdl, void *data)
> +{
> +	char buf[MNL_SOCKET_BUFFER_SIZE];
> +	__u8 id[16] = {0,};
> +	__u64 *w0 = (__u64 *)&id[0];
> +	__u64 *w1 = (__u64 *)&id[8];
> +	struct nlattr *nest;
> +	char *str;
> +
> +	if (cmdl->argc != cmdl->optind + 1) {
> +		fprintf(stderr, "Usage: %s peer remove identity NODEID\n",
> +			cmdl->argv[0]);
> +		return -EINVAL;
> +	}
> +
> +	str = shift_cmdl(cmdl);
> +	if (str2nodeid(str, id)) {
> +		fprintf(stderr, "Invalid node identity\n");
> +		return -EINVAL;
> +	}
> +
> +	nlh = msg_init(buf, TIPC_NL_PEER_REMOVE);
> +	if (!nlh) {
> +		fprintf(stderr, "error, message initialisation failed\n");
> +		return -1;
> +	}
> +
> +	nest = mnl_attr_nest_start(nlh, TIPC_NLA_NET);
> +	mnl_attr_put_u64(nlh, TIPC_NLA_NET_NODEID, *w0);
> +	mnl_attr_put_u64(nlh, TIPC_NLA_NET_NODEID_W1, *w1);
> +	mnl_attr_nest_end(nlh, nest);
> +
> +	return msg_doit(nlh, NULL, NULL);
> +}
> +
>   static void cmd_peer_rm_help(struct cmdl *cmdl)
> +{
> +	fprintf(stderr, "Usage: %s peer remove PROPERTY\n\n"
> +		"PROPERTIES\n"
> +		" identity NODEID         - Remove peer node identity\n",
> +		cmdl->argv[0]);
> +}
> +
> +static void cmd_peer_rm_addr_help(struct cmdl *cmdl)
>   {
>   	fprintf(stderr, "Usage: %s peer remove address ADDRESS\n",
>   		cmdl->argv[0]);
>   }
>   
> +static void cmd_peer_rm_nodeid_help(struct cmdl *cmdl)
> +{
> +	fprintf(stderr, "Usage: %s peer remove identity NODEID\n",
> +		cmdl->argv[0]);
> +}
> +
>   static int cmd_peer_rm(struct nlmsghdr *nlh, const struct cmd *cmd,
>   			struct cmdl *cmdl, void *data)
>   {
>   	const struct cmd cmds[] = {
> -		{ "address",	cmd_peer_rm_addr,	cmd_peer_rm_help },
> +		{ "address",  cmd_peer_rm_addr,   cmd_peer_rm_addr_help },
> +		{ "identity", cmd_peer_rm_nodeid, cmd_peer_rm_nodeid_help },
>   		{ NULL }
>   	};
>   
Acked-by: Jon Maloy <jm...@re...>

[tipc-discussion] [PATCH] tipc: fix use-after-free in tipc_bcast_get_mode

[Hoang H. Le <hoa...@de...>]

Syzbot has reported those issues as:

==================================================================
BUG: KASAN: use-after-free in tipc_bcast_get_mode+0x3ab/0x400 net/tipc/bcast.c:759
Read of size 1 at addr ffff88805e6b3571 by task kworker/0:6/3850

CPU: 0 PID: 3850 Comm: kworker/0:6 Not tainted 5.8.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events tipc_net_finalize_work

Thread 1's call trace:
[...]
  kfree+0x103/0x2c0 mm/slab.c:3757 <- bcbase releasing
  tipc_bcast_stop+0x1b0/0x2f0 net/tipc/bcast.c:721
  tipc_exit_net+0x24/0x270 net/tipc/core.c:112
[...]

Thread 2's call trace:
[...]
  tipc_bcast_get_mode+0x3ab/0x400 net/tipc/bcast.c:759 <- bcbase
has already been freed by Thread 1

  tipc_node_broadcast+0x9e/0xcc0 net/tipc/node.c:1744
  tipc_nametbl_publish+0x60b/0x970 net/tipc/name_table.c:752
  tipc_net_finalize net/tipc/net.c:141 [inline]
  tipc_net_finalize+0x1fa/0x310 net/tipc/net.c:131
  tipc_net_finalize_work+0x55/0x80 net/tipc/net.c:150
[...]

==================================================================
BUG: KASAN: use-after-free in tipc_named_reinit+0xef/0x290 net/tipc/name_distr.c:344
Read of size 8 at addr ffff888052ab2000 by task kworker/0:13/30628
CPU: 0 PID: 30628 Comm: kworker/0:13 Not tainted 5.8.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events tipc_net_finalize_work
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x1f0/0x31e lib/dump_stack.c:118
 print_address_description+0x66/0x5a0 mm/kasan/report.c:383
 __kasan_report mm/kasan/report.c:513 [inline]
 kasan_report+0x132/0x1d0 mm/kasan/report.c:530
 tipc_named_reinit+0xef/0x290 net/tipc/name_distr.c:344
 tipc_net_finalize+0x85/0xe0 net/tipc/net.c:138
 tipc_net_finalize_work+0x50/0x70 net/tipc/net.c:150
 process_one_work+0x789/0xfc0 kernel/workqueue.c:2269
 worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
 kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
[...]
Freed by task 14058:
 save_stack mm/kasan/common.c:48 [inline]
 set_track mm/kasan/common.c:56 [inline]
 kasan_set_free_info mm/kasan/common.c:316 [inline]
 __kasan_slab_free+0x114/0x170 mm/kasan/common.c:455
 __cache_free mm/slab.c:3426 [inline]
 kfree+0x10a/0x220 mm/slab.c:3757
 tipc_exit_net+0x29/0x50 net/tipc/core.c:113
 ops_exit_list net/core/net_namespace.c:186 [inline]
 cleanup_net+0x708/0xba0 net/core/net_namespace.c:603
 process_one_work+0x789/0xfc0 kernel/workqueue.c:2269
 worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
 kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293

Solution 1 (preferer):
Fix it by calling flush_scheduled_work() to make sure the
tipc_net_finalize_work() stopped before releasing bcbase object.

Solution 2:
Fix it by introducing a bit flag and returning if flag is zero
(object had already been freed)

Reported-by: syz...@sy...
Reported-by: syz...@sy...
Signed-off-by: Hoang Huu Le <hoa...@de...>
---
 net/tipc/bcast.c | 1 +
 net/tipc/core.c  | 1 +
 net/tipc/core.h  | 1 +
 net/tipc/net.c   | 3 +++
 4 files changed, 6 insertions(+)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 940d176e0e87..56b624c8b6d4 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -718,6 +718,7 @@ void tipc_bcast_stop(struct net *net)
 	struct tipc_net *tn = net_generic(net, tipc_net_id);
 
 	synchronize_net();
+	clear_bit_unlock(0, &tn->net_exit_flag);
 	kfree(tn->bcbase);
 	kfree(tn->bcl);
 }
diff --git a/net/tipc/core.c b/net/tipc/core.c
index 4f6dc74adf45..93ea7dc05bf2 100644
--- a/net/tipc/core.c
+++ b/net/tipc/core.c
@@ -60,6 +60,7 @@ static int __net_init tipc_init_net(struct net *net)
 	tn->trial_addr = 0;
 	tn->addr_trial_end = 0;
 	tn->capabilities = TIPC_NODE_CAPABILITIES;
+	test_and_set_bit_lock(0, &tn->net_exit_flag);
 	memset(tn->node_id, 0, sizeof(tn->node_id));
 	memset(tn->node_id_string, 0, sizeof(tn->node_id_string));
 	tn->mon_threshold = TIPC_DEF_MON_THRESHOLD;
diff --git a/net/tipc/core.h b/net/tipc/core.h
index 631d83c9705f..aa75882dd932 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -143,6 +143,7 @@ struct tipc_net {
 	/* TX crypto handler */
 	struct tipc_crypto *crypto_tx;
 #endif
+	unsigned long net_exit_flag;
 };
 
 static inline struct tipc_net *tipc_net(struct net *net)
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 85400e4242de..0dcbfcff5ad3 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -132,6 +132,9 @@ static void tipc_net_finalize(struct net *net, u32 addr)
 {
 	struct tipc_net *tn = tipc_net(net);
 
+	if (unlikely(!test_bit(0, &tn->net_exit_flag)))
+		return;
+
 	if (cmpxchg(&tn->node_addr, 0, addr))
 		return;
 	tipc_set_node_addr(net, addr);
-- 
2.25.1

[tipc-discussion] [PATCH] tipc: fix use-after-free in tipc_bcast_get_mode

[Hoang H. Le <hoa...@de...>]

Syzbot has reported those issues as:

==================================================================
BUG: KASAN: use-after-free in tipc_bcast_get_mode+0x3ab/0x400 net/tipc/bcast.c:759
Read of size 1 at addr ffff88805e6b3571 by task kworker/0:6/3850

CPU: 0 PID: 3850 Comm: kworker/0:6 Not tainted 5.8.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events tipc_net_finalize_work

Thread 1's call trace:
[...]
  kfree+0x103/0x2c0 mm/slab.c:3757 <- bcbase releasing
  tipc_bcast_stop+0x1b0/0x2f0 net/tipc/bcast.c:721
  tipc_exit_net+0x24/0x270 net/tipc/core.c:112
[...]

Thread 2's call trace:
[...]
  tipc_bcast_get_mode+0x3ab/0x400 net/tipc/bcast.c:759 <- bcbase
has already been freed by Thread 1

  tipc_node_broadcast+0x9e/0xcc0 net/tipc/node.c:1744
  tipc_nametbl_publish+0x60b/0x970 net/tipc/name_table.c:752
  tipc_net_finalize net/tipc/net.c:141 [inline]
  tipc_net_finalize+0x1fa/0x310 net/tipc/net.c:131
  tipc_net_finalize_work+0x55/0x80 net/tipc/net.c:150
[...]

==================================================================
BUG: KASAN: use-after-free in tipc_named_reinit+0xef/0x290 net/tipc/name_distr.c:344
Read of size 8 at addr ffff888052ab2000 by task kworker/0:13/30628
CPU: 0 PID: 30628 Comm: kworker/0:13 Not tainted 5.8.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events tipc_net_finalize_work
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x1f0/0x31e lib/dump_stack.c:118
 print_address_description+0x66/0x5a0 mm/kasan/report.c:383
 __kasan_report mm/kasan/report.c:513 [inline]
 kasan_report+0x132/0x1d0 mm/kasan/report.c:530
 tipc_named_reinit+0xef/0x290 net/tipc/name_distr.c:344
 tipc_net_finalize+0x85/0xe0 net/tipc/net.c:138
 tipc_net_finalize_work+0x50/0x70 net/tipc/net.c:150
 process_one_work+0x789/0xfc0 kernel/workqueue.c:2269
 worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
 kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
[...]
Freed by task 14058:
 save_stack mm/kasan/common.c:48 [inline]
 set_track mm/kasan/common.c:56 [inline]
 kasan_set_free_info mm/kasan/common.c:316 [inline]
 __kasan_slab_free+0x114/0x170 mm/kasan/common.c:455
 __cache_free mm/slab.c:3426 [inline]
 kfree+0x10a/0x220 mm/slab.c:3757
 tipc_exit_net+0x29/0x50 net/tipc/core.c:113
 ops_exit_list net/core/net_namespace.c:186 [inline]
 cleanup_net+0x708/0xba0 net/core/net_namespace.c:603
 process_one_work+0x789/0xfc0 kernel/workqueue.c:2269
 worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
 kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293

Solution 1 (preferer):
Fix it by calling flush_scheduled_work() to make sure the
tipc_net_finalize_work() stopped before releasing bcbase object.

Solution 2:
Fix it by introducing a bit flag and returning if flag is zero
(object had already been freed)

Reported-by: syz...@sy...
Reported-by: syz...@sy...
Signed-off-by: Hoang Huu Le <hoa...@de...>
---
 net/tipc/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/tipc/core.c b/net/tipc/core.c
index 4f6dc74adf45..37d8695548cf 100644
--- a/net/tipc/core.c
+++ b/net/tipc/core.c
@@ -109,6 +109,11 @@ static void __net_exit tipc_exit_net(struct net *net)
 {
 	tipc_detach_loopback(net);
 	tipc_net_stop(net);
+
+	/* Make sure the tipc_net_finalize_work stopped
+	 * before releasing the resources.
+	 */
+	flush_scheduled_work();
 	tipc_bcast_stop(net);
 	tipc_nametbl_stop(net);
 	tipc_sk_rht_destroy(net);
-- 
2.25.1

[tipc-discussion] [iproute2-next v2] tipc: support 128bit node identity for peer removing

[Hoang H. Le <hoa...@de...>]

From: Hoang Le <hoa...@de...>

Problem:
In kernel upstream, we add the support to set node identity with
128bit. However, we are still using legacy format in command tipc
peer removing. Then, we got a problem when trying to remove
offline node i.e:

$ tipc node list
Node Identity                    Hash     State
d6babc1c1c6d                     1cbcd7ca down

$ tipc peer remove address d6babc1c1c6d
invalid network address, syntax: Z.C.N
error: No such device or address

Solution:
We add the support to remove a specific node down with 128bit
node identifier, as an alternative to legacy 32-bit node address.

Signed-off-by: Hoang Le <hoa...@de...>
Signed-off-by: Hoang Huu Le <hoa...@de...>
---
 tipc/peer.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/tipc/peer.c b/tipc/peer.c
index f6380777033d..f14ec35e6f71 100644
--- a/tipc/peer.c
+++ b/tipc/peer.c
@@ -59,17 +59,68 @@ static int cmd_peer_rm_addr(struct nlmsghdr *nlh, const struct cmd *cmd,
 	return msg_doit(nlh, NULL, NULL);
 }
 
+static int cmd_peer_rm_nodeid(struct nlmsghdr *nlh, const struct cmd *cmd,
+			      struct cmdl *cmdl, void *data)
+{
+	char buf[MNL_SOCKET_BUFFER_SIZE];
+	__u8 id[16] = {0,};
+	__u64 *w0 = (__u64 *)&id[0];
+	__u64 *w1 = (__u64 *)&id[8];
+	struct nlattr *nest;
+	char *str;
+
+	if (cmdl->argc != cmdl->optind + 1) {
+		fprintf(stderr, "Usage: %s peer remove identity NODEID\n",
+			cmdl->argv[0]);
+		return -EINVAL;
+	}
+
+	str = shift_cmdl(cmdl);
+	if (str2nodeid(str, id)) {
+		fprintf(stderr, "Invalid node identity\n");
+		return -EINVAL;
+	}
+
+	nlh = msg_init(buf, TIPC_NL_PEER_REMOVE);
+	if (!nlh) {
+		fprintf(stderr, "error, message initialisation failed\n");
+		return -1;
+	}
+
+	nest = mnl_attr_nest_start(nlh, TIPC_NLA_NET);
+	mnl_attr_put_u64(nlh, TIPC_NLA_NET_NODEID, *w0);
+	mnl_attr_put_u64(nlh, TIPC_NLA_NET_NODEID_W1, *w1);
+	mnl_attr_nest_end(nlh, nest);
+
+	return msg_doit(nlh, NULL, NULL);
+}
+
 static void cmd_peer_rm_help(struct cmdl *cmdl)
+{
+	fprintf(stderr, "Usage: %s peer remove PROPERTY\n\n"
+		"PROPERTIES\n"
+		" identity NODEID         - Remove peer node identity\n",
+		cmdl->argv[0]);
+}
+
+static void cmd_peer_rm_addr_help(struct cmdl *cmdl)
 {
 	fprintf(stderr, "Usage: %s peer remove address ADDRESS\n",
 		cmdl->argv[0]);
 }
 
+static void cmd_peer_rm_nodeid_help(struct cmdl *cmdl)
+{
+	fprintf(stderr, "Usage: %s peer remove identity NODEID\n",
+		cmdl->argv[0]);
+}
+
 static int cmd_peer_rm(struct nlmsghdr *nlh, const struct cmd *cmd,
 			struct cmdl *cmdl, void *data)
 {
 	const struct cmd cmds[] = {
-		{ "address",	cmd_peer_rm_addr,	cmd_peer_rm_help },
+		{ "address",  cmd_peer_rm_addr,   cmd_peer_rm_addr_help },
+		{ "identity", cmd_peer_rm_nodeid, cmd_peer_rm_nodeid_help },
 		{ NULL }
 	};
 
-- 
2.25.1

[tipc-discussion] [iproute2-next] tipc: support 128bit node identity for peer removing

[Hoang H. Le <hoa...@de...>]

From: Hoang Le <hoa...@de...>

Problem:
In kernel upstream, we add the support to set node identity with
128bit. However, we are still using legacy format in command tipc
peer removing. Then, we got a problem when trying to remove
offline node i.e:

Node Identity                    Hash     State
d6babc1c1c6d                     1cbcd7ca down

invalid network address, syntax: Z.C.N
error: No such device or address

Solution:
We add the support to remove a specific node down with 128bit
node identifier, as an alternative to legacy 32-bit node address.

Signed-off-by: Hoang Le <hoa...@de...>
Signed-off-by: Hoang Huu Le <hoa...@de...>
---
 tipc/peer.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/tipc/peer.c b/tipc/peer.c
index f6380777033d..f14ec35e6f71 100644
--- a/tipc/peer.c
+++ b/tipc/peer.c
@@ -59,17 +59,68 @@ static int cmd_peer_rm_addr(struct nlmsghdr *nlh, const struct cmd *cmd,
 	return msg_doit(nlh, NULL, NULL);
 }
 
+static int cmd_peer_rm_nodeid(struct nlmsghdr *nlh, const struct cmd *cmd,
+			      struct cmdl *cmdl, void *data)
+{
+	char buf[MNL_SOCKET_BUFFER_SIZE];
+	__u8 id[16] = {0,};
+	__u64 *w0 = (__u64 *)&id[0];
+	__u64 *w1 = (__u64 *)&id[8];
+	struct nlattr *nest;
+	char *str;
+
+	if (cmdl->argc != cmdl->optind + 1) {
+		fprintf(stderr, "Usage: %s peer remove identity NODEID\n",
+			cmdl->argv[0]);
+		return -EINVAL;
+	}
+
+	str = shift_cmdl(cmdl);
+	if (str2nodeid(str, id)) {
+		fprintf(stderr, "Invalid node identity\n");
+		return -EINVAL;
+	}
+
+	nlh = msg_init(buf, TIPC_NL_PEER_REMOVE);
+	if (!nlh) {
+		fprintf(stderr, "error, message initialisation failed\n");
+		return -1;
+	}
+
+	nest = mnl_attr_nest_start(nlh, TIPC_NLA_NET);
+	mnl_attr_put_u64(nlh, TIPC_NLA_NET_NODEID, *w0);
+	mnl_attr_put_u64(nlh, TIPC_NLA_NET_NODEID_W1, *w1);
+	mnl_attr_nest_end(nlh, nest);
+
+	return msg_doit(nlh, NULL, NULL);
+}
+
 static void cmd_peer_rm_help(struct cmdl *cmdl)
+{
+	fprintf(stderr, "Usage: %s peer remove PROPERTY\n\n"
+		"PROPERTIES\n"
+		" identity NODEID         - Remove peer node identity\n",
+		cmdl->argv[0]);
+}
+
+static void cmd_peer_rm_addr_help(struct cmdl *cmdl)
 {
 	fprintf(stderr, "Usage: %s peer remove address ADDRESS\n",
 		cmdl->argv[0]);
 }
 
+static void cmd_peer_rm_nodeid_help(struct cmdl *cmdl)
+{
+	fprintf(stderr, "Usage: %s peer remove identity NODEID\n",
+		cmdl->argv[0]);
+}
+
 static int cmd_peer_rm(struct nlmsghdr *nlh, const struct cmd *cmd,
 			struct cmdl *cmdl, void *data)
 {
 	const struct cmd cmds[] = {
-		{ "address",	cmd_peer_rm_addr,	cmd_peer_rm_help },
+		{ "address",  cmd_peer_rm_addr,   cmd_peer_rm_addr_help },
+		{ "identity", cmd_peer_rm_nodeid, cmd_peer_rm_nodeid_help },
 		{ NULL }
 	};
 
-- 
2.25.1

Re: [tipc-discussion] [PATCH net] tipc: call rcu_read_lock() in tipc_aead_encrypt_done()

[David M. <da...@da...>]

From: Xin Long <luc...@gm...>
Date: Thu, 20 Aug 2020 15:34:47 +0800

> b->media->send_msg() requires rcu_read_lock(), as we can see
> elsewhere in tipc,  tipc_bearer_xmit, tipc_bearer_xmit_skb
> and tipc_bearer_bc_xmit().
> 
> Syzbot has reported this issue as:
> 
>   net/tipc/bearer.c:466 suspicious rcu_dereference_check() usage!
>   Workqueue: cryptd cryptd_queue_worker
>   Call Trace:
>    tipc_l2_send_msg+0x354/0x420 net/tipc/bearer.c:466
>    tipc_aead_encrypt_done+0x204/0x3a0 net/tipc/crypto.c:761
>    cryptd_aead_crypt+0xe8/0x1d0 crypto/cryptd.c:739
>    cryptd_queue_worker+0x118/0x1b0 crypto/cryptd.c:181
>    process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
>    worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
>    kthread+0x3b5/0x4a0 kernel/kthread.c:291
>    ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
> 
> So fix it by calling rcu_read_lock() in tipc_aead_encrypt_done()
> for b->media->send_msg().
> 
> Fixes: fc1b6d6de220 ("tipc: introduce TIPC encryption & authentication")
> Reported-by: syz...@sy...
> Signed-off-by: Xin Long <luc...@gm...>

Applied and queued up for -stable, thank you.

[tipc-discussion] [PATCH net] tipc: call rcu_read_lock() in tipc_aead_encrypt_done()

[Xin L. <luc...@gm...>]

b->media->send_msg() requires rcu_read_lock(), as we can see
elsewhere in tipc,  tipc_bearer_xmit, tipc_bearer_xmit_skb
and tipc_bearer_bc_xmit().

Syzbot has reported this issue as:

  net/tipc/bearer.c:466 suspicious rcu_dereference_check() usage!
  Workqueue: cryptd cryptd_queue_worker
  Call Trace:
   tipc_l2_send_msg+0x354/0x420 net/tipc/bearer.c:466
   tipc_aead_encrypt_done+0x204/0x3a0 net/tipc/crypto.c:761
   cryptd_aead_crypt+0xe8/0x1d0 crypto/cryptd.c:739
   cryptd_queue_worker+0x118/0x1b0 crypto/cryptd.c:181
   process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
   worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
   kthread+0x3b5/0x4a0 kernel/kthread.c:291
   ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293

So fix it by calling rcu_read_lock() in tipc_aead_encrypt_done()
for b->media->send_msg().

Fixes: fc1b6d6de220 ("tipc: introduce TIPC encryption & authentication")
Reported-by: syz...@sy...
Signed-off-by: Xin Long <luc...@gm...>
---
 net/tipc/crypto.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c
index 001bcb0..c38baba 100644
--- a/net/tipc/crypto.c
+++ b/net/tipc/crypto.c
@@ -757,10 +757,12 @@ static void tipc_aead_encrypt_done(struct crypto_async_request *base, int err)
 	switch (err) {
 	case 0:
 		this_cpu_inc(tx->stats->stat[STAT_ASYNC_OK]);
+		rcu_read_lock();
 		if (likely(test_bit(0, &b->up)))
 			b->media->send_msg(net, skb, b, &tx_ctx->dst);
 		else
 			kfree_skb(skb);
+		rcu_read_unlock();
 		break;
 	case -EINPROGRESS:
 		return;
-- 
2.1.0

Re: [tipc-discussion] WARNING: suspicious RCU usage in tipc_l2_send_msg

[Xin L. <luc...@gm...>]

On Sat, Jun 27, 2020 at 1:25 AM syzbot
<syz...@sy...> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    b835a71e usbnet: smsc95xx: Fix use-after-free after removal
> git tree:       net
> console output: https://syzkaller.appspot.com/x/log.txt?x=1095a51d100000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=dcc6334acae363d4
> dashboard link: https://syzkaller.appspot.com/bug?extid=47bbc6b678d317cccbe0
> compiler:       gcc (GCC) 10.1.0-syz 20200507
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syz...@sy...
>
> =============================
> WARNING: suspicious RCU usage
> 5.8.0-rc1-syzkaller #0 Not tainted
> -----------------------------
> net/tipc/bearer.c:466 suspicious rcu_dereference_check() usage!
>
> other info that might help us debug this:
>
>
> rcu_scheduler_active = 2, debug_locks = 1
> 2 locks held by kworker/0:16/19143:
>  #0: ffff8880a6901d38 ((wq_completion)cryptd){+.+.}-{0:0}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
>  #0: ffff8880a6901d38 ((wq_completion)cryptd){+.+.}-{0:0}, at: atomic64_set include/asm-generic/atomic-instrumented.h:856 [inline]
>  #0: ffff8880a6901d38 ((wq_completion)cryptd){+.+.}-{0:0}, at: atomic_long_set include/asm-generic/atomic-long.h:41 [inline]
>  #0: ffff8880a6901d38 ((wq_completion)cryptd){+.+.}-{0:0}, at: set_work_data kernel/workqueue.c:616 [inline]
>  #0: ffff8880a6901d38 ((wq_completion)cryptd){+.+.}-{0:0}, at: set_work_pool_and_clear_pending kernel/workqueue.c:643 [inline]
>  #0: ffff8880a6901d38 ((wq_completion)cryptd){+.+.}-{0:0}, at: process_one_work+0x82b/0x1670 kernel/workqueue.c:2240
>  #1: ffffc90006f9fda8 ((work_completion)(&cpu_queue->work)){+.+.}-{0:0}, at: process_one_work+0x85f/0x1670 kernel/workqueue.c:2244
>
> stack backtrace:
> CPU: 0 PID: 19143 Comm: kworker/0:16 Not tainted 5.8.0-rc1-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Workqueue: cryptd cryptd_queue_worker
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x18f/0x20d lib/dump_stack.c:118
>  tipc_l2_send_msg+0x354/0x420 net/tipc/bearer.c:466
>  tipc_aead_encrypt_done+0x204/0x3a0 net/tipc/crypto.c:761
>  cryptd_aead_crypt+0xe8/0x1d0 crypto/cryptd.c:739
>  cryptd_queue_worker+0x118/0x1b0 crypto/cryptd.c:181
>  process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
>  worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
>  kthread+0x3b5/0x4a0 kernel/kthread.c:291
>  ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
>
Like in bearer.c, rcu_read_lock() is needed before calling
b->media->send_msg() in tipc_aead_encrypt_done():

@@ -757,10 +757,12 @@ static void tipc_aead_encrypt_done(struct
crypto_async_request *base, int err)
        switch (err) {
        case 0:
                this_cpu_inc(tx->stats->stat[STAT_ASYNC_OK]);
+               rcu_read_lock();
                if (likely(test_bit(0, &b->up)))
                        b->media->send_msg(net, skb, b, &tx_ctx->dst);
                else
                        kfree_skb(skb);
+               rcu_read_unlock();
                break;
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syz...@go....
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

Re: [tipc-discussion] [PATCH net] ipv6: some fixes for ipv6_dev_find()

[David M. <da...@da...>]

From: Xin Long <luc...@gm...>
Date: Mon, 17 Aug 2020 14:30:49 +0800

> This patch is to do 3 things for ipv6_dev_find():
> 
>   As David A. noticed,
> 
>   - rt6_lookup() is not really needed. Different from __ip_dev_find(),
>     ipv6_dev_find() doesn't have a compatibility problem, so remove it.
> 
>   As Hideaki suggested,
> 
>   - "valid" (non-tentative) check for the address is also needed.
>     ipv6_chk_addr() calls ipv6_chk_addr_and_flags(), which will
>     traverse the address hash list, but it's heavy to be called
>     inside ipv6_dev_find(). This patch is to reuse the code of
>     ipv6_chk_addr_and_flags() for ipv6_dev_find().
> 
>   - dev parameter is passed into ipv6_dev_find(), as link-local
>     addresses from user space has sin6_scope_id set and the dev
>     lookup needs it.
> 
> Fixes: 81f6cb31222d ("ipv6: add ipv6_dev_find()")
> Suggested-by: YOSHIFUJI Hideaki <hid...@mi...>
> Reported-by: David Ahern <ds...@gm...>
> Signed-off-by: Xin Long <luc...@gm...>

Applied, thank you.

[tipc-discussion] syzbot bugs

[Jon M. <jm...@re...>]

Hi all,
Here is the link to the syzkaller dashboard I mentioned this morning.
If you search on the string "tipc" on the page you'll find that we have 
10 open issues.
I am sure that some of those already are solved, since there are people 
like our friend Cong Wang working on this kind of bugs, but this at 
least should give you an overview of the status.

https://syzkaller.appspot.com/upstream

///jon

Re: [tipc-discussion] [PATCH] net: tipc: Convert to use the preferred fallthrough macro

[David M. <da...@da...>]

From: Miaohe Lin <lin...@hu...>
Date: Tue, 18 Aug 2020 08:07:13 -0400

> Convert the uses of fallthrough comments to fallthrough macro.
> 
> Signed-off-by: Miaohe Lin <lin...@hu...>

Applied to net-next.

Re: [tipc-discussion] [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Xin L. <luc...@gm...>]

On Tue, Aug 18, 2020 at 6:20 AM Cong Wang <xiy...@gm...> wrote:
>
> On Mon, Aug 17, 2020 at 2:39 PM David Miller <da...@da...> wrote:
> >
> > From: Cong Wang <xiy...@gm...>
> > Date: Mon, 17 Aug 2020 13:59:46 -0700
> >
> > > Is this a new Kconfig feature? ipv6_stub was introduced for
> > > VXLAN, at that time I don't remember we have such kind of
> > > Kconfig rules, otherwise it would not be needed.
> >
> > The ipv6_stub exists in order to allow the troublesome
> > "ipv6=m && feature_using_ipv6=y" combination.
For certain code, instead of IS_ENABLE(), use IS_REACHABLE().

>
> Hmm, so "IPV6=m && TIPC=y" is not a concern here as you pick
> this patch over adding a ipv6_stub?
>
This is more a question for TIPC users.

Hi, Jon and Ying,

Have you met any users having "IPV6=m && TIPC=y" in their kernels?

Re: [tipc-discussion] [PATCH net] tipc: not enable tipc when ipv6 works as a module

[David M. <da...@da...>]

From: Cong Wang <xiy...@gm...>
Date: Mon, 17 Aug 2020 13:59:46 -0700

> Is this a new Kconfig feature? ipv6_stub was introduced for
> VXLAN, at that time I don't remember we have such kind of
> Kconfig rules, otherwise it would not be needed.

The ipv6_stub exists in order to allow the troublesome
"ipv6=m && feature_using_ipv6=y" combination.

Re: [tipc-discussion] [PATCH net] tipc: not enable tipc when ipv6 works as a module

[David M. <da...@da...>]

From: Cong Wang <xiy...@gm...>
Date: Mon, 17 Aug 2020 13:29:40 -0700

> On Mon, Aug 17, 2020 at 12:55 PM Randy Dunlap <rd...@in...> wrote:
>>
>> TIPC=m and IPV6=m builds just fine.
>>
>> Having tipc autoload ipv6 is a different problem. (IMO)
>>
>>
>> This Kconfig entry:
>>  menuconfig TIPC
>>         tristate "The TIPC Protocol"
>>         depends on INET
>> +       depends on IPV6 || IPV6=n
>>
>> says:
>> If IPV6=n, TIPC can be y/m/n.
>> If IPV6=y/m, TIPC is limited to whatever IPV6 is set to.
> 
> Hmm, nowadays we _do_ have IPV6=y on popular distros.
> So this means TIPC would have to be builtin after this patch??

Note the word "limited", ipv6=y allows y and m, ipv6=m (more limited)
allows only m.

Re: [tipc-discussion] [PATCH net] tipc: not enable tipc when ipv6 works as a module

[David M. <da...@da...>]

From: Cong Wang <xiy...@gm...>
Date: Mon, 17 Aug 2020 11:55:55 -0700

> On Mon, Aug 17, 2020 at 11:49 AM Randy Dunlap <rd...@in...> wrote:
>>
>> It just restricts how TIPC can be built, so that
>> TIPC=y and IPV6=m cannot happen together, which causes
>> a build error.
> 
> It also disallows TIPC=m and IPV6=m, right?

That combination is allowed.

The whole "X || X=n" construct means X must be off or equal to the
value of the Kconfig entry this dependency is for.

Thus you'll see "depends IPV6 || IPV6=n" everywhere.

Re: [tipc-discussion] [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Xin L. <luc...@gm...>]

On Mon, Aug 17, 2020 at 2:29 AM Cong Wang <xiy...@gm...> wrote:
>
> On Sun, Aug 16, 2020 at 4:54 AM Xin Long <luc...@gm...> wrote:
> >
> > When using ipv6_dev_find() in one module, it requires ipv6 not to
> > work as a module. Otherwise, this error occurs in build:
> >
> >   undefined reference to `ipv6_dev_find'.
> >
> > So fix it by adding "depends on IPV6 || IPV6=n" to tipc/Kconfig,
> > as it does in sctp/Kconfig.
>
> Or put it into struct ipv6_stub?
Hi Cong,

That could be one way. We may do it when this new function becomes more common.
By now, I think it's okay to make TIPC depend on IPV6 || IPV6=n.

Thanks.

[tipc-discussion] [PATCH net] ipv6: some fixes for ipv6_dev_find()

[Xin L. <luc...@gm...>]

This patch is to do 3 things for ipv6_dev_find():

  As David A. noticed,

  - rt6_lookup() is not really needed. Different from __ip_dev_find(),
    ipv6_dev_find() doesn't have a compatibility problem, so remove it.

  As Hideaki suggested,

  - "valid" (non-tentative) check for the address is also needed.
    ipv6_chk_addr() calls ipv6_chk_addr_and_flags(), which will
    traverse the address hash list, but it's heavy to be called
    inside ipv6_dev_find(). This patch is to reuse the code of
    ipv6_chk_addr_and_flags() for ipv6_dev_find().

  - dev parameter is passed into ipv6_dev_find(), as link-local
    addresses from user space has sin6_scope_id set and the dev
    lookup needs it.

Fixes: 81f6cb31222d ("ipv6: add ipv6_dev_find()")
Suggested-by: YOSHIFUJI Hideaki <hid...@mi...>
Reported-by: David Ahern <ds...@gm...>
Signed-off-by: Xin Long <luc...@gm...>
---
 include/net/addrconf.h |  3 ++-
 net/ipv6/addrconf.c    | 60 +++++++++++++++++++-------------------------------
 net/tipc/udp_media.c   |  8 +++----
 3 files changed, 28 insertions(+), 43 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index ba3f6c15..18f783d 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -97,7 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr,
 
 int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev);
 
-struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr);
+struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr,
+				 struct net_device *dev);
 
 struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net,
 				     const struct in6_addr *addr,
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8e761b8..01146b6 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1893,12 +1893,13 @@ EXPORT_SYMBOL(ipv6_chk_addr);
  *   2. does the address exist on the specific device
  *      (skip_dev_check = false)
  */
-int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
-			    const struct net_device *dev, bool skip_dev_check,
-			    int strict, u32 banned_flags)
+static struct net_device *
+__ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
+			  const struct net_device *dev, bool skip_dev_check,
+			  int strict, u32 banned_flags)
 {
 	unsigned int hash = inet6_addr_hash(net, addr);
-	const struct net_device *l3mdev;
+	struct net_device *l3mdev, *ndev;
 	struct inet6_ifaddr *ifp;
 	u32 ifp_flags;
 
@@ -1909,10 +1910,11 @@ int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
 		dev = NULL;
 
 	hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) {
-		if (!net_eq(dev_net(ifp->idev->dev), net))
+		ndev = ifp->idev->dev;
+		if (!net_eq(dev_net(ndev), net))
 			continue;
 
-		if (l3mdev_master_dev_rcu(ifp->idev->dev) != l3mdev)
+		if (l3mdev_master_dev_rcu(ndev) != l3mdev)
 			continue;
 
 		/* Decouple optimistic from tentative for evaluation here.
@@ -1923,15 +1925,23 @@ int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
 			    : ifp->flags;
 		if (ipv6_addr_equal(&ifp->addr, addr) &&
 		    !(ifp_flags&banned_flags) &&
-		    (!dev || ifp->idev->dev == dev ||
+		    (!dev || ndev == dev ||
 		     !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))) {
 			rcu_read_unlock();
-			return 1;
+			return ndev;
 		}
 	}
 
 	rcu_read_unlock();
-	return 0;
+	return NULL;
+}
+
+int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
+			    const struct net_device *dev, bool skip_dev_check,
+			    int strict, u32 banned_flags)
+{
+	return __ipv6_chk_addr_and_flags(net, addr, dev, skip_dev_check,
+					 strict, banned_flags) ? 1 : 0;
 }
 EXPORT_SYMBOL(ipv6_chk_addr_and_flags);
 
@@ -1990,35 +2000,11 @@ EXPORT_SYMBOL(ipv6_chk_prefix);
  *
  * The caller should be protected by RCU, or RTNL.
  */
-struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr)
+struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr,
+				 struct net_device *dev)
 {
-	unsigned int hash = inet6_addr_hash(net, addr);
-	struct inet6_ifaddr *ifp, *result = NULL;
-	struct net_device *dev = NULL;
-
-	rcu_read_lock();
-	hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) {
-		if (net_eq(dev_net(ifp->idev->dev), net) &&
-		    ipv6_addr_equal(&ifp->addr, addr)) {
-			result = ifp;
-			break;
-		}
-	}
-
-	if (!result) {
-		struct rt6_info *rt;
-
-		rt = rt6_lookup(net, addr, NULL, 0, NULL, 0);
-		if (rt) {
-			dev = rt->dst.dev;
-			ip6_rt_put(rt);
-		}
-	} else {
-		dev = result->idev->dev;
-	}
-	rcu_read_unlock();
-
-	return dev;
+	return __ipv6_chk_addr_and_flags(net, addr, dev, !dev, 1,
+					 IFA_F_TENTATIVE);
 }
 EXPORT_SYMBOL(ipv6_dev_find);
 
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 53f0de0..911d13c 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -660,6 +660,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
 	struct udp_tunnel_sock_cfg tuncfg = {NULL};
 	struct nlattr *opts[TIPC_NLA_UDP_MAX + 1];
 	u8 node_id[NODE_ID_LEN] = {0,};
+	struct net_device *dev;
 	int rmcast = 0;
 
 	ub = kzalloc(sizeof(*ub), GFP_ATOMIC);
@@ -714,8 +715,6 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
 	rcu_assign_pointer(ub->bearer, b);
 	tipc_udp_media_addr_set(&b->addr, &local);
 	if (local.proto == htons(ETH_P_IP)) {
-		struct net_device *dev;
-
 		dev = __ip_dev_find(net, local.ipv4.s_addr, false);
 		if (!dev) {
 			err = -ENODEV;
@@ -738,9 +737,8 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
 		b->mtu = b->media->mtu;
 #if IS_ENABLED(CONFIG_IPV6)
 	} else if (local.proto == htons(ETH_P_IPV6)) {
-		struct net_device *dev;
-
-		dev = ipv6_dev_find(net, &local.ipv6);
+		dev = ub->ifindex ? __dev_get_by_index(net, ub->ifindex) : NULL;
+		dev = ipv6_dev_find(net, &local.ipv6, dev);
 		if (!dev) {
 			err = -ENODEV;
 			goto err;
-- 
2.1.0

Re: [tipc-discussion] [PATCH net] tipc: not enable tipc when ipv6 works as a module

[David M. <da...@da...>]

From: Xin Long <luc...@gm...>
Date: Sun, 16 Aug 2020 17:32:03 +0800

> When using ipv6_dev_find() in one module, it requires ipv6 not to
> work as a module. Otherwise, this error occurs in build:
> 
>   undefined reference to `ipv6_dev_find'.
> 
> So fix it by adding "depends on IPV6 || IPV6=n" to tipc/Kconfig,
> as it does in sctp/Kconfig.
> 
> Fixes: 5a6f6f579178 ("tipc: set ub->ifindex for local ipv6 address")
> Reported-by: kernel test robot <lk...@in...>
> Acked-by: Randy Dunlap <rd...@in...>
> Signed-off-by: Xin Long <luc...@gm...>

Applied.

[tipc-discussion] [PATCH net] tipc: not enable tipc when ipv6 works as a module

[Xin L. <luc...@gm...>]

When using ipv6_dev_find() in one module, it requires ipv6 not to
work as a module. Otherwise, this error occurs in build:

  undefined reference to `ipv6_dev_find'.

So fix it by adding "depends on IPV6 || IPV6=n" to tipc/Kconfig,
as it does in sctp/Kconfig.

Fixes: 5a6f6f579178 ("tipc: set ub->ifindex for local ipv6 address")
Reported-by: kernel test robot <lk...@in...>
Acked-by: Randy Dunlap <rd...@in...>
Signed-off-by: Xin Long <luc...@gm...>
---
 net/tipc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/tipc/Kconfig b/net/tipc/Kconfig
index 9dd7802..be1c400 100644
--- a/net/tipc/Kconfig
+++ b/net/tipc/Kconfig
@@ -6,6 +6,7 @@
 menuconfig TIPC
 	tristate "The TIPC Protocol"
 	depends on INET
+	depends on IPV6 || IPV6=n
 	help
 	  The Transparent Inter Process Communication (TIPC) protocol is
 	  specially designed for intra cluster communication. This protocol
-- 
2.1.0

Re: [tipc-discussion] [PATCH net 1/2] ipv6: add ipv6_dev_find()

[Xin L. <luc...@gm...>]

On Tue, Aug 11, 2020 at 10:26 AM Hideaki Yoshifuji
<hid...@mi...> wrote:
>
> Hi,
>
> 2020年8月9日(日) 19:52 Xin Long <luc...@gm...>:
> >
> > On Fri, Aug 7, 2020 at 5:26 PM Hideaki Yoshifuji
> > <hid...@mi...> wrote:
> > >
> > > Hi,
> > >
> > > 2020年8月6日(木) 23:03 David Ahern <ds...@gm...>:
> > > >
> > > > On 8/6/20 2:55 AM, Xin Long wrote:
> > > > > On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji
> > > > > <hid...@mi...> wrote:
> > > > >>
> > > > >> Hi,
> > > > >>
> > > > >> 2020年8月4日(火) 0:35 Xin Long <luc...@gm...>:
> > > > >>>
> > > > >>> This is to add an ip_dev_find like function for ipv6, used to find
> > > > >>> the dev by saddr.
> > > > >>>
> > > > >>> It will be used by TIPC protocol. So also export it.
> > > > >>>
> > > > >>> Signed-off-by: Xin Long <luc...@gm...>
> > > > >>> ---
> > > > >>>  include/net/addrconf.h |  2 ++
> > > > >>>  net/ipv6/addrconf.c    | 39 +++++++++++++++++++++++++++++++++++++++
> > > > >>>  2 files changed, 41 insertions(+)
> > > > >>>
> > > > >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> > > > >>> index 8418b7d..ba3f6c15 100644
> > > > >>> --- a/include/net/addrconf.h
> > > > >>> +++ b/include/net/addrconf.h
> > > > >>> @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr,
> > > > >>>
> > > > >>>  int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev);
> > > > >>>
> > > > >>> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr);
> > > > >>> +
> > > > >>
> > > > >> How do we handle link-local addresses?
> > > > > This is what "if (!result)" branch meant to do:
> > > > >
> > > > > +       if (!result) {
> > > > > +               struct rt6_info *rt;
> > > > > +
> > > > > +               rt = rt6_lookup(net, addr, NULL, 0, NULL, 0);
> > > > > +               if (rt) {
> > > > > +                       dev = rt->dst.dev;
> > > > > +                       ip6_rt_put(rt);
> > > > > +               }
> > > > > +       } else {
> > > > > +               dev = result->idev->dev;
> > > > > +       }
> > > > >
> > > >
> > > > the stated purpose of this function is to find the netdevice to which an
> > > > address is attached. A route lookup should not be needed. Walking the
> > > > address hash list finds the address and hence the netdev or it does not.
> > > >
> > > >
> > >
> > > User supplied scope id which should be set for link-local addresses
> > > in TIPC_NLA_UDP_LOCAL attribute must be honored when we
> > > check the address.
> > Hi, Hideaki san,
> >
> > Sorry for not understanding your comment earlier.
> >
> > The bad thing is tipc in iproute2 doesn't seem able to set scope_id.
>
> I looked into the iproute2 code quickly and I think it should; it uses
> getaddrinfo(3) and it will fill if you say "fe80::1%eth0" or something
> like that.... OR, fix the bug.
right, thanks.

>
> > I saw many places in kernel doing this check:
> >
> >                          if (__ipv6_addr_needs_scope_id(atype) &&
> >                              !ip6->sin6_scope_id) { return -EINVAL; }
> >
> > Can I ask why scope id is needed for link-local addresses?
> > and is that for link-local addresses only?
>
> Because we distinguish link-local scope addresses on different interfaces.
> On the other hand, we do not distinguish global scope addresses on
> different interfaces.
okay.

>
> >
> > >
> > > ipv6_chk_addr() can check if the address and supplied ifindex is a valid
> > > local address.  Or introduce an extra ifindex argument to ipv6_dev_find().
> > Yeah, but if scope id means ifindex for  link-local addresses, ipv6_dev_find()
> > would be more like a function to validate the address with right scope id.
> >
>
> I think we should find a net_device with a specific "valid" (non-tentative)
> address here, and your initial implementation is not enough because it does
> not reject tentative addresses.  I'd recommend using generic ipv6_chk_addr()
> inside.
ipv6_chk_addr() is calling ipv6_chk_addr_and_flags(), which traverses
the addr hash list again. So I'm thinking to reuse the code of
ipv6_chk_addr_and_flags(), and do:

+static struct net_device *
+__ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
+                         const struct net_device *dev, bool skip_dev_check,
+                         int strict, u32 banned_flags)
 {
        unsigned int hash = inet6_addr_hash(net, addr);
        const struct net_device *l3mdev;
@@ -1926,12 +1918,29 @@ int ipv6_chk_addr_and_flags(struct net *net,
const struct in6_addr *addr,
                    (!dev || ifp->idev->dev == dev ||
                     !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))) {
                        rcu_read_unlock();
-                       return 1;
+                       return ifp->idev->dev;
                }
        }

        rcu_read_unlock();
-       return 0;
+       return NULL;
+}

and change these functions to :

int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
                            const struct net_device *dev, bool skip_dev_check,
                            int strict, u32 banned_flags)
{
        return __ipv6_chk_addr_and_flags(net, addr, dev, skip_dev_check,
                                         strict, banned_flags) ? 1 : 0;
}
EXPORT_SYMBOL(ipv6_chk_addr_and_flags);

struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr,
                                 struct net_device *dev)
{
        return __ipv6_chk_addr_and_flags(net, addr, NULL, 0, 1,
                                         IFA_F_TENTATIVE);
}
EXPORT_SYMBOL(ipv6_dev_find);

what do you think?

Re: [tipc-discussion] [PATCH net 1/2] ipv6: add ipv6_dev_find()

[Xin L. <luc...@gm...>]

On Fri, Aug 7, 2020 at 5:26 PM Hideaki Yoshifuji
<hid...@mi...> wrote:
>
> Hi,
>
> 2020年8月6日(木) 23:03 David Ahern <ds...@gm...>:
> >
> > On 8/6/20 2:55 AM, Xin Long wrote:
> > > On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji
> > > <hid...@mi...> wrote:
> > >>
> > >> Hi,
> > >>
> > >> 2020年8月4日(火) 0:35 Xin Long <luc...@gm...>:
> > >>>
> > >>> This is to add an ip_dev_find like function for ipv6, used to find
> > >>> the dev by saddr.
> > >>>
> > >>> It will be used by TIPC protocol. So also export it.
> > >>>
> > >>> Signed-off-by: Xin Long <luc...@gm...>
> > >>> ---
> > >>>  include/net/addrconf.h |  2 ++
> > >>>  net/ipv6/addrconf.c    | 39 +++++++++++++++++++++++++++++++++++++++
> > >>>  2 files changed, 41 insertions(+)
> > >>>
> > >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> > >>> index 8418b7d..ba3f6c15 100644
> > >>> --- a/include/net/addrconf.h
> > >>> +++ b/include/net/addrconf.h
> > >>> @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr,
> > >>>
> > >>>  int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev);
> > >>>
> > >>> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr);
> > >>> +
> > >>
> > >> How do we handle link-local addresses?
> > > This is what "if (!result)" branch meant to do:
> > >
> > > +       if (!result) {
> > > +               struct rt6_info *rt;
> > > +
> > > +               rt = rt6_lookup(net, addr, NULL, 0, NULL, 0);
> > > +               if (rt) {
> > > +                       dev = rt->dst.dev;
> > > +                       ip6_rt_put(rt);
> > > +               }
> > > +       } else {
> > > +               dev = result->idev->dev;
> > > +       }
> > >
> >
> > the stated purpose of this function is to find the netdevice to which an
> > address is attached. A route lookup should not be needed. Walking the
> > address hash list finds the address and hence the netdev or it does not.
> >
> >
>
> User supplied scope id which should be set for link-local addresses
> in TIPC_NLA_UDP_LOCAL attribute must be honored when we
> check the address.
Hi, Hideaki san,

Sorry for not understanding your comment earlier.

The bad thing is tipc in iproute2 doesn't seem able to set scope_id.
I saw many places in kernel doing this check:

                         if (__ipv6_addr_needs_scope_id(atype) &&
                             !ip6->sin6_scope_id) { return -EINVAL; }

Can I ask why scope id is needed for link-local addresses?
and is that for link-local addresses only?

>
> ipv6_chk_addr() can check if the address and supplied ifindex is a valid
> local address.  Or introduce an extra ifindex argument to ipv6_dev_find().
Yeah, but if scope id means ifindex for  link-local addresses, ipv6_dev_find()
would be more like a function to validate the address with right scope id.

Thanks for your reviewing.

Re: [tipc-discussion] [PATCH net 1/2] ipv6: add ipv6_dev_find()

[Xin L. <luc...@gm...>]

On Thu, Aug 6, 2020 at 10:03 PM David Ahern <ds...@gm...> wrote:
>
> On 8/6/20 2:55 AM, Xin Long wrote:
> > On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji
> > <hid...@mi...> wrote:
> >>
> >> Hi,
> >>
> >> 2020年8月4日(火) 0:35 Xin Long <luc...@gm...>:
> >>>
> >>> This is to add an ip_dev_find like function for ipv6, used to find
> >>> the dev by saddr.
> >>>
> >>> It will be used by TIPC protocol. So also export it.
> >>>
> >>> Signed-off-by: Xin Long <luc...@gm...>
> >>> ---
> >>>  include/net/addrconf.h |  2 ++
> >>>  net/ipv6/addrconf.c    | 39 +++++++++++++++++++++++++++++++++++++++
> >>>  2 files changed, 41 insertions(+)
> >>>
> >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> >>> index 8418b7d..ba3f6c15 100644
> >>> --- a/include/net/addrconf.h
> >>> +++ b/include/net/addrconf.h
> >>> @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr,
> >>>
> >>>  int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev);
> >>>
> >>> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr);
> >>> +
> >>
> >> How do we handle link-local addresses?
> > This is what "if (!result)" branch meant to do:
> >
> > +       if (!result) {
> > +               struct rt6_info *rt;
> > +
> > +               rt = rt6_lookup(net, addr, NULL, 0, NULL, 0);
> > +               if (rt) {
> > +                       dev = rt->dst.dev;
> > +                       ip6_rt_put(rt);
> > +               }
> > +       } else {
> > +               dev = result->idev->dev;
> > +       }
> >
>
> the stated purpose of this function is to find the netdevice to which an
> address is attached. A route lookup should not be needed. Walking the
> address hash list finds the address and hence the netdev or it does not.
Hi, David,
Sorry. it does. I misunderstood the code in __ip_dev_find().
I will delete the rt6_lookup() part from ipv6_dev_find().

Also for the compatibility, tipc part should change to:
@@ -741,10 +741,8 @@ static int tipc_udp_enable(struct net *net,
struct tipc_bearer *b,
                struct net_device *dev;

               dev = ipv6_dev_find(net, &local.ipv6);
               if (!dev)
                      ub->ifindex = dev->ifindex;

as when dev is not found from the hash list, it should fall back to
the old tipc code.

Ying, what do you think?

Re: [tipc-discussion] [PATCH net 1/2] ipv6: add ipv6_dev_find()

[Xin L. <luc...@gm...>]

On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji
<hid...@mi...> wrote:
>
> Hi,
>
> 2020年8月4日(火) 0:35 Xin Long <luc...@gm...>:
> >
> > This is to add an ip_dev_find like function for ipv6, used to find
> > the dev by saddr.
> >
> > It will be used by TIPC protocol. So also export it.
> >
> > Signed-off-by: Xin Long <luc...@gm...>
> > ---
> >  include/net/addrconf.h |  2 ++
> >  net/ipv6/addrconf.c    | 39 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 41 insertions(+)
> >
> > diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> > index 8418b7d..ba3f6c15 100644
> > --- a/include/net/addrconf.h
> > +++ b/include/net/addrconf.h
> > @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr,
> >
> >  int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev);
> >
> > +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr);
> > +
>
> How do we handle link-local addresses?
This is what "if (!result)" branch meant to do:

+       if (!result) {
+               struct rt6_info *rt;
+
+               rt = rt6_lookup(net, addr, NULL, 0, NULL, 0);
+               if (rt) {
+                       dev = rt->dst.dev;
+                       ip6_rt_put(rt);
+               }
+       } else {
+               dev = result->idev->dev;
+       }

Thanks.

>
> --yoshfuji
>
> >  struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net,
> >                                      const struct in6_addr *addr,
> >                                      struct net_device *dev, int strict);
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index 840bfdb..857d6f9 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -1983,6 +1983,45 @@ int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev)
> >  }
> >  EXPORT_SYMBOL(ipv6_chk_prefix);
> >
> > +/**
> > + * ipv6_dev_find - find the first device with a given source address.
> > + * @net: the net namespace
> > + * @addr: the source address
> > + *
> > + * The caller should be protected by RCU, or RTNL.
> > + */
> > +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr)
> > +{
> > +       unsigned int hash = inet6_addr_hash(net, addr);
> > +       struct inet6_ifaddr *ifp, *result = NULL;
> > +       struct net_device *dev = NULL;
> > +
> > +       rcu_read_lock();
> > +       hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) {
> > +               if (net_eq(dev_net(ifp->idev->dev), net) &&
> > +                   ipv6_addr_equal(&ifp->addr, addr)) {
> > +                       result = ifp;
> > +                       break;
> > +               }
> > +       }
> > +
> > +       if (!result) {
> > +               struct rt6_info *rt;
> > +
> > +               rt = rt6_lookup(net, addr, NULL, 0, NULL, 0);
> > +               if (rt) {
> > +                       dev = rt->dst.dev;
> > +                       ip6_rt_put(rt);
> > +               }
> > +       } else {
> > +               dev = result->idev->dev;
> > +       }
> > +       rcu_read_unlock();
> > +
> > +       return dev;
> > +}
> > +EXPORT_SYMBOL(ipv6_dev_find);
> > +
> >  struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *addr,
> >                                      struct net_device *dev, int strict)
> >  {
> > --
> > 2.1.0
> >

Re: [tipc-discussion] [PATCH net 0/2] net: fix a mcast issue for tipc udp media

[David M. <da...@da...>]

From: Xin Long <luc...@gm...>
Date: Mon,  3 Aug 2020 23:34:45 +0800

> Patch 1 is to add a function to get the dev by source address,
> whcih will be used by Patch 2.

Series applied, thank you.