public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
diff for duplicates of <20230815091324.GL22185@unreal>

diff --git a/a/1.txt b/N1/1.txt
index 6acce23..ce0752a 100644
--- a/a/1.txt
+++ b/N1/1.txt
@@ -1,101 +1,127 @@
 On Tue, Aug 15, 2023 at 04:47:58PM +0800, Dong Chenchen wrote:
-> On Mon, Aug 14, 2023 at 10:00:13PM +0800, Dong Chenchen wrote:
-> >> BUG: KASAN: slab-use-after-free in xfrm_policy_inexact_list_reinsert+0xb6/0x430
-> >> Read of size 1 at addr ffff8881051f3bf8 by task ip/668
-> >> 
-> >> CPU: 2 PID: 668 Comm: ip Not tainted 6.5.0-rc5-00182-g25aa0bebba72-dirty #64
-> >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13 04/01/2014
-> >> Call Trace:
-> >>  <TASK>
-> >>  dump_stack_lvl+0x72/0xa0
-> >>  print_report+0xd0/0x620
-> >>  kasan_report+0xb6/0xf0
-> >>  xfrm_policy_inexact_list_reinsert+0xb6/0x430
-> >>  xfrm_policy_inexact_insert_node.constprop.0+0x537/0x800
-> >>  xfrm_policy_inexact_alloc_chain+0x23f/0x320
-> >>  xfrm_policy_inexact_insert+0x6b/0x590
-> >>  xfrm_policy_insert+0x3b1/0x480
-> >>  xfrm_add_policy+0x23c/0x3c0
-> >>  xfrm_user_rcv_msg+0x2d0/0x510
-> >>  netlink_rcv_skb+0x10d/0x2d0
-> >>  xfrm_netlink_rcv+0x49/0x60
-> >>  netlink_unicast+0x3fe/0x540
-> >>  netlink_sendmsg+0x528/0x970
-> >>  sock_sendmsg+0x14a/0x160
-> >>  ____sys_sendmsg+0x4fc/0x580
-> >>  ___sys_sendmsg+0xef/0x160
-> >>  __sys_sendmsg+0xf7/0x1b0
-> >>  do_syscall_64+0x3f/0x90
-> >>  entry_SYSCALL_64_after_hwframe+0x73/0xdd
-> >> 
-> >> The root cause is:
-> >> 
-> >> cpu 0			cpu1
-> >> xfrm_dump_policy
-> >> xfrm_policy_walk
-> >> list_move_tail
-> >> 			xfrm_add_policy
-> >> 			... ...
-> >> 			xfrm_policy_inexact_list_reinsert
-> >> 			list_for_each_entry_reverse
-> >> 				if (!policy->bydst_reinsert)
-> >> 				//read non-existent policy
-> >> xfrm_dump_policy_done
-> >> xfrm_policy_walk_done
-> >> list_del(&walk->walk.all);
-> >> 
-> >> If dump_one_policy() returns err (triggered by netlink socket),
-> >> xfrm_policy_walk() will move walk initialized by socket to list
-> >> net->xfrm.policy_all. so this socket becomes visible in the global
-> >> policy list. The head *walk can be traversed when users add policies
-> >> with different prefixlen and trigger xfrm_policy node merge.
-> >> 
-> >> The issue can also be triggered by policy list traversal while rehashing
-> >> and flushing policies.
-> >> 
-> >> It can be fixed by skip such "policies" with walk.dead set to 1.
-> >> 
-> >> Fixes: 9cf545ebd591 ("xfrm: policy: store inexact policies in a tree ordered by destination address")
-> >> Fixes: 12a169e7d8f4 ("ipsec: Put dumpers on the dump list")
-> >> Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com>
-> >> ---
-> >> v2: fix similiar similar while rehashing and flushing policies
-> >> ---
-> >>  net/xfrm/xfrm_policy.c | 20 +++++++++++++++-----
-> >>  1 file changed, 15 insertions(+), 5 deletions(-)
+>> On Mon, Aug 14, 2023 at 10:00:13PM +0800, Dong Chenchen wrote:
+>> >> BUG: KASAN: slab-use-after-free in xfrm_policy_inexact_list_reinsert+0xb6/0x430
+>> >> Read of size 1 at addr ffff8881051f3bf8 by task ip/668
+>> >> 
+>> >> CPU: 2 PID: 668 Comm: ip Not tainted 6.5.0-rc5-00182-g25aa0bebba72-dirty #64
+>> >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13 04/01/2014
+>> >> Call Trace:
+>> >>  <TASK>
+>> >>  dump_stack_lvl+0x72/0xa0
+>> >>  print_report+0xd0/0x620
+>> >>  kasan_report+0xb6/0xf0
+>> >>  xfrm_policy_inexact_list_reinsert+0xb6/0x430
+>> >>  xfrm_policy_inexact_insert_node.constprop.0+0x537/0x800
+>> >>  xfrm_policy_inexact_alloc_chain+0x23f/0x320
+>> >>  xfrm_policy_inexact_insert+0x6b/0x590
+>> >>  xfrm_policy_insert+0x3b1/0x480
+>> >>  xfrm_add_policy+0x23c/0x3c0
+>> >>  xfrm_user_rcv_msg+0x2d0/0x510
+>> >>  netlink_rcv_skb+0x10d/0x2d0
+>> >>  xfrm_netlink_rcv+0x49/0x60
+>> >>  netlink_unicast+0x3fe/0x540
+>> >>  netlink_sendmsg+0x528/0x970
+>> >>  sock_sendmsg+0x14a/0x160
+>> >>  ____sys_sendmsg+0x4fc/0x580
+>> >>  ___sys_sendmsg+0xef/0x160
+>> >>  __sys_sendmsg+0xf7/0x1b0
+>> >>  do_syscall_64+0x3f/0x90
+>> >>  entry_SYSCALL_64_after_hwframe+0x73/0xdd
+>> >> 
+>> >> The root cause is:
+>> >> 
+>> >> cpu 0			cpu1
+>> >> xfrm_dump_policy
+>> >> xfrm_policy_walk
+>> >> list_move_tail
+>> >> 			xfrm_add_policy
+>> >> 			... ...
+>> >> 			xfrm_policy_inexact_list_reinsert
+>> >> 			list_for_each_entry_reverse
+>> >> 				if (!policy->bydst_reinsert)
+>> >> 				//read non-existent policy
+>> >> xfrm_dump_policy_done
+>> >> xfrm_policy_walk_done
+>> >> list_del(&walk->walk.all);
+>> >> 
+>> >> If dump_one_policy() returns err (triggered by netlink socket),
+>> >> xfrm_policy_walk() will move walk initialized by socket to list
+>> >> net->xfrm.policy_all. so this socket becomes visible in the global
+>> >> policy list. The head *walk can be traversed when users add policies
+>> >> with different prefixlen and trigger xfrm_policy node merge.
+>> >> 
+>> >> The issue can also be triggered by policy list traversal while rehashing
+>> >> and flushing policies.
+>> >> 
+>> >> It can be fixed by skip such "policies" with walk.dead set to 1.
+>> >> 
+>> >> Fixes: 9cf545ebd591 ("xfrm: policy: store inexact policies in a tree ordered by destination address")
+>> >> Fixes: 12a169e7d8f4 ("ipsec: Put dumpers on the dump list")
+>> >> Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com>
+>> >> ---
+>> >> v2: fix similiar similar while rehashing and flushing policies
+>> >> ---
+>> >>  net/xfrm/xfrm_policy.c | 20 +++++++++++++++-----
+>> >>  1 file changed, 15 insertions(+), 5 deletions(-)
+>
+><...>
+>
+>> >> @@ -1253,11 +1256,14 @@ static void xfrm_hash_rebuild(struct work_struct *work)
+>> >>  	 * we start with destructive action.
+>> >>  	 */
+>> >>  	list_for_each_entry(policy, &net->xfrm.policy_all, walk.all) {
+>> >> +		if (policy->walk.dead)
+>> >> +			continue;
+>> >> +
+>> >>  		struct xfrm_pol_inexact_bin *bin;
+>> >>  		u8 dbits, sbits;
+>> >
+>> >Same comment as above.
+>> >
+>> >>  
+>> >>  		dir = xfrm_policy_id2dir(policy->index);
+>> >> -		if (policy->walk.dead || dir >= XFRM_POLICY_MAX)
+>> >> +		if (dir >= XFRM_POLICY_MAX)
+>> >
+>> >This change is unnecessary, previous code was perfectly fine.
+>> >
+>> The walker object initialized by xfrm_policy_walk_init() doesnt have policy. 
+>> list_for_each_entry() will use the walker offset to calculate policy address.
+>> It's nonexistent and different from invalid dead policy. It will read memory 
+>> that doesnt belong to walker if dereference policy->index.
+>> I think we should protect the memory.
+>
+>But all operations here are an outcome of "list_for_each_entry(policy,
+>&net->xfrm.policy_all, walk.all)" which stores in policy iterator
+>the pointer to struct xfrm_policy.
+>
+>How at the same time access to policy->walk.dead is valid while
+>policy->index is not?
+>
+>Thanks
+1.walker init: its only a list head, no policy
+xfrm_dump_policy_start
+	xfrm_policy_walk_init(walk, XFRM_POLICY_TYPE_ANY);
+		INIT_LIST_HEAD(&walk->walk.all);
+		walk->walk.dead = 1;
 
