Re: [PATCH net] calipso: Fix memory leak in netlbl_calipso_add_pass()

[Paul Moore]

On Wed, Nov 22, 2023 at 8:55 AM Gavrilov Ilia <Ilia.Gavrilov@xxxxxxxxxxx> 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