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

diff --git a/a/1.txt b/N1/1.txt
index bdf1acd..93be934 100644
--- a/a/1.txt
+++ b/N1/1.txt
@@ -1,141 +1,149 @@
 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(-)
-> 
-> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
-> index d6b405782b63..33efd46fb291 100644
-> --- a/net/xfrm/xfrm_policy.c
-> +++ b/net/xfrm/xfrm_policy.c
-> @@ -848,6 +848,9 @@ static void xfrm_policy_inexact_list_reinsert(struct net *net,
->  	matched_d = 0;
->  
->  	list_for_each_entry_reverse(policy, &net->xfrm.policy_all, walk.all) {
-> +		if (policy->walk.dead)
-> +			continue;
-> +
->  		struct hlist_node *newpos = NULL;
->  		bool matches_s, matches_d;
-
-You can't declare new variables in the middle of execution scope in C.
-
->  
-> @@ -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.
-
->  			continue;
->  
->  		if ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) {
-> @@ -1823,9 +1829,11 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
->  
->  again:
->  	list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
-> +		if (pol->walk.dead)
-> +			continue;
-> +
->  		dir = xfrm_policy_id2dir(pol->index);
-> -		if (pol->walk.dead ||
-> -		    dir >= XFRM_POLICY_MAX ||
-> +		if (dir >= XFRM_POLICY_MAX ||
-
-This change is unnecessary, previous code was perfectly fine.
-
->  		    pol->type != type)
->  			continue;
->  
-> @@ -1862,9 +1870,11 @@ int xfrm_dev_policy_flush(struct net *net, struct net_device *dev,
->  
->  again:
->  	list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
-> +		if (pol->walk.dead)
-> +			continue;
-> +
->  		dir = xfrm_policy_id2dir(pol->index);
-> -		if (pol->walk.dead ||
-> -		    dir >= XFRM_POLICY_MAX ||
-> +		if (dir >= XFRM_POLICY_MAX ||
->  		    pol->xdo.dev != dev)
->  			continue;
-
-Ditto.
+>> 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(-)
+>> 
+>> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
+>> index d6b405782b63..33efd46fb291 100644
+>> --- a/net/xfrm/xfrm_policy.c
+>> +++ b/net/xfrm/xfrm_policy.c
+>> @@ -848,6 +848,9 @@ static void xfrm_policy_inexact_list_reinsert(struct net *net,
+>>  	matched_d = 0;
+>>  
+>>  	list_for_each_entry_reverse(policy, &net->xfrm.policy_all, walk.all) {
+>> +		if (policy->walk.dead)
+>> +			continue;
+>> +
+>>  		struct hlist_node *newpos = NULL;
+>>  		bool matches_s, matches_d;
+>
+>You can't declare new variables in the middle of execution scope in C.
+Thank you for your suggestions. I will fix it in v3.
+>
+>>  
+>> @@ -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.
 
->  
-> -- 
-> 2.25.1
+Thanks
+>>  			continue;
+>>  
+>>  		if ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) {
+>> @@ -1823,9 +1829,11 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
+>>  
+>>  again:
+>>  	list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
+>> +		if (pol->walk.dead)
+>> +			continue;
+>> +
+>>  		dir = xfrm_policy_id2dir(pol->index);
+>> -		if (pol->walk.dead ||
+>> -		    dir >= XFRM_POLICY_MAX ||
+>> +		if (dir >= XFRM_POLICY_MAX ||
+>
+>This change is unnecessary, previous code was perfectly fine.
+>
+>>  		    pol->type != type)
+>>  			continue;
+>>  
+>> @@ -1862,9 +1870,11 @@ int xfrm_dev_policy_flush(struct net *net, struct net_device *dev,
+>>  
+>>  again:
+>>  	list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
+>> +		if (pol->walk.dead)
+>> +			continue;
+>> +
+>>  		dir = xfrm_policy_id2dir(pol->index);
+>> -		if (pol->walk.dead ||
+>> -		    dir >= XFRM_POLICY_MAX ||
+>> +		if (dir >= XFRM_POLICY_MAX ||
+>>  		    pol->xdo.dev != dev)
+>>  			continue;
+>
+>Ditto.
 >
+>>  
+>> -- 
+>> 2.25.1
+>>
diff --git a/a/content_digest b/N1/content_digest
index 4604059..e471551 100644
--- a/a/content_digest
+++ b/N1/content_digest
@@ -1,162 +1,170 @@
  "ref\020230814140013.712001-1-dongchenchen2@huawei.com\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 09:00:26 +0300\0"
- "To\0Dong Chenchen <dongchenchen2@huawei.com>\0"
- "Cc\0steffen.klassert@secunet.com"
-  herbert@gondor.apana.org.au
-  davem@davemloft.net
-  fw@strlen.de
-  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 16:47:58 +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 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"
- "> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c\n"
- "> index d6b405782b63..33efd46fb291 100644\n"
- "> --- a/net/xfrm/xfrm_policy.c\n"
- "> +++ b/net/xfrm/xfrm_policy.c\n"
- "> @@ -848,6 +848,9 @@ static void xfrm_policy_inexact_list_reinsert(struct net *net,\n"
- ">  \tmatched_d = 0;\n"
- ">  \n"
- ">  \tlist_for_each_entry_reverse(policy, &net->xfrm.policy_all, walk.all) {\n"
- "> +\t\tif (policy->walk.dead)\n"
- "> +\t\t\tcontinue;\n"
- "> +\n"
- ">  \t\tstruct hlist_node *newpos = NULL;\n"
- ">  \t\tbool matches_s, matches_d;\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"
+ ">> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c\n"
+ ">> index d6b405782b63..33efd46fb291 100644\n"
+ ">> --- a/net/xfrm/xfrm_policy.c\n"
+ ">> +++ b/net/xfrm/xfrm_policy.c\n"
+ ">> @@ -848,6 +848,9 @@ static void xfrm_policy_inexact_list_reinsert(struct net *net,\n"
+ ">>  \tmatched_d = 0;\n"
+ ">>  \n"
+ ">>  \tlist_for_each_entry_reverse(policy, &net->xfrm.policy_all, walk.all) {\n"
+ ">> +\t\tif (policy->walk.dead)\n"
+ ">> +\t\t\tcontinue;\n"
+ ">> +\n"
+ ">>  \t\tstruct hlist_node *newpos = NULL;\n"
+ ">>  \t\tbool matches_s, matches_d;\n"
+ ">\n"
+ ">You can't declare new variables in the middle of execution scope in C.\n"
+ "Thank you for your suggestions. I will fix it in v3.\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"
- "You can't declare new variables in the middle of execution scope in C.\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"
- ">  \t\t\tcontinue;\n"
- ">  \n"
- ">  \t\tif ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) {\n"
- "> @@ -1823,9 +1829,11 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)\n"
- ">  \n"
- ">  again:\n"
- ">  \tlist_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {\n"
- "> +\t\tif (pol->walk.dead)\n"
- "> +\t\t\tcontinue;\n"
- "> +\n"
- ">  \t\tdir = xfrm_policy_id2dir(pol->index);\n"
- "> -\t\tif (pol->walk.dead ||\n"
- "> -\t\t    dir >= XFRM_POLICY_MAX ||\n"
- "> +\t\tif (dir >= XFRM_POLICY_MAX ||\n"
- "\n"
- "This change is unnecessary, previous code was perfectly fine.\n"
- "\n"
- ">  \t\t    pol->type != type)\n"
- ">  \t\t\tcontinue;\n"
- ">  \n"
- "> @@ -1862,9 +1870,11 @@ int xfrm_dev_policy_flush(struct net *net, struct net_device *dev,\n"
- ">  \n"
- ">  again:\n"
- ">  \tlist_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {\n"
- "> +\t\tif (pol->walk.dead)\n"
- "> +\t\t\tcontinue;\n"
- "> +\n"
- ">  \t\tdir = xfrm_policy_id2dir(pol->index);\n"
- "> -\t\tif (pol->walk.dead ||\n"
- "> -\t\t    dir >= XFRM_POLICY_MAX ||\n"
- "> +\t\tif (dir >= XFRM_POLICY_MAX ||\n"
- ">  \t\t    pol->xdo.dev != dev)\n"
- ">  \t\t\tcontinue;\n"
- "\n"
- "Ditto.\n"
- "\n"
- ">  \n"
- "> -- \n"
- "> 2.25.1\n"
- >
+ "Thanks\n"
+ ">>  \t\t\tcontinue;\n"
+ ">>  \n"
+ ">>  \t\tif ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) {\n"
+ ">> @@ -1823,9 +1829,11 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)\n"
+ ">>  \n"
+ ">>  again:\n"
+ ">>  \tlist_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {\n"
+ ">> +\t\tif (pol->walk.dead)\n"
+ ">> +\t\t\tcontinue;\n"
+ ">> +\n"
+ ">>  \t\tdir = xfrm_policy_id2dir(pol->index);\n"
+ ">> -\t\tif (pol->walk.dead ||\n"
+ ">> -\t\t    dir >= XFRM_POLICY_MAX ||\n"
+ ">> +\t\tif (dir >= XFRM_POLICY_MAX ||\n"
+ ">\n"
+ ">This change is unnecessary, previous code was perfectly fine.\n"
+ ">\n"
+ ">>  \t\t    pol->type != type)\n"
+ ">>  \t\t\tcontinue;\n"
+ ">>  \n"
+ ">> @@ -1862,9 +1870,11 @@ int xfrm_dev_policy_flush(struct net *net, struct net_device *dev,\n"
+ ">>  \n"
+ ">>  again:\n"
+ ">>  \tlist_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {\n"
+ ">> +\t\tif (pol->walk.dead)\n"
+ ">> +\t\t\tcontinue;\n"
+ ">> +\n"
+ ">>  \t\tdir = xfrm_policy_id2dir(pol->index);\n"
+ ">> -\t\tif (pol->walk.dead ||\n"
+ ">> -\t\t    dir >= XFRM_POLICY_MAX ||\n"
+ ">> +\t\tif (dir >= XFRM_POLICY_MAX ||\n"
+ ">>  \t\t    pol->xdo.dev != dev)\n"
+ ">>  \t\t\tcontinue;\n"
+ ">\n"
+ ">Ditto.\n"
+ ">\n"
+ ">>  \n"
+ ">> -- \n"
+ ">> 2.25.1\n"
+ >>
 
-6b67e0ddf42911d3fe0a8d19cf0a582c5d638f0bb553c57b5583ecae3dc7fcd9
+cfed49eb4a9b7b67423f711541ab265964aa25f2e759c77da401696e4ef3ce0b

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