(sorry to send this e-mail again, last mail is rejected by server due to non-acceptable content)
Florian Westphal [mailto:[email protected]] wrote: >Correct. This is broken since the central spin lock removal, since >nf_conntrack_lock no longer protects both get_next_corpse and >conntrack_confirm. > >Please send a patch, moving dying check after removal of conntrack from >the percpu list, Since unconfirmed conntrack is stored in unconfirmed-list which is per-cpu list and protected by per-cpu spin-lock, we can remove it from uncomfirmed-list and insert it into ct-hash-table separately. that is to say, we can remove it from uncomfirmed-list without holding corresponding hash-lock, then check if it is dying. if it is dying, we add it to the dying-list, then quit __nf_conntrack_confirm. we do this to follow the rules that the conntrack must alternatively at unconfirmed-list or dying-list when it is abort to be destroyed. >> 2. operation on ct->status should be atomic, because it race aginst >> get_next_corpse. > >Alternatively we could also get rid of the unconfirmed list handling in get_next_corpse, >it looks to me as if its simply not worth the trouble to also caring >about unconfirmed lists. yes, I think so. if there is a race at operating ct->status, there will be in alternative case: 1) IPS_DYING bit which set in get_next_corpse override other bits (e.g. IPS_SRC_NAT_DONE_BIT), or 2) other bits (e.g. IPS_SRC_NAT_DONE_BIT) which set in nf_nat_setup_info override IPS_DYING bit. but, any case seems to be okay.
fix_conntrack_confirm_race.patch
Description: Binary data
