netdev - Re: [PATCH] tcp: make urg+gso work for real this time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081219195954.GA11899@gondor.apana.org.au>
Date:	Sat, 20 Dec 2008 06:59:54 +1100
From:	Herbert Xu <herbert@...dor.apana.org.au>
To:	Alexey Kuznetsov <kuznet@....inr.ac.ru>
Cc:	Petr Tesarik <ptesarik@...e.cz>,
	Ilpo J??rvinen <ilpo.jarvinen@...sinki.fi>,
	davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH] tcp: make urg+gso work for real this time

On Fri, Dec 19, 2008 at 04:46:44PM +0300, Alexey Kuznetsov wrote:
>
> BSD snd_nxt points to current transmission head,
> which is rewinded to snd_una when it starts to retransmit.

Right, but it would seem that it's broken for sack retransmits
at least.  This is from NetBSD but the others are similar:

	/*
	 * If we are doing retransmissions, then snd_nxt will
	 * not reflect the first unsent octet.  For ACK only
	 * packets, we do not want the sequence number of the
	 * retransmitted packet, we want the sequence number
	 * of the next unsent octet.  So, if there is no data
	 * (and no SYN or FIN), use snd_max instead of snd_nxt
	 * when filling in ti_seq.  But if we are in persist
	 * state, snd_max might reflect one byte beyond the
	 * right edge of the window, so use snd_nxt in that
	 * case, since we know we aren't doing a retransmission.
	 * (retransmit and persist are mutually exclusive...)
	 */
	if (TCP_SACK_ENABLED(tp) && sack_rxmit) {
		th->th_seq = htonl(p->rxmit);
		p->rxmit += len;
	} else {
		if (len || (flags & (TH_SYN|TH_FIN)) ||
		    TCP_TIMER_ISARMED(tp, TCPT_PERSIST))
			th->th_seq = htonl(tp->snd_nxt);
		else
			th->th_seq = htonl(tp->snd_max);
	}
 
> BTW, that line of code which I sent was logically (not practically) wrong,
> Dave spotted this instantly. :-) Logically correct way would be
> to check against future snd_nxt which will be advanced
> after the packet is submitted. Practically, it does not matter,
> the difference is only for jumbo frames, which are inhibited
> for tcp.

Actually I think the unadvanced snd_nxt makes sense too.  What we're
asking there is has the urgent data been transmitted before, i.e.,
is the current skb a retransmit (since we've already established
that the urgent pointer is forward of the current packet).

Yes this would be broken if we ever do jumbo frames, but it'd
be broken for the same reason that the current test'd be brokens,
that is, we need to split the packet in urgent mode.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html