On Thu, Apr 22, 2010 at 06:02:08PM +0200, Eric Dumazet wrote: > Le jeudi 22 avril 2010 à 08:51 -0700, Paul E. McKenney a écrit : > > On Thu, Apr 22, 2010 at 04:53:49PM +0200, Eric Dumazet wrote: > > > Le jeudi 22 avril 2010 à 16:36 +0200, Eric Dumazet a écrit : > > > > > > > If one hash slot is under attack, then there is a bug somewhere. > > > > > > > > If we cannot avoid this, we can fallback to a secure mode at the second > > > > retry, and take the spinlock. > > > > > > > > Tis way, most of lookups stay lockless (one pass), and some might take > > > > the slot lock to avoid the possibility of a loop. > > > > > > > > I suspect a bug elsewhere, quite frankly ! > > > > > > > > We have a chain that have an end pointer that doesnt match the expected > > > > one. > > > > > > > > > > On normal situation, we always finish the lookup : > > > > > > 1) If we found the thing we were looking at. > > > > > > 2) We get the list end (item not found), we then check if it is the > > > expected end. > > > > > > It is _not_ the expected end only if some writer deleted/inserted an > > > element in _this_ chain during our lookup. > > > > So this situation uses SLAB_DESTROY_BY_RCU to quickly recycle deleted > > elements? (Not obvious from the code, but my ignorance of the networking > > code is such that many things in that part of the kernel are not obvious > > to me, I am afraid.) > > Yes, this uses SLAB_DESTROY_BY_RCU, like tcp/udp lookups. OK, that will do it!!! ;-) One way of throttling the bad effects of updates on readers is to periodically force updates through a grace period. But this seems to be a very big hammer, and likely to have little practical effect. Another approach would be to have multiple list pointers per element, so that a given element could be reused a small number of times without messing up concurrent readers (sort of like Herbert's resizable hash table). But as you say, if some other bug is really behind this, better to fix that bug than to work around it. > > Otherwise, of course you would simply allow deleted elements to continue > > pointing where they did previously, so that concurrent readers would not > > miss anything. > > > Of course, the same potential might arise on insertion, but it is usually > > OK to miss an element that was inserted after you started searching. > > > > > Because our lookup is lockless, we then have to redo it because we might > > > miss the object we are looking for. > > > > Ah... Is there also a resize operation? Herbert did do a resizable > > hash table recently, but I was under the impression that (1) it was in > > some other part of the networking stack and (2) it avoided the need to > > restart readers. > > > > > If we can do the 'retry' a 10 times, it means the attacker was really > > > clever enough to inject new packets (new conntracks) at the right > > > moment, in the right hash chain, and this sounds so higly incredible > > > that I cannot believe it at all :) > > > > Or maybe the DoS attack is injecting so many new conntracks that a large > > fraction of the hash chains are being modified at any given time? > > maybe hash table has one slot :) ;-) ;-) ;-) Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
