Re: [PATCH net 2/2] net/iucv: do not count the SHT frame against the receive window

[Bryam Vargas]

On Tue, 27 Oct 2026 02:50:25 +0000, netdev-bot+sashiko@xxxxxxxxxx wrote:
> [Severity: High]
> Can an empty frame read after the over-window close reach
> iucv_send_ctrl() while iucv->hs_dev is NULL?

Yes. The close purges backlog_skb_q, not sk_receive_queue, msg_recv
stays over the limit, and with the length test an empty frame read
after it skips the -EFAULT branch and lands in the window update. Repro
on the emulated s390x setup with SO_MSGLIMIT 2: the peer sends twice
(the window), then from a tap on its interface one data frame over the
window, its SHUT_WR for the SHT, and 16 more data frames behind it, so
backlog_skb_q is not empty and no read sends a window update before
the close (a Linux peer blocks at the window; the overrun is injected).
Six reads: 1, 1, -EFAULT (the close), -ECONNRESET, 0 (the SHT),
-EFAULT (a data frame behind the SHT closes again). On v1 the fifth
read enters iucv_send_ctrl() once, with nothing leaving the adapter.
The reads are the same with and without the bug: on a default boot the
NULL read lands in the lowcore and afiucv_hs_send() fails with -ENODEV,
as you say, so nothing shows from user space. v2 sends the update only
while the socket is connected; the same run then shows no attempt
after the close.

> [Severity: Medium]
> This is a pre-existing issue, but does a zero-length data frame always
> arrive at the receiver as an empty data frame?

No. With a credit piggybacked it is a window update to afiucv_hs_rcv()
and never reaches recvmsg(). v2 narrows the sentence to "a zero-length
data frame that reaches recvmsg()". The slot the sender loses there is
the tx side's own accounting, not this patch's.

> [Severity: Medium]
> This is a pre-existing issue, but the classic IUCV transport still has a
> WARN that a peer can trigger, of the same kind as the one removed here.

Same kind, agreed, and iucv_process_message_q() retries the oversize
message on every read. That is the z/VM transport, not this path; out
of scope here.

> [Severity: High]
> This isn't a bug introduced by this patch, but this read-time check is
> the only window enforcement on the receive side. Can a peer that ignores
> its window grow iucv->backlog_skb_q without limit?

Today it can: nothing on arrival counts against the window, and
backlog_skb_q is neither bounded nor charged. This patch takes a WARN
off that check and does not move the enforcement; the bound belongs on
the rx side at arrival and is not this patch's.

> [Severity: Medium]
> This is a pre-existing issue, but can a conforming peer still reach this
> close?

Yes, by lowering SO_MSGLIMIT in IUCV_BOUND below the window the SYN
advertised; that is the case in the notes below the patch. Not in the
commit message or the comment: 2/2 only stops counting the SHT and
takes the WARN off, and the socket enforcing a window other than the
one it advertised is the series' fix. The locking series keeps the
advertised window in its own field, so the budget, this check and the
update threshold read the same number.

> [Severity: Low]
> Two unsynchronized writes to sk->sk_shutdown can lose RCV_SHUTDOWN.

Pre-existing; the series puts both writes under the same lock.

Thanks,
Bryam

pw-bot: cr