Re: [PATCH net v3] net/iucv: take a private, writable frame before rewriting it in place

[Simon Horman]

On Wed, Oct 28, 2026 at 11:47:13PM -0500, Bryam Vargas wrote:
> afiucv_hs_rcv() rewrites the frame in place -- EBCASC() on four name
> fields, then afiucv_swap_src_dest() swaps them and pushes an Ethernet
> header -- and can hand the same skb to dev_queue_xmit(), without making
> it private first, so a packet socket that also gets the frame reads the
> rewritten names. An ETH_P_ALL tap runs first and leaves af_iucv a
> cloned skb; a socket opened for ETH_P_AF_IUCV sits on a ptype_specific
> list, walked after ptype_base[], so deliver_skb() takes a reference and
> af_iucv runs on an skb that is shared but not cloned.
> 
> Unshare, then cow, in that order: skb_cow() can reach
> pskb_expand_head(), which has BUG_ON(skb_shared()). skb_cow() tests
> skb_cloned(), which is what a rewrite of the data needs; skb_cow_head()
> tests only the header. Asking for ETH_HLEN also covers the unchecked
> push in afiucv_swap_src_dest(); on the ordinary path eth_type_trans()
> has already pulled that much, so it only compares.
> 
> Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
> Closes: https://sashiko.dev/#/patchset/20260813-b4-disp-60433a46-v1-1-509e1200533e@xxxxxxxxx?part=1
> Signed-off-by: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
> ---
> v3: skb_cow() instead of skb_cow_head(), as Paolo and the netdev AI
>     review asked: the rewrite is in the data, and skb_cow_head()'s
>     skb_header_cloned() test would leave it shared with a nohdr holder
>     (none reaches af_iucv today; still the wrong contract). One-word
>     change, rebased on net. v2 carried Alexandra's Reviewed-by; v3
>     changes the helper on Paolo's point, so it is not carried. Same
>     s390x QEMU A/B as v2, re-run on v3: same eight boots, same table.
>     Unpatched, the tap and the ETH_P_AF_IUCV socket both read the 48
>     name bytes in ASCII and a SYN with no listener never reaches the
>     socket; with this version every frame arrives as sent. Drop
>     skb_share_check() and the socket is broken again; drop skb_cow() and
>     both are.
> 
> v2: https://lore.kernel.org/all/20261008-b4-disp-bf46f1ba-v2-1-46ff1ea9c40b@xxxxxxxxx/
> v1: https://lore.kernel.org/all/20260815-b4-disp-dc82fde4-v1-0-e83b10b22ce9@xxxxxxxxx/

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>