-<...>
+2.add the walk head to net->xfrm.policy_all
+xfrm_policy_walk
+    list_for_each_entry_from(x, &net->xfrm.policy_all, all)
+	if (error) {
+		list_move_tail(&walk->walk.all, &x->all);
+		//add the walk to list tail
 
-> >> @@ -1253,11 +1256,14 @@ static void xfrm_hash_rebuild(struct work_struct *work)
-> >>  	 * we start with destructive action.
-> >>  	 */
-> >>  	list_for_each_entry(policy, &net->xfrm.policy_all, walk.all) {
-> >> +		if (policy->walk.dead)
-> >> +			continue;
-> >> +
-> >>  		struct xfrm_pol_inexact_bin *bin;
-> >>  		u8 dbits, sbits;
-> >
-> >Same comment as above.
-> >
-> >>  
-> >>  		dir = xfrm_policy_id2dir(policy->index);
-> >> -		if (policy->walk.dead || dir >= XFRM_POLICY_MAX)
-> >> +		if (dir >= XFRM_POLICY_MAX)
-> >
-> >This change is unnecessary, previous code was perfectly fine.
-> >
-> The walker object initialized by xfrm_policy_walk_init() doesnt have policy. 
-> list_for_each_entry() will use the walker offset to calculate policy address.
-> It's nonexistent and different from invalid dead policy. It will read memory 
-> that doesnt belong to walker if dereference policy->index.
-> I think we should protect the memory.
+3.traverse the walk list
+xfrm_policy_flush
+list_for_each_entry(pol, &net->xfrm.policy_all, walk.all)
+	 dir = xfrm_policy_id2dir(pol->index);
 
-But all operations here are an outcome of "list_for_each_entry(policy,
-&net->xfrm.policy_all, walk.all)" which stores in policy iterator
-the pointer to struct xfrm_policy.
+it gets policy by &net->xfrm.policy_all-0x130(offset of walk in policy)
+but when walk is head, we will read others memory by the calculated policy.
+such as:
+  walk addr  		policy addr
+0xffff0000d7f3b530    0xffff0000d7f3b400 (non-existent) 
 
-How at the same time access to policy->walk.dead is valid while
-policy->index is not?
-
-Thanks
+head walker of net->xfrm.policy_all can be skipped by  list_for_each_entry().
+but the walker created by socket is located list tail. so we should skip it.
diff --git a/a/content_digest b/N1/content_digest
index e22239f..0fd341f 100644
--- a/a/content_digest
+++ b/N1/content_digest
@@ -1,123 +1,149 @@
  "ref\020230814140013.712001-1-dongchenchen2@huawei.com\0"
  "ref\020230815060026.GE22185@unreal\0"
- "From\0Leon Romanovsky <leon@kernel.org>\0"
+ "From\0Dong Chenchen <dongchenchen2@huawei.com>\0"
  "Subject\0Re: [Patch net, v2] net: xfrm: skip policies marked as dead while reinserting policies\0"
- "Date\0Tue, 15 Aug 2023 12:13:24 +0300\0"
- "To\0Dong Chenchen <dongchenchen2@huawei.com>\0"
- "Cc\0fw@strlen.de"
-  steffen.klassert@secunet.com
-  herbert@gondor.apana.org.au
-  davem@davemloft.net
-  edumazet@google.com
-  kuba@kernel.org
-  pabeni@redhat.com
-  timo.teras@iki.fi
-  yuehaibing@huawei.com
-  weiyongjun1@huawei.com
-  netdev@vger.kernel.org
- " linux-kernel@vger.kernel.org\0"
+ "Date\0Tue, 15 Aug 2023 19:35:13 +0800\0"
+ "To\0<leon@kernel.org>\0"
+ "Cc\0<fw@strlen.de>"
+  <steffen.klassert@secunet.com>
+  <herbert@gondor.apana.org.au>
+  <davem@davemloft.net>
+  <edumazet@google.com>
+  <kuba@kernel.org>
+  <pabeni@redhat.com>
+  <timo.teras@iki.fi>
+  <yuehaibing@huawei.com>
+  <weiyongjun1@huawei.com>
+  <netdev@vger.kernel.org>
+ " <linux-kernel@vger.kernel.org>\0"
  "\00:1\0"
  "b\0"
  "On Tue, Aug 15, 2023 at 04:47:58PM +0800, Dong Chenchen wrote:\n"
- "> On Mon, Aug 14, 2023 at 10:00:13PM +0800, Dong Chenchen wrote:\n"
- "> >> BUG: KASAN: slab-use-after-free in xfrm_policy_inexact_list_reinsert+0xb6/0x430\n"
- "> >> Read of size 1 at addr ffff8881051f3bf8 by task ip/668\n"
- "> >> \n"
- "> >> CPU: 2 PID: 668 Comm: ip Not tainted 6.5.0-rc5-00182-g25aa0bebba72-dirty #64\n"
- "> >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13 04/01/2014\n"
- "> >> Call Trace:\n"
- "> >>  <TASK>\n"
- "> >>  dump_stack_lvl+0x72/0xa0\n"
- "> >>  print_report+0xd0/0x620\n"
- "> >>  kasan_report+0xb6/0xf0\n"
- "> >>  xfrm_policy_inexact_list_reinsert+0xb6/0x430\n"
- "> >>  xfrm_policy_inexact_insert_node.constprop.0+0x537/0x800\n"
- "> >>  xfrm_policy_inexact_alloc_chain+0x23f/0x320\n"
- "> >>  xfrm_policy_inexact_insert+0x6b/0x590\n"
- "> >>  xfrm_policy_insert+0x3b1/0x480\n"
- "> >>  xfrm_add_policy+0x23c/0x3c0\n"
- "> >>  xfrm_user_rcv_msg+0x2d0/0x510\n"
- "> >>  netlink_rcv_skb+0x10d/0x2d0\n"
- "> >>  xfrm_netlink_rcv+0x49/0x60\n"
- "> >>  netlink_unicast+0x3fe/0x540\n"
- "> >>  netlink_sendmsg+0x528/0x970\n"
- "> >>  sock_sendmsg+0x14a/0x160\n"
- "> >>  ____sys_sendmsg+0x4fc/0x580\n"
- "> >>  ___sys_sendmsg+0xef/0x160\n"
- "> >>  __sys_sendmsg+0xf7/0x1b0\n"
- "> >>  do_syscall_64+0x3f/0x90\n"
- "> >>  entry_SYSCALL_64_after_hwframe+0x73/0xdd\n"
- "> >> \n"
- "> >> The root cause is:\n"
- "> >> \n"
- "> >> cpu 0\t\t\tcpu1\n"
- "> >> xfrm_dump_policy\n"
- "> >> xfrm_policy_walk\n"
- "> >> list_move_tail\n"
- "> >> \t\t\txfrm_add_policy\n"
- "> >> \t\t\t... ...\n"
- "> >> \t\t\txfrm_policy_inexact_list_reinsert\n"
- "> >> \t\t\tlist_for_each_entry_reverse\n"
- "> >> \t\t\t\tif (!policy->bydst_reinsert)\n"
- "> >> \t\t\t\t//read non-existent policy\n"
- "> >> xfrm_dump_policy_done\n"
- "> >> xfrm_policy_walk_done\n"
- "> >> list_del(&walk->walk.all);\n"
- "> >> \n"
- "> >> If dump_one_policy() returns err (triggered by netlink socket),\n"
- "> >> xfrm_policy_walk() will move walk initialized by socket to list\n"
- "> >> net->xfrm.policy_all. so this socket becomes visible in the global\n"
- "> >> policy list. The head *walk can be traversed when users add policies\n"
- "> >> with different prefixlen and trigger xfrm_policy node merge.\n"
- "> >> \n"
- "> >> The issue can also be triggered by policy list traversal while rehashing\n"
- "> >> and flushing policies.\n"
- "> >> \n"
- "> >> It can be fixed by skip such \"policies\" with walk.dead set to 1.\n"
- "> >> \n"
- "> >> Fixes: 9cf545ebd591 (\"xfrm: policy: store inexact policies in a tree ordered by destination address\")\n"
- "> >> Fixes: 12a169e7d8f4 (\"ipsec: Put dumpers on the dump list\")\n"
- "> >> Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com>\n"
- "> >> ---\n"
- "> >> v2: fix similiar similar while rehashing and flushing policies\n"
- "> >> ---\n"
- "> >>  net/xfrm/xfrm_policy.c | 20 +++++++++++++++-----\n"
- "> >>  1 file changed, 15 insertions(+), 5 deletions(-)\n"
+ ">> On Mon, Aug 14, 2023 at 10:00:13PM +0800, Dong Chenchen wrote:\n"
+ ">> >> BUG: KASAN: slab-use-after-free in xfrm_policy_inexact_list_reinsert+0xb6/0x430\n"
+ ">> >> Read of size 1 at addr ffff8881051f3bf8 by task ip/668\n"
+ ">> >> \n"
+ ">> >> CPU: 2 PID: 668 Comm: ip Not tainted 6.5.0-rc5-00182-g25aa0bebba72-dirty #64\n"
+ ">> >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13 04/01/2014\n"
+ ">> >> Call Trace:\n"
+ ">> >>  <TASK>\n"
+ ">> >>  dump_stack_lvl+0x72/0xa0\n"
+ ">> >>  print_report+0xd0/0x620\n"
+ ">> >>  kasan_report+0xb6/0xf0\n"
+ ">> >>  xfrm_policy_inexact_list_reinsert+0xb6/0x430\n"
+ ">> >>  xfrm_policy_inexact_insert_node.constprop.0+0x537/0x800\n"
+ ">> >>  xfrm_policy_inexact_alloc_chain+0x23f/0x320\n"
+ ">> >>  xfrm_policy_inexact_insert+0x6b/0x590\n"
+ ">> >>  xfrm_policy_insert+0x3b1/0x480\n"
+ ">> >>  xfrm_add_policy+0x23c/0x3c0\n"
+ ">> >>  xfrm_user_rcv_msg+0x2d0/0x510\n"
+ ">> >>  netlink_rcv_skb+0x10d/0x2d0\n"
+ ">> >>  xfrm_netlink_rcv+0x49/0x60\n"
+ ">> >>  netlink_unicast+0x3fe/0x540\n"
+ ">> >>  netlink_sendmsg+0x528/0x970\n"
+ ">> >>  sock_sendmsg+0x14a/0x160\n"
+ ">> >>  ____sys_sendmsg+0x4fc/0x580\n"
+ ">> >>  ___sys_sendmsg+0xef/0x160\n"
+ ">> >>  __sys_sendmsg+0xf7/0x1b0\n"
+ ">> >>  do_syscall_64+0x3f/0x90\n"
+ ">> >>  entry_SYSCALL_64_after_hwframe+0x73/0xdd\n"
+ ">> >> \n"
+ ">> >> The root cause is:\n"
+ ">> >> \n"
+ ">> >> cpu 0\t\t\tcpu1\n"
+ ">> >> xfrm_dump_policy\n"
+ ">> >> xfrm_policy_walk\n"
+ ">> >> list_move_tail\n"
+ ">> >> \t\t\txfrm_add_policy\n"
+ ">> >> \t\t\t... ...\n"
+ ">> >> \t\t\txfrm_policy_inexact_list_reinsert\n"
+ ">> >> \t\t\tlist_for_each_entry_reverse\n"
+ ">> >> \t\t\t\tif (!policy->bydst_reinsert)\n"
+ ">> >> \t\t\t\t//read non-existent policy\n"
+ ">> >> xfrm_dump_policy_done\n"
+ ">> >> xfrm_policy_walk_done\n"
+ ">> >> list_del(&walk->walk.all);\n"
+ ">> >> \n"
+ ">> >> If dump_one_policy() returns err (triggered by netlink socket),\n"
+ ">> >> xfrm_policy_walk() will move walk initialized by socket to list\n"
+ ">> >> net->xfrm.policy_all. so this socket becomes visible in the global\n"
+ ">> >> policy list. The head *walk can be traversed when users add policies\n"
+ ">> >> with different prefixlen and trigger xfrm_policy node merge.\n"
+ ">> >> \n"
+ ">> >> The issue can also be triggered by policy list traversal while rehashing\n"
+ ">> >> and flushing policies.\n"
+ ">> >> \n"
+ ">> >> It can be fixed by skip such \"policies\" with walk.dead set to 1.\n"
+ ">> >> \n"
+ ">> >> Fixes: 9cf545ebd591 (\"xfrm: policy: store inexact policies in a tree ordered by destination address\")\n"
+ ">> >> Fixes: 12a169e7d8f4 (\"ipsec: Put dumpers on the dump list\")\n"
+ ">> >> Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com>\n"
+ ">> >> ---\n"
+ ">> >> v2: fix similiar similar while rehashing and flushing policies\n"
+ ">> >> ---\n"
+ ">> >>  net/xfrm/xfrm_policy.c | 20 +++++++++++++++-----\n"
+ ">> >>  1 file changed, 15 insertions(+), 5 deletions(-)\n"
+ ">\n"
+ "><...>\n"
+ ">\n"
+ ">> >> @@ -1253,11 +1256,14 @@ static void xfrm_hash_rebuild(struct work_struct *work)\n"
+ ">> >>  \t * we start with destructive action.\n"
+ ">> >>  \t */\n"
+ ">> >>  \tlist_for_each_entry(policy, &net->xfrm.policy_all, walk.all) {\n"
+ ">> >> +\t\tif (policy->walk.dead)\n"
+ ">> >> +\t\t\tcontinue;\n"
+ ">> >> +\n"
+ ">> >>  \t\tstruct xfrm_pol_inexact_bin *bin;\n"
+ ">> >>  \t\tu8 dbits, sbits;\n"
+ ">> >\n"
+ ">> >Same comment as above.\n"
+ ">> >\n"
+ ">> >>  \n"
+ ">> >>  \t\tdir = xfrm_policy_id2dir(policy->index);\n"
+ ">> >> -\t\tif (policy->walk.dead || dir >= XFRM_POLICY_MAX)\n"
+ ">> >> +\t\tif (dir >= XFRM_POLICY_MAX)\n"
+ ">> >\n"
+ ">> >This change is unnecessary, previous code was perfectly fine.\n"
+ ">> >\n"
+ ">> The walker object initialized by xfrm_policy_walk_init() doesnt have policy. \n"
+ ">> list_for_each_entry() will use the walker offset to calculate policy address.\n"
+ ">> It's nonexistent and different from invalid dead policy. It will read memory \n"
+ ">> that doesnt belong to walker if dereference policy->index.\n"
+ ">> I think we should protect the memory.\n"
+ ">\n"
+ ">But all operations here are an outcome of \"list_for_each_entry(policy,\n"
+ ">&net->xfrm.policy_all, walk.all)\" which stores in policy iterator\n"
+ ">the pointer to struct xfrm_policy.\n"
+ ">\n"
+ ">How at the same time access to policy->walk.dead is valid while\n"
+ ">policy->index is not?\n"
+ ">\n"
+ ">Thanks\n"
+ "1.walker init: its only a list head, no policy\n"
+ "xfrm_dump_policy_start\n"
+ "\txfrm_policy_walk_init(walk, XFRM_POLICY_TYPE_ANY);\n"
+ "\t\tINIT_LIST_HEAD(&walk->walk.all);\n"
+ "\t\twalk->walk.dead = 1;\n"
  "\n"
- "<...>\n"
+ "2.add the walk head to net->xfrm.policy_all\n"
+ "xfrm_policy_walk\n"
+ "    list_for_each_entry_from(x, &net->xfrm.policy_all, all)\n"
+ "\tif (error) {\n"
+ "\t\tlist_move_tail(&walk->walk.all, &x->all);\n"
+ "\t\t//add the walk to list tail\n"
  "\n"
- "> >> @@ -1253,11 +1256,14 @@ static void xfrm_hash_rebuild(struct work_struct *work)\n"
- "> >>  \t * we start with destructive action.\n"
- "> >>  \t */\n"
- "> >>  \tlist_for_each_entry(policy, &net->xfrm.policy_all, walk.all) {\n"
- "> >> +\t\tif (policy->walk.dead)\n"
- "> >> +\t\t\tcontinue;\n"
- "> >> +\n"
- "> >>  \t\tstruct xfrm_pol_inexact_bin *bin;\n"
- "> >>  \t\tu8 dbits, sbits;\n"
- "> >\n"
- "> >Same comment as above.\n"
- "> >\n"
- "> >>  \n"
- "> >>  \t\tdir = xfrm_policy_id2dir(policy->index);\n"
- "> >> -\t\tif (policy->walk.dead || dir >= XFRM_POLICY_MAX)\n"
- "> >> +\t\tif (dir >= XFRM_POLICY_MAX)\n"
- "> >\n"
- "> >This change is unnecessary, previous code was perfectly fine.\n"
- "> >\n"
- "> The walker object initialized by xfrm_policy_walk_init() doesnt have policy. \n"
- "> list_for_each_entry() will use the walker offset to calculate policy address.\n"
- "> It's nonexistent and different from invalid dead policy. It will read memory \n"
- "> that doesnt belong to walker if dereference policy->index.\n"
- "> I think we should protect the memory.\n"
+ "3.traverse the walk list\n"
+ "xfrm_policy_flush\n"
+ "list_for_each_entry(pol, &net->xfrm.policy_all, walk.all)\n"
+ "\t dir = xfrm_policy_id2dir(pol->index);\n"
  "\n"
- "But all operations here are an outcome of \"list_for_each_entry(policy,\n"
- "&net->xfrm.policy_all, walk.all)\" which stores in policy iterator\n"
- "the pointer to struct xfrm_policy.\n"
+ "it gets policy by &net->xfrm.policy_all-0x130(offset of walk in policy)\n"
+ "but when walk is head, we will read others memory by the calculated policy.\n"
+ "such as:\n"
+ "  walk addr  \t\tpolicy addr\n"
+ "0xffff0000d7f3b530    0xffff0000d7f3b400 (non-existent) \n"
  "\n"
- "How at the same time access to policy->walk.dead is valid while\n"
- "policy->index is not?\n"
- "\n"
- Thanks
+ "head walker of net->xfrm.policy_all can be skipped by  list_for_each_entry().\n"
+ but the walker created by socket is located list tail. so we should skip it.
 
-343239f1bea65a7cae259506fcb5a85441238b17645ce909b5a45d5b5c36791c
+7d3c9e4cc876ca41f672733708d7edf34d5b13c052db69c3e8b15c8882db77d5

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox