Commit 24980136 authored by Jakub Kicinski's avatar Jakub Kicinski

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

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: 50a896cf ("genetlink: properly support per-op policy dumping")
Link: https://lore.kernel.org/r/20220816161939.577583-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 5c23d6b7
...@@ -1174,13 +1174,17 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb) ...@@ -1174,13 +1174,17 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
op.policy, op.policy,
op.maxattr); op.maxattr);
if (err) if (err)
return err; goto err_free_state;
} }
} }
if (!ctx->state) if (!ctx->state)
return -ENODATA; return -ENODATA;
return 0; return 0;
err_free_state:
netlink_policy_dump_free(ctx->state);
return err;
} }
static void *ctrl_dumppolicy_prep(struct sk_buff *skb, static void *ctrl_dumppolicy_prep(struct sk_buff *skb,
......
...@@ -144,7 +144,7 @@ int netlink_policy_dump_add_policy(struct netlink_policy_dump_state **pstate, ...@@ -144,7 +144,7 @@ int netlink_policy_dump_add_policy(struct netlink_policy_dump_state **pstate,
err = add_policy(&state, policy, maxtype); err = add_policy(&state, policy, maxtype);
if (err) if (err)
return err; goto err_try_undo;
for (policy_idx = 0; for (policy_idx = 0;
policy_idx < state->n_alloc && state->policies[policy_idx].policy; 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, ...@@ -164,7 +164,7 @@ int netlink_policy_dump_add_policy(struct netlink_policy_dump_state **pstate,
policy[type].nested_policy, policy[type].nested_policy,
policy[type].len); policy[type].len);
if (err) if (err)
return err; goto err_try_undo;
break; break;
default: default:
break; break;
...@@ -174,6 +174,16 @@ int netlink_policy_dump_add_policy(struct netlink_policy_dump_state **pstate, ...@@ -174,6 +174,16 @@ int netlink_policy_dump_add_policy(struct netlink_policy_dump_state **pstate,
*pstate = state; *pstate = state;
return 0; 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 static bool
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment