net: genl: fix error path memory leak in policy dumping

commit 249801360db3dec4f73768c502192020bfddeacc upstream.

If construction of the array of policies fails when recording
non-first policy we need to unwind.

netlink_policy_dump_add_policy() itself also needs fixing as
it currently gives up on error without recording the allocated
pointer in the pstate pointer.

Reported-by: syzbot+dc54d9ba8153b216cae0@syzkaller.appspotmail.com
Fixes: 50a896cf2d6f ("genetlink: properly support per-op policy dumping")
Link: https://lore.kernel.org/r/20220816161939.577583-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 1afca2a6c2ac15f5107ae48509e4b282ff1297c3..57010927e20a805b003d0e31d6e0b24f0207c799 100644 (file)
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1174,13 +1174,17 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
                                                             op.policy,
                                                             op.maxattr);
                        if (err)
-                               return err;
+                               goto err_free_state;
                }
        }
 
        if (!ctx->state)
                return -ENODATA;
        return 0;
+
+err_free_state:
+       netlink_policy_dump_free(ctx->state);
+       return err;
 }
 
 static void *ctrl_dumppolicy_prep(struct sk_buff *skb,

diff --git a/net/netlink/policy.c b/net/netlink/policy.c
index 8d7c900e27f4c5e8f31ed9759bd14ca0bd023a5d..87e3de0fde8963ec8ba967fe9c6610770b9e6475 100644 (file)
--- a/net/netlink/policy.c
+++ b/net/netlink/policy.c
@@ -144,7 +144,7 @@ int netlink_policy_dump_add_policy(struct netlink_policy_dump_state **pstate,
 
        err = add_policy(&state, policy, maxtype);
        if (err)
-               return err;
+               goto err_try_undo;
 
        for (policy_idx = 0;
             policy_idx < state->n_alloc && state->policies[policy_idx].policy;
@@ -164,7 +164,7 @@ int netlink_policy_dump_add_policy(struct netlink_policy_dump_state **pstate,
                                                 policy[type].nested_policy,
                                                 policy[type].len);
                                if (err)
-                                       return err;
+                                       goto err_try_undo;
                                break;
                        default:
                                break;
@@ -174,6 +174,16 @@ int netlink_policy_dump_add_policy(struct netlink_policy_dump_state **pstate,
 
        *pstate = state;
        return 0;
+
+err_try_undo:
+       /* Try to preserve reasonable unwind semantics - if we're starting from
+        * scratch clean up fully, otherwise record what we got and caller will.
+        */
+       if (!*pstate)
+               netlink_policy_dump_free(state);
+       else
+               *pstate = state;
+       return err;
 }
 
 static bool