Skip to content

Commit

Permalink
net: genl: fix error path memory leak in policy dumping
Browse files Browse the repository at this point in the history
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: 50a896c ("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>
  • Loading branch information
Jakub Kicinski committed Aug 18, 2022
1 parent 5c23d6b commit 2498013
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
6 changes: 5 additions & 1 deletion net/netlink/genetlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 12 additions & 2 deletions net/netlink/policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down

0 comments on commit 2498013

Please sign in to comment.