Commit 41c87425 authored by Jason A. Donenfeld's avatar Jason A. Donenfeld Committed by David S. Miller

netlink: do not set cb_running if dump's start() errs

It turns out that multiple places can call netlink_dump(), which means
it's still possible to dereference partially initialized values in
dump() that were the result of a faulty returned start().

This fixes the issue by calling start() _before_ setting cb_running to
true, so that there's no chance at all of hitting the dump() function
through any indirect paths.

It also moves the call to start() to be when the mutex is held. This has
the nice side effect of serializing invocations to start(), which is
likely desirable anyway. It also prevents any possible other races that
might come out of this logic.

In testing this with several different pieces of tricky code to trigger
these issues, this commit fixes all avenues that I'm aware of.
Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Reviewed-by: default avatarJohannes Berg <johannes@sipsolutions.net>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 6df4d17c
...@@ -2266,15 +2266,16 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb, ...@@ -2266,15 +2266,16 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
cb->min_dump_alloc = control->min_dump_alloc; cb->min_dump_alloc = control->min_dump_alloc;
cb->skb = skb; cb->skb = skb;
if (cb->start) {
ret = cb->start(cb);
if (ret)
goto error_unlock;
}
nlk->cb_running = true; nlk->cb_running = true;
mutex_unlock(nlk->cb_mutex); mutex_unlock(nlk->cb_mutex);
ret = 0;
if (cb->start)
ret = cb->start(cb);
if (!ret)
ret = netlink_dump(sk); ret = netlink_dump(sk);
sock_put(sk); sock_put(sk);
......
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