On Fri, 15 Jun 2007, David Miller wrote: > From: "Ilpo_Järvinen" <[EMAIL PROTECTED]> > Date: Fri, 15 Jun 2007 16:07:37 +0300 (EEST) > > > Commit 6f74651ae626ec672028587bc700538076dfbefb is found guilty > > of breaking DSACK counting, which should be done only for the > > SACK block reported by the DSACK instead of every SACK block > > that is received along with DSACK information. > > > > Signed-off-by: Ilpo Järvinen <[EMAIL PROTECTED]> > > Thanks for performing such rigorous audits and finding > regressions like this!
They just come across, one thing leads to another, which again leads to another and so on... ...I was just trying to address the concerns you noted about tcp-2.6 patchset a while ago and came across other issues... There are still some things I must think carefully in sacktag processing since it does not validate start_seq and end_seq at all which can be abused currently at least in tcp-2.6. ...I would rather put end to the whole russian roulette in tcp-2.6 sacktag rather than fix/think individual cases and leave future modifications of it similarily hazardous. It's not very clear to me how to handle all possible cases of invalid SACK blocks though, perhaps TCP should just ignore such sack blocks without doing any processing based on them, i.e., ignore them whenever start_seq-end_seq does not fully fit to snd_una-snd_nxt (expect DSACK of course, which should be processed if it's between undo_marker-snd_nxt). Do you have any thoughts about this? It seems to me that net-2.6 is safe in this respect (probably just by accident) but the analysis wasn't that trivial, my main concern was tcp_fragment's pkt_len argument, if it ever becomes > skb->len, a boom would result (and perhaps zero has similar issues)! I couldn't find breakage analytically (but I could be wrong in it due to mind-boggling number of negations :-)) nor by bruteforcing seqno combinations near 0, skb->len and 2^31 wrap. > Patch applied. ...I think it should go to stable as well. -- i.
