net: three TCP receive bugs - caller corruption, data loss, early EOF

Found by writing nc(1): a 96-byte receive buffer against a host that
answers in 200-byte segments hung the process every time.

- recv/recv_nb copied RXLEN bytes into the caller's buffer and never
  looked at the caller's maxlen, so any short buffer got overrun and its
  stack smashed. Deliver min(RXLEN, maxlen) and keep the tail in RXBUF
  for the next call. wget only survived because its buffer (220) happens
  to exceed our MSS.
- tcp_buffer_data overwrote RXBUF from offset 0 on every segment, so a
  segment arriving before the app drained the previous one destroyed
  unread bytes - silent, undetectable data loss. Append at RXBUF+RXLEN
  instead, and drop (without advancing rcv_nxt) when it doesn't fit, so
  the peer retransmits once there's room. RXLEN is one byte, so 255 is
  the capacity that matters, not RXBUF's 288.
- A draining recv now clears RXLEN as well as HASRX: RXLEN is the append
  offset, and a stale one made every later segment look too big to fit,
  stalling the connection until the peer gave up.
- A FIN past a gap is not end-of-stream. tcp_in accepted any FIN whose
  segment carried no data, without a sequence check, marking the socket
  DONE and making recv report EOF at the hole - truncating a transfer
  right before its last segments. Decide in-order-ness once, before
  rcv_nxt moves, and re-ACK an out-of-order FIN instead.

Verified against a 6300-byte reply received while sending 831 bytes:
byte-exact (6339 = payload + banner, 303 lines, first/mid/last intact),
plus no regression in ping / nslookup / wget / irc.

1 files changed, 137 insertions, 14 deletions

diff --git a/src/socket.asm b/src/socket.asm
index a9e9ef3..d0821a8 100644
--- a/src/socket.asm
+++ b/src/socket.asm
@@ -74,6 +74,9 @@ wConHead::    DS 1
 wConTail::    DS 1
 wNetUdpPort::  DS 2      ; scratch: UDP dst port being demuxed
 wNetTO::       DS 3      ; recv timeout counter (net_pump clobbers registers)
+wNetRxRem::    DS 1      ; recv: bytes left in RXBUF after a clamped delivery
+wNetRxOff::    DS 1      ; tcp_buffer_data: append offset (unread bytes)
+wNetTcpInOrd:: DS 1      ; tcp_in: did this segment start exactly at rcv_nxt?
 wNetTcpFlags:: DS 1      ; TCP flags for the segment being built
 wNetTcpDlen::  DS 1      ; TCP payload length for the segment being built
 wNetTcpIn::    DS 1      ; incoming TCP flags
@@ -530,7 +533,12 @@ net_op_recv:
     xor a                       ; connection closed, no data -> 0 bytes (EOF)
     ret
 .got
-    ; copy sock.RXBUF -> process NR_BUF, len = sock.RXLEN, NR_IP = sock.SRCIP
+    ; copy sock.RXBUF -> process NR_BUF, len = min(sock.RXLEN, caller maxlen).
+    ;
+    ; The clamp is not optional: a TCP segment can be up to 208 bytes here, so
+    ; copying RXLEN blindly overran any caller whose buffer was smaller and
+    ; silently smashed its locals/stack (a 96-byte buffer + a 200-byte reply =
+    ; a hung process). Deliver what fits, keep the tail for the next recv.
     call sock_ptr_hl            ; HL = &sock
     push hl
     ld a, l
@@ -540,8 +548,16 @@ net_op_recv:
     adc 0
     ld h, a
     ld a, [hl]                  ; RXLEN
-    ld c, a                     ; C = length
+    ld c, a                     ; C = bytes buffered
+    ld b, a                     ; B = keep the untruncated total
     pop hl
+    ld a, [wNetReq+NR_LEN]      ; caller's maxlen
+    or a
+    jr z, .toosmall             ; maxlen 0: refuse rather than corrupt
+    cp c
+    jr nc, .fits                ; maxlen >= buffered -> deliver it all
+    ld c, a                     ; else deliver exactly maxlen
+.fits
     ; source IP -> wNetReq.NR_IP (returned to caller via buffer? no - copy to caller req is skipped; give src via NR_IP in kernel copy only)
     push bc
     push hl
@@ -567,8 +583,23 @@ net_op_recv:
     jr nz, .cpl
 .nocopy
     pop hl                      ; &sock
-    pop bc                      ; C = length
-    ; clear HASRX
+    pop bc                      ; B = bytes buffered, C = bytes delivered
+    ld a, b
+    sub c
+    jr nz, .partial             ; caller's buffer was too small for all of it
+    ; fully drained: clear HASRX *and* RXLEN. RXLEN is the append offset for
+    ; the next segment (tcp_buffer_data), so leaving it stale makes every
+    ; later segment look too big to fit and the connection stalls out.
+    push hl
+    ld a, l
+    add SK_RXLEN
+    ld l, a
+    ld a, h
+    adc 0
+    ld h, a
+    xor a
+    ld [hl], a
+    pop hl
     ld a, l
     add SK_HASRX
     ld l, a
@@ -579,6 +610,46 @@ net_op_recv:
     ld [hl], a
     ld a, c                     ; return length
     ret
+.toosmall
+    ld a, $FE                   ; nothing delivered, nothing consumed
+    ret
+.partial
+    ; A = leftover bytes. Shift them down to the front of RXBUF and leave
+    ; HASRX set, so the next recv() returns the rest of the segment.
+    ld [wNetRxRem], a
+    ld a, l
+    add SK_RXBUF
+    ld l, a
+    ld a, h
+    adc 0
+    ld h, a                     ; HL = &RXBUF
+    ld d, h
+    ld e, l                     ; DE = dst = &RXBUF
+    ld a, l
+    add c
+    ld l, a
+    ld a, h
+    adc 0
+    ld h, a                     ; HL = src = &RXBUF + delivered
+    ld a, [wNetRxRem]
+    ld b, a                     ; B = leftover count (1..255)
+.shift
+    ld a, [hl+]
+    ld [de], a
+    inc de
+    dec b
+    jr nz, .shift
+    call sock_ptr_hl
+    ld a, l
+    add SK_RXLEN
+    ld l, a
+    ld a, h
+    adc 0
+    ld h, a
+    ld a, [wNetRxRem]
+    ld [hl], a                  ; RXLEN = leftover (HASRX stays set)
+    ld a, [wNetReq+NR_LEN]      ; delivered exactly maxlen
+    ret
 
 ; sock_ptr_hl -> HL = current socket pointer (from wNetSockPtr)
 sock_ptr_hl:
@@ -1764,31 +1835,59 @@ tcp_close:
     ld [hl], a                  ; SK_TYPE = free
     ret
 
-; tcp_buffer_data - store the current segment's payload into the socket rx slot
+; tcp_rx_fits -> CF set if this segment cannot be buffered without destroying
+; data the application has not read yet. RXLEN is a single byte, so the usable
+; capacity is 255 (RXBUF itself is 288) - any total that carries out of 8 bits
+; does not fit. The caller then drops the segment and re-ACKs its old rcv_nxt,
+; so the peer retransmits once we have drained. Dropping is correct TCP and
+; cannot deadlock; silently overwriting unread bytes (what we used to do) is
+; data corruption the application can never detect.
+tcp_rx_fits:
+    call sock_ptr_hl
+    ld a, l
+    add SK_RXLEN
+    ld l, a
+    ld a, h
+    adc 0
+    ld h, a
+    ld c, [hl]                  ; C = bytes still unread
+    ld a, [wNetTcpDlen2]
+    cp 209
+    jr c, .len
+    ld a, 208
+.len
+    add c                       ; carry => unread + new > 255: no room
+    ret
+
+; tcp_buffer_data - append the current segment's payload to the socket rx slot
+; (the app may still owe us a read of an earlier segment; see tcp_rx_fits).
 tcp_buffer_data:
     call sock_ptr_hl
     ld a, l
-    add SK_HASRX
+    add SK_RXLEN
     ld l, a
     ld a, h
     adc 0
     ld h, a
-    ld a, 1
-    ld [hl], a
+    ld a, [hl]
+    ld [wNetRxOff], a           ; append at the end of the unread bytes
     ld a, [wNetTcpDlen2]
     cp 209
     jr c, .oklen
     ld a, 208
 .oklen
     ld c, a
+    ld a, [wNetRxOff]
+    add c
+    ld [hl], a                  ; RXLEN = unread + new
     call sock_ptr_hl
     ld a, l
-    add SK_RXLEN
+    add SK_HASRX
     ld l, a
     ld a, h
     adc 0
     ld h, a
-    ld a, c
+    ld a, 1
     ld [hl], a
     call sock_ptr_hl
     ld a, l
@@ -1797,8 +1896,12 @@ tcp_buffer_data:
     ld a, h
     adc 0
     ld h, a
-    ld d, h
-    ld e, l                     ; DE = dest
+    ld a, [wNetRxOff]
+    add l
+    ld e, a
+    ld a, 0
+    adc h
+    ld d, a                     ; DE = dest = &RXBUF + unread
     ld a, [wNetTcpDoff]
     ld l, a
     ld h, 0
@@ -1901,13 +2004,26 @@ tcp_in:
     ld a, b
     sub c
     ld [wNetTcpDlen2], a
+    ; Decide in-order-ness ONCE, here, before rcv_nxt moves: the FIN check
+    ; below needs the answer too, and by then rcv_nxt has advanced past this
+    ; segment's seq so re-testing would always say "out of order".
+    call sk_seq_in_order
+    ld a, 0
+    jr nz, .oo
+    inc a
+.oo
+    ld [wNetTcpInOrd], a
+    ld a, [wNetTcpDlen2]
     or a
     jr z, .checkfin
     ; Only accept data that starts exactly at rcv_nxt. A dup/out-of-order
     ; segment is dropped, but re-ACKed with our current rcv_nxt so the peer
     ; converges instead of retransmitting forever.
-    call sk_seq_in_order
-    jr nz, .reack
+    ld a, [wNetTcpInOrd]
+    or a
+    jr z, .reack
+    call tcp_rx_fits
+    jr c, .reack                ; no room for it yet: make the peer resend
     call tcp_buffer_data
     ld a, [wNetTcpDlen2]
     call sk_add_rcv
@@ -1920,6 +2036,13 @@ tcp_in:
     ld a, [wNetTcpIn]
     and TF_FIN
     ret z
+    ; A FIN that sits past a gap is NOT end-of-stream: accepting it here (we
+    ; used to, unconditionally) marks the socket DONE and recv reports EOF,
+    ; silently truncating the transfer at the hole. Re-ACK instead, so the
+    ; peer retransmits what we missed and the real FIN arrives in order.
+    ld a, [wNetTcpInOrd]
+    or a
+    jr z, .reack
     ld a, 1
     call sk_add_rcv
     ld a, TF_ACK