On 09/06/2017 06:24 PM, Daniel Borkmann wrote: [...]
Otherwise it looks good, but note that it also doesn't really resolve the issue you mention wrt stale map pointers by the way. This would need a different way to clear out the pointers from redirect_info, I'm thinking when we have devmap dismantle time after RCU grace period we should check whether there are still stale pointers from this map around and clear them under disabled preemption, but need to brainstorm a bit more on that first.
Scratch that approach, doesn't work. So thinking bit more on this, what we could do here is the following: verifier knows we called bpf_xdp_redirect_map() helper, so it could do a small insn rewrite in the sense that it fills R4 with a pointer to the bpf_prog. We have that at verification time anyway and R4 is allowed to be populated since we scratch it per convention. Then, the helper would store the prog pointer in struct redirect_info. Later in xdp_do_*_redirect() we check whether the redirect_info's prog pointer is the same as passed xdp_prog pointer, and if that's the case then all good, since the prog holds a ref on the map anyway, if they are not equal in the unlikely case, it means stale pointer, so we bail out right there. That would work imo, will see to code it up and check it out.