netdev - Re: [PATCH net] calipso: Fix memory leak in netlbl_calipso_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhTiq1xPXXsETNKRBOtfkB5wohVwhBeae+5QW9uV-h5vvg@mail.gmail.com>
Date: Wed, 22 Nov 2023 10:56:57 -0500
From: Paul Moore <paul@...l-moore.com>
To: Gavrilov Ilia <Ilia.Gavrilov@...otecs.ru>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Huw Davies <huw@...eweavers.com>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, 
	"linux-security-module@...r.kernel.org" <linux-security-module@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"lvc-project@...uxtesting.org" <lvc-project@...uxtesting.org>
Subject: Re: [PATCH net] calipso: Fix memory leak in netlbl_calipso_add_pass()

On Wed, Nov 22, 2023 at 8:55 AM Gavrilov Ilia <Ilia.Gavrilov@...otecs.ru> wrote:
>
> If IPv6 support is disabled at boot (ipv6.disable=1),
> the calipso_init() -> netlbl_calipso_ops_register() function isn't called,
> and the netlbl_calipso_ops_get() function always returns NULL.
> In this case, the netlbl_calipso_add_pass() function allocates memory
> for the doi_def variable but doesn't free it with the calipso_doi_free().

It looks like a better option would be to return an error code in
netlbl_calipso_add() so we never allocate the memory in the first
place.

Untested patch below, copy-n-paste'd so there is likely whitespace
damage, but you get the idea.

diff --git a/net/netlabel/netlabel_calipso.c b/net/netlabel/netlabel_calipso.c
index f1d5b8465217..26a504dc6e57 100644
--- a/net/netlabel/netlabel_calipso.c
+++ b/net/netlabel/netlabel_calipso.c
@@ -54,8 +54,31 @@ static const struct nla_policy
calipso_genl_policy[NLBL_CALIPSO_A_MAX + 1] = {
       [NLBL_CALIPSO_A_MTYPE] = { .type = NLA_U32 },
};

+static const struct netlbl_calipso_ops *calipso_ops;
+
+/**
+ * netlbl_calipso_ops_register - Register the CALIPSO operations
+ * @ops: ops to register
+ *
+ * Description:
+ * Register the CALIPSO packet engine operations.
+ *
+ */
+const struct netlbl_calipso_ops *
+netlbl_calipso_ops_register(const struct netlbl_calipso_ops *ops)
+{
+       return xchg(&calipso_ops, ops);
+}
+EXPORT_SYMBOL(netlbl_calipso_ops_register);
+
+static const struct netlbl_calipso_ops *netlbl_calipso_ops_get(void)
+{
+       return READ_ONCE(calipso_ops);
+}
+
/* NetLabel Command Handlers
 */
+
/**
 * netlbl_calipso_add_pass - Adds a CALIPSO pass DOI definition
 * @info: the Generic NETLINK info block
@@ -100,10 +123,13 @@ static int netlbl_calipso_add(struct sk_buff
*skb, struct genl_info *info)
{
       int ret_val = -EINVAL;
       struct netlbl_audit audit_info;
+       const struct netlbl_calipso_ops *ops = netlbl_calipso_ops_get();

       if (!info->attrs[NLBL_CALIPSO_A_DOI] ||
           !info->attrs[NLBL_CALIPSO_A_MTYPE])
               return -EINVAL;
+       if (!ops)
+               return -EOPNOTSUPP;

       netlbl_netlink_auditinfo(&audit_info);
       switch (nla_get_u32(info->attrs[NLBL_CALIPSO_A_MTYPE])) {
@@ -363,28 +389,6 @@ int __init netlbl_calipso_genl_init(void)
       return genl_register_family(&netlbl_calipso_gnl_family);
}

-static const struct netlbl_calipso_ops *calipso_ops;
-
-/**
- * netlbl_calipso_ops_register - Register the CALIPSO operations
- * @ops: ops to register
- *
- * Description:
- * Register the CALIPSO packet engine operations.
- *
- */
-const struct netlbl_calipso_ops *
-netlbl_calipso_ops_register(const struct netlbl_calipso_ops *ops)
-{
-       return xchg(&calipso_ops, ops);
-}
-EXPORT_SYMBOL(netlbl_calipso_ops_register);
-
-static const struct netlbl_calipso_ops *netlbl_calipso_ops_get(void)
-{
-       return READ_ONCE(calipso_ops);
-}
-
/**
 * calipso_doi_add - Add a new DOI to the CALIPSO protocol engine
 * @doi_def: the DOI structure

-- 
paul-moore.com