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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Concurrent Use-After-Free of `iucv_path` due to incorrect ordering of queue cleanup and path freeing in `iucv_sever_path()`.
--

--- Patch [1]: [PATCH net] net/iucv: fix use-after-free of a severed iucv_path ---
commit 9139c973711b1b0860aa18a8e778aceb20d5b1dd
Author: Bryam Vargas <hexlabsecurity@proton.me>

net/iucv: fix use-after-free of a severed iucv_path

This commit drops queued message notifications in iucv->message_q when an
IUCV path is severed. It acquires the message_q.lock and frees any remaining
notifications that reference the severed path to prevent use-after-free
during a later recvmsg().

Closes: https://sashiko.dev/#/patchset/20260705-b4-disp-fc79c0dc-v1-1-d2cdcb57afa9@proton.me?part=1

> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index fed240b453bd9..2869a103f7fa2 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c

[ ... ]

> @@ -348,6 +349,19 @@ static void iucv_sever_path(struct sock *sk, int with_user_data)
>  		} else
>  			pr_iucv->path_sever(path, NULL);
>  		iucv_path_free(path);

[Severity: High]
This is a pre-existing issue, but does freeing the path before clearing
the queue leave a race window for a concurrent receive?

If a concurrent recvmsg() is running, it could acquire the message_q.lock
and pop a notification containing the freed path pointer before we acquire
the lock here to clear the queue:

CPU1 in iucv_sever_path():
    iucv_path_free(path);
    // preempted or delayed before acquiring message_q.lock
            
CPU2 in iucv_sock_recvmsg():
    // pops notification from message_q
    iucv_process_message_q()
        iucv_process_message(sk, skb, p->path, &p->msg);
            __iucv_message_receive()
                parm->db.ippathid = path->pathid; // use-after-free read
                        
Would it be safer to move iucv_path_free(path) after the spin_unlock_bh
block that drops the queued notifications?

> +
> +		/*
> +		 * Message notifications queued on message_q still reference
> +		 * the now freed path; drop them, otherwise a later recvmsg()
> +		 * would pass the freed iucv_path to message_receive() via
> +		 * iucv_process_message_q().
> +		 */
> +		spin_lock_bh(&iucv->message_q.lock);
> +		list_for_each_entry_safe(p, n, &iucv->message_q.list, list) {
> +			list_del(&p->list);
> +			kfree(p);
> +		}
> +		spin_unlock_bh(&iucv->message_q.lock);
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-b4-disp-783fedbb-v1-1-463b9dbda2ea@proton.me?part=1