From c4c22d267117900b9582d5c2e934c107419c9603 Mon Sep 17 00:00:00 2001 From: Thomas Egerer Date: Mon, 27 Nov 2023 15:58:19 +0100 Subject: [PATCH 1/2] xfrm/sp: fix reference counters of sa selector/tmpl addresses It's a similar issue as in commit 3f4f1dda, when calling xfrmnl_sp_parse, the refcount of the addresses for selectors and templates increases to two, as xfrmnl_sel_set_[s|d]addr and xfrmnl_user_tmpl_set_[s|d]addr add another reference to the address object. As only one of those refs is dropped in sel_destroy or xfrmnl_user_tmpl_free respectively the address objects' refcount will never drop to zero, causing a leak. Signed-off-by: Thomas Egerer Fixes: 917154470895 ('xfrm: add xfrm support') --- lib/xfrm/sp.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/xfrm/sp.c b/lib/xfrm/sp.c index cf106517..81c3a0f7 100644 --- a/lib/xfrm/sp.c +++ b/lib/xfrm/sp.c @@ -596,6 +596,8 @@ int xfrmnl_sp_parse(struct nlmsghdr *n, struct xfrmnl_sp **result) addr = nl_addr_build (sp_info->sel.family, &sp_info->sel.daddr.a6, sizeof (sp_info->sel.daddr.a6)); nl_addr_set_prefixlen (addr, sp_info->sel.prefixlen_d); xfrmnl_sel_set_daddr (sp->sel, addr); + /* Drop the reference count from the above set operation */ + nl_addr_put(addr); xfrmnl_sel_set_prefixlen_d (sp->sel, sp_info->sel.prefixlen_d); if (sp_info->sel.family == AF_INET) @@ -604,6 +606,8 @@ int xfrmnl_sp_parse(struct nlmsghdr *n, struct xfrmnl_sp **result) addr = nl_addr_build (sp_info->sel.family, &sp_info->sel.saddr.a6, sizeof (sp_info->sel.saddr.a6)); nl_addr_set_prefixlen (addr, sp_info->sel.prefixlen_s); xfrmnl_sel_set_saddr (sp->sel, addr); + /* Drop the reference count from the above set operation */ + nl_addr_put(addr); xfrmnl_sel_set_prefixlen_s (sp->sel, sp_info->sel.prefixlen_s); xfrmnl_sel_set_dport (sp->sel, ntohs (sp_info->sel.dport)); @@ -680,6 +684,8 @@ int xfrmnl_sp_parse(struct nlmsghdr *n, struct xfrmnl_sp **result) else addr = nl_addr_build(tmpl->family, &tmpl->id.daddr.a6, sizeof (tmpl->id.daddr.a6)); xfrmnl_user_tmpl_set_daddr (sputmpl, addr); + /* Drop the reference count from the above set operation */ + nl_addr_put(addr); xfrmnl_user_tmpl_set_spi (sputmpl, ntohl(tmpl->id.spi)); xfrmnl_user_tmpl_set_proto (sputmpl, tmpl->id.proto); xfrmnl_user_tmpl_set_family (sputmpl, tmpl->family); @@ -689,6 +695,8 @@ int xfrmnl_sp_parse(struct nlmsghdr *n, struct xfrmnl_sp **result) else addr = nl_addr_build(tmpl->family, &tmpl->saddr.a6, sizeof (tmpl->saddr.a6)); xfrmnl_user_tmpl_set_saddr (sputmpl, addr); + /* Drop the reference count from the above set operation */ + nl_addr_put(addr); xfrmnl_user_tmpl_set_reqid (sputmpl, tmpl->reqid); xfrmnl_user_tmpl_set_mode (sputmpl, tmpl->mode); From 664f8f1bea7f3c46bdfcd637e694e2c3c627fa7b Mon Sep 17 00:00:00 2001 From: Thomas Egerer Date: Tue, 17 Oct 2023 11:10:26 +0000 Subject: [PATCH 2/2] xfrm: clear XFRM_SP_ATTR_TMPL when removing the last template from a policy Leaving XFRM_SP_ATTR_TMPL active in the mask may not impose a problem but, when removing the last template from a policy, the value signifying attached templates should be cleared. Signed-off-by: Thomas Egerer --- lib/xfrm/sp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/xfrm/sp.c b/lib/xfrm/sp.c index 81c3a0f7..5a96ac93 100644 --- a/lib/xfrm/sp.c +++ b/lib/xfrm/sp.c @@ -1358,6 +1358,8 @@ void xfrmnl_sp_remove_usertemplate(struct xfrmnl_sp *sp, struct xfrmnl_user_tmpl if (sp->ce_mask & XFRM_SP_ATTR_TMPL) { sp->nr_user_tmpl--; nl_list_del(&utmpl->utmpl_list); + if (sp->nr_user_tmpl == 0) + sp->ce_mask &= ~XFRM_SP_ATTR_TMPL; } }