Re: [ovs-dev] [PATCH net-next v2 2/3] openvswitch: fix memory leak at failed datapath creation

2022-08-22 Thread Jakub Kicinski
On Mon, 22 Aug 2022 08:53:44 -0400 Aaron Conole wrote:
> Thanks for this patch.  I guess independent of this series, this patch
> should be applied to the net tree as well - it fixes an existing issue.

Yes, please, this needs to be reposted separately as [PATCH net].
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net-next v2 2/3] openvswitch: fix memory leak at failed datapath creation

2022-08-22 Thread Aaron Conole
Andrey Zhadchenko  writes:

> ovs_dp_cmd_new()->ovs_dp_change()->ovs_dp_set_upcall_portids()
> allocates array via kmalloc.
> If for some reason new_vport() fails during ovs_dp_cmd_new()
> dp->upcall_portids must be freed.
> Add missing kfree.
>
> Kmemleak example:
> unreferenced object 0x88800c382500 (size 64):
>   comm "dump_state", pid 323, jiffies 4294955418 (age 104.347s)
>   hex dump (first 32 bytes):
> 5e c2 79 e4 1f 7a 38 c7 09 21 38 0c 80 88 ff ff  ^.y..z8..!8.
> 03 00 00 00 0a 00 00 00 14 00 00 00 28 00 00 00  (...
>   backtrace:
> [<71bebc9f>] ovs_dp_set_upcall_portids+0x38/0xa0
> [<0187d8bd>] ovs_dp_change+0x63/0xe0
> [<2397e446>] ovs_dp_cmd_new+0x1f0/0x380
> [] genl_family_rcv_msg_doit+0xea/0x150
> [<8f583bc4>] genl_rcv_msg+0xdc/0x1e0
> [] netlink_rcv_skb+0x50/0x100
> [<4959cece>] genl_rcv+0x24/0x40
> [<4699ac7f>] netlink_unicast+0x23e/0x360
> [] netlink_sendmsg+0x24e/0x4b0
> [<6f4aa380>] sock_sendmsg+0x62/0x70
> [] sys_sendmsg+0x230/0x270
> [<12dacf7d>] ___sys_sendmsg+0x88/0xd0
> [<11776020>] __sys_sendmsg+0x59/0xa0
> [<2e8f2dc1>] do_syscall_64+0x3b/0x90
> [<3243e7cb>] entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> Fixes: b83d23a2a38b ("openvswitch: Introduce per-cpu upcall dispatch")
> Signed-off-by: Andrey Zhadchenko 
> ---

Thanks for this patch.  I guess independent of this series, this patch
should be applied to the net tree as well - it fixes an existing issue.

Acked-by: Aaron Conole 

I will try as well the other patches in the series.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH net-next v2 2/3] openvswitch: fix memory leak at failed datapath creation

2022-08-19 Thread Andrey Zhadchenko via dev
ovs_dp_cmd_new()->ovs_dp_change()->ovs_dp_set_upcall_portids()
allocates array via kmalloc.
If for some reason new_vport() fails during ovs_dp_cmd_new()
dp->upcall_portids must be freed.
Add missing kfree.

Kmemleak example:
unreferenced object 0x88800c382500 (size 64):
  comm "dump_state", pid 323, jiffies 4294955418 (age 104.347s)
  hex dump (first 32 bytes):
5e c2 79 e4 1f 7a 38 c7 09 21 38 0c 80 88 ff ff  ^.y..z8..!8.
03 00 00 00 0a 00 00 00 14 00 00 00 28 00 00 00  (...
  backtrace:
[<71bebc9f>] ovs_dp_set_upcall_portids+0x38/0xa0
[<0187d8bd>] ovs_dp_change+0x63/0xe0
[<2397e446>] ovs_dp_cmd_new+0x1f0/0x380
[] genl_family_rcv_msg_doit+0xea/0x150
[<8f583bc4>] genl_rcv_msg+0xdc/0x1e0
[] netlink_rcv_skb+0x50/0x100
[<4959cece>] genl_rcv+0x24/0x40
[<4699ac7f>] netlink_unicast+0x23e/0x360
[] netlink_sendmsg+0x24e/0x4b0
[<6f4aa380>] sock_sendmsg+0x62/0x70
[] sys_sendmsg+0x230/0x270
[<12dacf7d>] ___sys_sendmsg+0x88/0xd0
[<11776020>] __sys_sendmsg+0x59/0xa0
[<2e8f2dc1>] do_syscall_64+0x3b/0x90
[<3243e7cb>] entry_SYSCALL_64_after_hwframe+0x63/0xcd

Fixes: b83d23a2a38b ("openvswitch: Introduce per-cpu upcall dispatch")
Signed-off-by: Andrey Zhadchenko 
---
 net/openvswitch/datapath.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 8033c97a8d65..20c9964b74cc 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1804,7 +1804,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
ovs_dp_reset_user_features(skb, info);
}
 
-   goto err_unlock_and_destroy_meters;
+   goto err_destroy_pids;
}
 
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
@@ -1819,6 +1819,9 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
ovs_notify(_datapath_genl_family, reply, info);
return 0;
 
+err_destroy_pids:
+   if (rcu_dereference_raw(dp->upcall_portids))
+   kfree(rcu_dereference_raw(dp->upcall_portids));
 err_unlock_and_destroy_meters:
ovs_unlock();
ovs_meters_exit(dp);
-- 
2.31.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev