All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net/iucv: fix use-after-free of a severed iucv_path
@ 2026-07-07  7:00 ` Bryam Vargas via B4 Relay
  0 siblings, 0 replies; 5+ messages in thread
From: Bryam Vargas @ 2026-07-07  7:00 UTC (permalink / raw)
  To: Paolo Abeni, Thorsten Winkler, Jakub Kicinski, Alexandra Winter,
	Eric Dumazet, David S. Miller
  Cc: linux-s390, Hidayath Khan, linux-kernel, netdev, Simon Horman,
	Nagamani PV

af_iucv queues not-yet-received message notifications on iucv->message_q,
each holding a raw pointer to the connection's iucv_path.  When the peer
severs the connection, iucv_sever_path() frees that path with
iucv_path_free() but leaves the notifications queued.  A later recvmsg()
drains message_q via iucv_process_message_q() and hands the stale path to
message_receive() -- a use-after-free of the freed iucv_path.

Drop the queued notifications when the path is severed; once the path is
gone they can no longer be received.  This also frees the notifications
leaked when a socket is closed with messages still queued.

Fixes: f0703c80e515 ("[AF_IUCV]: postpone receival of iucv-packets")
Closes: https://sashiko.dev/#/patchset/20260705-b4-disp-fc79c0dc-v1-1-d2cdcb57afa9@proton.me?part=1
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 net/iucv/af_iucv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index fed240b453bd..2869a103f7fa 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -337,6 +337,7 @@ static void iucv_sever_path(struct sock *sk, int with_user_data)
 	unsigned char user_data[16];
 	struct iucv_sock *iucv = iucv_sk(sk);
 	struct iucv_path *path = iucv->path;
+	struct sock_msg_q *p, *n;
 
 	/* Whoever resets the path pointer, must sever and free it. */
 	if (xchg(&iucv->path, NULL)) {
@@ -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);
+
+		/*
+		 * 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);
 	}
 }
 

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260707-b4-disp-783fedbb-39bc1bb9d4e0

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net] net/iucv: fix use-after-free of a severed iucv_path
@ 2026-07-07  7:00 ` Bryam Vargas via B4 Relay
  0 siblings, 0 replies; 5+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-07-07  7:00 UTC (permalink / raw)
  To: Paolo Abeni, Thorsten Winkler, Jakub Kicinski, Alexandra Winter,
	Eric Dumazet, David S. Miller
  Cc: linux-s390, Hidayath Khan, linux-kernel, netdev, Simon Horman,
	Nagamani PV

From: Bryam Vargas <hexlabsecurity@proton.me>

af_iucv queues not-yet-received message notifications on iucv->message_q,
each holding a raw pointer to the connection's iucv_path.  When the peer
severs the connection, iucv_sever_path() frees that path with
iucv_path_free() but leaves the notifications queued.  A later recvmsg()
drains message_q via iucv_process_message_q() and hands the stale path to
message_receive() -- a use-after-free of the freed iucv_path.

Drop the queued notifications when the path is severed; once the path is
gone they can no longer be received.  This also frees the notifications
leaked when a socket is closed with messages still queued.

Fixes: f0703c80e515 ("[AF_IUCV]: postpone receival of iucv-packets")
Closes: https://sashiko.dev/#/patchset/20260705-b4-disp-fc79c0dc-v1-1-d2cdcb57afa9@proton.me?part=1
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 net/iucv/af_iucv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index fed240b453bd..2869a103f7fa 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -337,6 +337,7 @@ static void iucv_sever_path(struct sock *sk, int with_user_data)
 	unsigned char user_data[16];
 	struct iucv_sock *iucv = iucv_sk(sk);
 	struct iucv_path *path = iucv->path;
+	struct sock_msg_q *p, *n;
 
 	/* Whoever resets the path pointer, must sever and free it. */
 	if (xchg(&iucv->path, NULL)) {
@@ -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);
+
+		/*
+		 * 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);
 	}
 }
 

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260707-b4-disp-783fedbb-39bc1bb9d4e0

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net] net/iucv: fix use-after-free of a severed iucv_path
  2026-07-07  7:00 ` Bryam Vargas via B4 Relay
  (?)
@ 2026-07-08  7:01 ` sashiko-bot
  -1 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-08  7:01 UTC (permalink / raw)
  To: Bryam Vargas; +Cc: Vasily Gorbik, Alexander Gordeev, linux-s390, Heiko Carstens

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] net/iucv: fix use-after-free of a severed iucv_path
  2026-07-07  7:00 ` Bryam Vargas via B4 Relay
  (?)
  (?)
@ 2026-07-21  7:30 ` patchwork-bot+netdevbpf
  2026-07-21 11:37   ` Alexandra Winter
  -1 siblings, 1 reply; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-07-21  7:30 UTC (permalink / raw)
  To: Bryam Vargas
  Cc: pabeni, twinkler, kuba, wintera, edumazet, davem, linux-s390,
	hidayath, linux-kernel, netdev, horms, nagamani

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 07 Jul 2026 02:00:54 -0500 you wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> af_iucv queues not-yet-received message notifications on iucv->message_q,
> each holding a raw pointer to the connection's iucv_path.  When the peer
> severs the connection, iucv_sever_path() frees that path with
> iucv_path_free() but leaves the notifications queued.  A later recvmsg()
> drains message_q via iucv_process_message_q() and hands the stale path to
> message_receive() -- a use-after-free of the freed iucv_path.
> 
> [...]

Here is the summary with links:
  - [net] net/iucv: fix use-after-free of a severed iucv_path
    https://git.kernel.org/netdev/net/c/be7cc4656eb1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] net/iucv: fix use-after-free of a severed iucv_path
  2026-07-21  7:30 ` patchwork-bot+netdevbpf
@ 2026-07-21 11:37   ` Alexandra Winter
  0 siblings, 0 replies; 5+ messages in thread
From: Alexandra Winter @ 2026-07-21 11:37 UTC (permalink / raw)
  To: patchwork-bot+netdevbpf, Bryam Vargas
  Cc: pabeni, twinkler, kuba, edumazet, davem, linux-s390, hidayath,
	linux-kernel, netdev, horms, nagamani



On 21.07.26 09:30, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This patch was applied to netdev/net.git (main)
> by Paolo Abeni <pabeni@redhat.com>:
> 
-- Original Patch for reference:  --
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> af_iucv queues not-yet-received message notifications on iucv->message_q,
> each holding a raw pointer to the connection's iucv_path.  When the peer
> severs the connection, iucv_sever_path() frees that path with
> iucv_path_free() but leaves the notifications queued.  A later recvmsg()
> drains message_q via iucv_process_message_q() and hands the stale path to
> message_receive() -- a use-after-free of the freed iucv_path.
>
> Drop the queued notifications when the path is severed; once the path is
> gone they can no longer be received.  This also frees the notifications
> leaked when a socket is closed with messages still queued.
>
> Fixes: f0703c80e515 ("[AF_IUCV]: postpone receival of iucv-packets")
> Closes: https://sashiko.dev/#/patchset/20260705-b4-disp-fc79c0dc-v1-1-d2cdcb57afa9@proton.me?part=1
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  net/iucv/af_iucv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index fed240b453bd..2869a103f7fa 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -337,6 +337,7 @@ static void iucv_sever_path(struct sock *sk, int with_user_data)
>  	unsigned char user_data[16];
>  	struct iucv_sock *iucv = iucv_sk(sk);
>  	struct iucv_path *path = iucv->path;
> +	struct sock_msg_q *p, *n;
>
>  	/* Whoever resets the path pointer, must sever and free it. */
>  	if (xchg(&iucv->path, NULL)) {
> @@ -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);
> +
> +		/*
> +		 * 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);
>  	}
>  }
>
>
--- end of patch --

> 
> Here is the summary with links:
>   - [net] net/iucv: fix use-after-free of a severed iucv_path
>     https://git.kernel.org/netdev/net/c/be7cc4656eb1
> 
> You are awesome, thank you!


Ah, Paolo was faster than me.

@Bryam and for the records,
this should not be a use-after-free. After iucv_server_path it should not be possible to call
iucv_process_message_q() anymore. I agree that it is a message leak, the pending messages indicators
in iucv->message_q are not freed anywhere. I would have preferred to do that in iucv_sock_close()
instead of iucv_sever_path() for symmetry with iucv_sock_alloc(), but this should work as well.

Having said that, as we discussed in [1] the socket locking in af_iucv wrt receive path has
deficiencies, and we will follow up anyhow.

[1] https://lore.kernel.org/all/20260711041119.12764-1-hexlabsecurity@proton.me/




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-21 11:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07  7:00 [PATCH net] net/iucv: fix use-after-free of a severed iucv_path Bryam Vargas
2026-07-07  7:00 ` Bryam Vargas via B4 Relay
2026-07-08  7:01 ` sashiko-bot
2026-07-21  7:30 ` patchwork-bot+netdevbpf
2026-07-21 11:37   ` Alexandra Winter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.