Re: [PATCH net] net/iucv: fix use-after-free of a severed iucv_path

[Bryam Vargas]

To:  Alexandra Winter <wintera@xxxxxxxxxxxxx>
Cc:  Thorsten Winkler <twinkler@xxxxxxxxxxxxx>,
     Hidayath Khan <hidayath@xxxxxxxxxxxxx>,
     Heiko Carstens <hca@xxxxxxxxxxxxx>,
     Vasily Gorbik <gor@xxxxxxxxxxxxx>,
     Alexander Gordeev <agordeev@xxxxxxxxxxxxx>,
     Paolo Abeni <pabeni@xxxxxxxxxx>,
     "David S. Miller" <davem@xxxxxxxxxxxxx>,
     Eric Dumazet <edumazet@xxxxxxxxxx>,
     Jakub Kicinski <kuba@xxxxxxxxxx>,
     linux-s390@xxxxxxxxxxxxxxx,
     netdev@xxxxxxxxxxxxxxx,
     linux-kernel@xxxxxxxxxxxxxxx

Alexandra,

> After iucv_sever_path it should not be possible to call
> iucv_process_message_q() anymore.

For the close path, yes -- iucv_sock_close() ends at IUCV_CLOSED/SOCK_ZAPPED
and recvmsg() is out, so message_q just leaks. The peer-sever path is
different: iucv_callback_connrej() severs and then sets IUCV_DISCONN, leaving
the socket open. recvmsg()'s early return needs message_q.list empty; with a
saved entry it falls through, and if a datagram is still on sk_receive_queue it
reaches iucv_process_message_q() and hands the freed path to message_receive()
-- pathid is the read.

It isn't a tight free-vs-use race. The core tasklet runs message_pending then
path_severed in arrival order under iucv_table_lock, so the entry is saved with
the path live and the sever frees it in the same pass; neither connrej nor close
drains message_q or purges sk_receive_queue. The stale entry then sits in
message_q.list until the next qualifying recvmsg -- deferred use, not a
nanosecond window. A peer that floods past rcvbuf (which is exactly what spills
into message_q) and then severs before the slow reader catches up owns the
ordering; and where a gap did need stretching, it's a lock-free interval a
blocked reader holds open, so I wouldn't read "narrow on HW" as a bound.

> I agree that it is a message leak [...] not freed anywhere.

Right, same missing drain. On close it's a leak; on connrej the socket stays
readable, so it's a use-after-free on the next recvmsg(). So: a leak from your
side, a reachable UAF from mine, gated to s390/z-VM and its timing. I verified
the pointer lifetime with KASAN (mocked transport) and CBMC, not on z/VM
hardware -- the measured HW trigger is yours to confirm, I'm not claiming it.

> I would have preferred to do that in iucv_sock_close() [...] but this should
> work as well.

Draining at sever covers both callers in one spot; close-side for symmetry with
iucv_sock_alloc() is fine too, no objection to moving it.

I'll fold the reachability point into the receive-path locking rework and send
that RFC to netdev and linux-s390, as you asked.

Thanks,
Bryam