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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unprotected socket state modification from softirq context
--

--- Patch [2]: [PATCH net 2/2] net/iucv: take a private, writable frame before rewriting it in place ---
commit 656ba922e59efdf102e7aa3950857e3c0061f9b8
Author: Bryam Vargas <[email protected]>

net/iucv: take a private, writable frame before rewriting it in place

This commit modifies afiucv_hs_rcv() to take a private, writable copy
of the socket buffer before rewriting the frame in place. It unshares
the skb and uses skb_cow_head() to ensure enough headroom.

Closes: https://sashiko.dev/#/patchset/[email protected]?part=1

> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index e3ec965d96ca7..10cfc5e82f047 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -2069,11 +2069,20 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
>  		return NET_RX_SUCCESS;
>  	}
>  

[ ... ]

[Severity: High]
This is a pre-existing issue, but while reviewing this file I noticed
an unprotected socket state modification:

net/iucv/af_iucv.c:afiucv_hs_callback_txnotify() {
        ...
        if (sk->sk_state == IUCV_CONNECTED) {
                sk->sk_state = IUCV_DISCONN;
                sk->sk_state_change(sk);
        }
        ...
}

Does this modification risk corrupting the socket state?

When qeth hardware issues TX completions, qeth_notify_skbs() is called from
softirq (NAPI) context and invokes afiucv_hs_callback_txnotify().

This modifies sk->sk_state directly without acquiring the socket lock
or checking sock_owned_by_user().

Could this lead to missed state transitions or concurrent execution with
process-context operations that assume exclusive access under lock_sock()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2