On 27/08/15(Thu) 14:19, Alexandr Nedvedicky wrote: > </large snip> > > > > Assuming the locking in MULTIPROCESSOR goes like: > > > interrupt grabs splsoftnet -> ip_input -> PF grabs KERNEL_LOCK() > > > We need to take care of ioctl() call path and purge thread. Those need to > > > get synchronize with packets using KERNEL_LOCK(). They should not to mess > > > with > > > splsoftnet() any more in MULTIPROCESSOR version. > > > > As long as we are using (soft-)interrupts to process incoming packets > > they should. > > > > If you don't raise the SPL level to softnet while CPU0 is executing some > > code in ioctl() path and it takes an interrupt at this point it might end > > up executing ip_input() *before* returning to the ioctl() (process) > > context. This scenario might corrupt the states you're trying to protect. > > > > just to verify I got everything right here. I assume I've got MULTIPROCESSOR > kernel with my broken patch, which basically trades or splfotnet() locks for > KERNEL_LOCKS()... > > We have ioctl() operation, which attempts to insert a rule into a global > ruleset, ioctl() grabs KERNEL_LOCK() only and gets to work. (yes my broken > patch does not grab splsoftnet()). > > while ioctl() is working there is interrupt, which delivers packet from NIC. > since no one holds splsoftnet() on CPU packet is free to go and enters > firewall (pf_test()). And that's very bad, interrupt tries to grab > KERNEL_LOCK(), which is still held by ioctl() packet has just interrupted. One > of the two bad things will happen: > > a) deadlock, we will be waiting for KERNEL_LOCK(), which is still held > by interrupted ioctl() operation. > > b) since KERNEL_LOCK() is recursive, the pf_test() will be free to > go possibly finding a list of rules in inconsistent state. In this > case no deadlock happens, but PF plays game with pointers... > > as Kettenis pointed out the KERNEL_LOCK() is recursive, I have an itchy > feeling > we are taking b) option. Is that right?
Yep :)
