</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? > > Does that idea sound right? If not what piece I'm still missing in puzzle? > > I think you're assuming that the "softnet" code path is running in a > thread which is not (yet) the case. yes, my context is not fully switched from Solaris to OpenBSD yet... thanks and regards sasha