Re: [PATCH net v2] net: Do not hold the reference for the same sk_rx_dst

On Sun, Mar 19, 2017 at 09:09:38PM -0700, Cong Wang wrote:

On Sat, Mar 18, 2017 at 9:03 PM, Kaiwen Xu [off-list ref] wrote:

quoted

On Sat, Mar 18, 2017 at 08:49:43PM -0700, Cong Wang wrote:

quoted

On Sat, Mar 18, 2017 at 6:48 PM, Kevin Xu [off-list ref] wrote:

quoted

In some rare cases, inet_sk_rx_dst_set() may be called multiple times
on the same dst, causing double refcounting. Eventually, it
prevents net_device to be destroyed. The bug manifested as

unregister_netdevice: waiting for lo to become free. Usage count = 1

in the kernel log, preventing new network namespace creation.

Signed-off-by: Kevin Xu <redacted>

Don't know why you don't follow the discussion on your v1...

It is protected by bh_lock_sock(), so your patch is not needed
at all.

Read net/ipv4/udp.c:

1762 /* For TCP sockets, sk_rx_dst is protected by socket lock
1763  * For UDP, we use xchg() to guard against concurrent changes.
1764  */

I probably misunderstood. Do you mean v2 patch is actually not needed or
the whole workaround is not necessary?

Your patch, no matter v1 or v2, is not needed because we use
bh_lock_sock() to serialize inet_sk_rx_dst_set(), unless you find
a case where we miss the bh_lock_sock(), but you don't say it in
your changelog. "some rare cases" is not enough to justify this bug.

I see, thanks for your explanation! I will try to dig in more to see if
I can find the root cause.