| Message ID | 20210611082119.2117194-1-liushixin2@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [-next,v2] netlabel: Fix memory leak in netlbl_mgmt_add_common | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | fail | 1 blamed authors not CCed: jmorris@namei.org; 1 maintainers not CCed: jmorris@namei.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | fail | ERROR: "foo * bar" should be "foo *bar" |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/header_inline | success | Link |
On Fri, Jun 11, 2021 at 3:50 AM Liu Shixin <liushixin2@huawei.com> 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 <hulkci@huawei.com> > Signed-off-by: Liu Shixin <liushixin2@huawei.com> > --- > 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.
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:
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 <hulkci@huawei.com> Signed-off-by: Liu Shixin <liushixin2@huawei.com> --- 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(-)