Hulk Robot reported memory leak in netlbl_mgmt_add_common.
The problem is non-freed map in case of netlbl_domhsh_add() failed.
BUG: memory leak
unreferenced object 0xffff888100ab7080 (size 96):
comm "syz-executor537", pid 360, jiffies 4294862456 (age 22.678s)
hex dump (first 32 bytes):
05 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
fe 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 ................
backtrace:
[<0000000008b40026>] netlbl_mgmt_add_common.isra.0+0xb2a/0x1b40
[<000000003be10950>] netlbl_mgmt_add+0x271/0x3c0
[<00000000c70487ed>] genl_family_rcv_msg_doit.isra.0+0x20e/0x320
[<000000001f2ff614>] genl_rcv_msg+0x2bf/0x4f0
[<0000000089045792>] netlink_rcv_skb+0x134/0x3d0
[<0000000020e96fdd>] genl_rcv+0x24/0x40
[<0000000042810c66>] netlink_unicast+0x4a0/0x6a0
[<000000002e1659f0>] netlink_sendmsg+0x789/0xc70
[<000000006e43415f>] sock_sendmsg+0x139/0x170
[<00000000680a73d7>] ____sys_sendmsg+0x658/0x7d0
[<0000000065cbb8af>] ___sys_sendmsg+0xf8/0x170
[<0000000019932b6c>] __sys_sendmsg+0xd3/0x190
[<00000000643ac172>] do_syscall_64+0x37/0x90
[<000000009b79d6dc>] entry_SYSCALL_64_after_hwframe+0x44/0xae
Fixes: 63c416887437 ("netlabel: Add network address selectors to the NetLabel/LSM domain mapping")
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Liu Shixin <[email protected]>
---
v1->v2: According to Dongliang's and Paul's advices, simplify the code.
net/netlabel/netlabel_mgmt.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
index e664ab990941..fa9e68e5f826 100644
--- a/net/netlabel/netlabel_mgmt.c
+++ b/net/netlabel/netlabel_mgmt.c
@@ -76,6 +76,7 @@ static const struct nla_policy netlbl_mgmt_genl_policy[NLBL_MGMT_A_MAX + 1] = {
static int netlbl_mgmt_add_common(struct genl_info *info,
struct netlbl_audit *audit_info)
{
+ void * pmap = NULL;
int ret_val = -EINVAL;
struct netlbl_domaddr_map *addrmap = NULL;
struct cipso_v4_doi *cipsov4 = NULL;
@@ -175,6 +176,8 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
ret_val = -ENOMEM;
goto add_free_addrmap;
}
+
+ pmap = map;
map->list.addr = addr->s_addr & mask->s_addr;
map->list.mask = mask->s_addr;
map->list.valid = 1;
@@ -183,14 +186,13 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
map->def.cipso = cipsov4;
ret_val = netlbl_af4list_add(&map->list, &addrmap->list4);
- if (ret_val != 0) {
- kfree(map);
- goto add_free_addrmap;
- }
+ if (ret_val != 0)
+ goto add_free_map;
entry->family = AF_INET;
entry->def.type = NETLBL_NLTYPE_ADDRSELECT;
entry->def.addrsel = addrmap;
+
#if IS_ENABLED(CONFIG_IPV6)
} else if (info->attrs[NLBL_MGMT_A_IPV6ADDR]) {
struct in6_addr *addr;
@@ -223,6 +225,8 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
ret_val = -ENOMEM;
goto add_free_addrmap;
}
+
+ pmap = map;
map->list.addr = *addr;
map->list.addr.s6_addr32[0] &= mask->s6_addr32[0];
map->list.addr.s6_addr32[1] &= mask->s6_addr32[1];
@@ -235,10 +239,8 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
map->def.calipso = calipso;
ret_val = netlbl_af6list_add(&map->list, &addrmap->list6);
- if (ret_val != 0) {
- kfree(map);
- goto add_free_addrmap;
- }
+ if (ret_val != 0)
+ goto add_free_map;
entry->family = AF_INET6;
entry->def.type = NETLBL_NLTYPE_ADDRSELECT;
@@ -248,10 +250,12 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
ret_val = netlbl_domhsh_add(entry, audit_info);
if (ret_val != 0)
- goto add_free_addrmap;
+ goto add_free_map;
return 0;
+add_free_map:
+ kfree(pmap);
add_free_addrmap:
kfree(addrmap);
add_doi_put_def:
--
2.18.0.huawei.25
On Fri, Jun 11, 2021 at 3:50 AM Liu Shixin <[email protected]> wrote:
>
> Hulk Robot reported memory leak in netlbl_mgmt_add_common.
> The problem is non-freed map in case of netlbl_domhsh_add() failed.
>
> BUG: memory leak
> unreferenced object 0xffff888100ab7080 (size 96):
> comm "syz-executor537", pid 360, jiffies 4294862456 (age 22.678s)
> hex dump (first 32 bytes):
> 05 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> fe 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 ................
> backtrace:
> [<0000000008b40026>] netlbl_mgmt_add_common.isra.0+0xb2a/0x1b40
> [<000000003be10950>] netlbl_mgmt_add+0x271/0x3c0
> [<00000000c70487ed>] genl_family_rcv_msg_doit.isra.0+0x20e/0x320
> [<000000001f2ff614>] genl_rcv_msg+0x2bf/0x4f0
> [<0000000089045792>] netlink_rcv_skb+0x134/0x3d0
> [<0000000020e96fdd>] genl_rcv+0x24/0x40
> [<0000000042810c66>] netlink_unicast+0x4a0/0x6a0
> [<000000002e1659f0>] netlink_sendmsg+0x789/0xc70
> [<000000006e43415f>] sock_sendmsg+0x139/0x170
> [<00000000680a73d7>] ____sys_sendmsg+0x658/0x7d0
> [<0000000065cbb8af>] ___sys_sendmsg+0xf8/0x170
> [<0000000019932b6c>] __sys_sendmsg+0xd3/0x190
> [<00000000643ac172>] do_syscall_64+0x37/0x90
> [<000000009b79d6dc>] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Fixes: 63c416887437 ("netlabel: Add network address selectors to the NetLabel/LSM domain mapping")
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Liu Shixin <[email protected]>
> ---
> v1->v2: According to Dongliang's and Paul's advices, simplify the code.
>
> net/netlabel/netlabel_mgmt.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
> index e664ab990941..fa9e68e5f826 100644
> --- a/net/netlabel/netlabel_mgmt.c
> +++ b/net/netlabel/netlabel_mgmt.c
> @@ -76,6 +76,7 @@ static const struct nla_policy netlbl_mgmt_genl_policy[NLBL_MGMT_A_MAX + 1] = {
> static int netlbl_mgmt_add_common(struct genl_info *info,
> struct netlbl_audit *audit_info)
> {
> + void * pmap = NULL;
You should use the 'void *pmap = NULL;' style that is used in the rest
of this function, and most everywhere in the kernel.
> int ret_val = -EINVAL;
> struct netlbl_domaddr_map *addrmap = NULL;
> struct cipso_v4_doi *cipsov4 = NULL;
> @@ -175,6 +176,8 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> ret_val = -ENOMEM;
> goto add_free_addrmap;
> }
> +
> + pmap = map;
There is no need for the extra vertical whitespace here.
> map->list.addr = addr->s_addr & mask->s_addr;
> map->list.mask = mask->s_addr;
> map->list.valid = 1;
> @@ -183,14 +186,13 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> map->def.cipso = cipsov4;
>
> ret_val = netlbl_af4list_add(&map->list, &addrmap->list4);
> - if (ret_val != 0) {
> - kfree(map);
> - goto add_free_addrmap;
> - }
> + if (ret_val != 0)
> + goto add_free_map;
>
> entry->family = AF_INET;
> entry->def.type = NETLBL_NLTYPE_ADDRSELECT;
> entry->def.addrsel = addrmap;
> +
Please don't add extra vertical whitespace here.
> #if IS_ENABLED(CONFIG_IPV6)
> } else if (info->attrs[NLBL_MGMT_A_IPV6ADDR]) {
> struct in6_addr *addr;
> @@ -223,6 +225,8 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> ret_val = -ENOMEM;
> goto add_free_addrmap;
> }
> +
> + pmap = map;
Same thing, no extra vertical whitespace please.
--
paul moore
http://www.paul-moore.com