On Tue, Aug 8, 2023 at 3:07 PM Sishuai Gong <sishuai.system@xxxxxxxxx> wrote: > > Hello, > > We observed a data race over addr->valid between sctp_inet6addr_event() and > sctp_copy_laddrs(). Under the following execution order, sctp_copy_laddrs() > might copy the addr that is no longer valid. > > sctp_copy_laddrs() sctp_inet6addr_event() > if (!addr->valid) > continue; > addr->valid = 0; > memcpy(&temp, &addr->a, sizeof(temp)); > Yes, there's no lock protecting valid's read and write. But it doesn't seem to matter. Even if we put a lock there, after calling sctp_copy_laddrs() or sctp_copy_local_addr_list() this addr can still be deleted in NETDEV_DOWN event, which could happen anytime. Checking addr->valid can only ensure the addr is available at that moment, which doesn't look really helpful IMHO, considering an address can be deleted, or the same address as the one already deleted can be added anytime. addr->valid and addr>rcu were introduced to fix an use-after-free. But it seems to me that addr->rcu is enough for this purpose, we can actually deleted addr->valid.
