[PATCH] platform/surface: aggregator: Fix use-after-free in ssh_ptl_shutdown()

[PATCH] platform/surface: aggregator: Fix use-after-free in ssh_ptl_shutdown()

[Wentao Liang]

ssh_packet_put() is called on the loop variable of both
list_for_each_entry() loops in ssh_ptl_shutdown(), freeing the packet
before the iterator advances past it. Use list_for_each_entry_safe()
and move pending packets to complete_p, as their links use
pending_node, so that each loop walks a single list.

Fixes: c167b9c7e3d6 ("platform/surface: Add Surface Aggregator subsystem")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/platform/surface/aggregator/ssh_packet_layer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/surface/aggregator/ssh_packet_layer.c b/drivers/platform/surface/aggregator/ssh_packet_layer.c
index 3dd22856570f..871780828419 100644
--- a/drivers/platform/surface/aggregator/ssh_packet_layer.c
+++ b/drivers/platform/surface/aggregator/ssh_packet_layer.c
@@ -1983,13 +1983,13 @@ void ssh_ptl_shutdown(struct ssh_ptl *ptl)
 		smp_mb__before_atomic();
 		clear_bit(SSH_PACKET_SF_PENDING_BIT, &p->state);
 
-		list_move_tail(&p->pending_node, &complete_q);
+		list_move_tail(&p->pending_node, &complete_p);
 	}
 	atomic_set(&ptl->pending.count, 0);
 	spin_unlock(&ptl->pending.lock);
 
 	/* Complete and drop packets on complete_q. */
-	list_for_each_entry(p, &complete_q, queue_node) {
+	list_for_each_entry_safe(p, n, &complete_q, queue_node) {
 		if (!test_and_set_bit(SSH_PACKET_SF_COMPLETED_BIT, &p->state))
 			__ssh_ptl_complete(p, -ESHUTDOWN);
 
@@ -1997,7 +1997,7 @@ void ssh_ptl_shutdown(struct ssh_ptl *ptl)
 	}
 
 	/* Complete and drop packets on complete_p. */
-	list_for_each_entry(p, &complete_p, pending_node) {
+	list_for_each_entry_safe(p, n, &complete_p, pending_node) {
 		if (!test_and_set_bit(SSH_PACKET_SF_COMPLETED_BIT, &p->state))
 			__ssh_ptl_complete(p, -ESHUTDOWN);
 
-- 
2.34.1

Re: [PATCH] platform/surface: aggregator: Fix use-after-free in ssh_ptl_shutdown()

[Ilpo Järvinen]

On Thu, 17 Sep 2026, Wentao Liang wrote:

> ssh_packet_put() is called on the loop variable of both
> list_for_each_entry() loops in ssh_ptl_shutdown(), freeing the packet
> before the iterator advances past it. Use list_for_each_entry_safe()
> and move pending packets to complete_p, as their links use
> pending_node, so that each loop walks a single list.

It sounds like there are two independent issues as per your description, 
so two patches would be needed.

> Fixes: c167b9c7e3d6 ("platform/surface: Add Surface Aggregator subsystem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/platform/surface/aggregator/ssh_packet_layer.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/surface/aggregator/ssh_packet_layer.c b/drivers/platform/surface/aggregator/ssh_packet_layer.c
> index 3dd22856570f..871780828419 100644
> --- a/drivers/platform/surface/aggregator/ssh_packet_layer.c
> +++ b/drivers/platform/surface/aggregator/ssh_packet_layer.c
> @@ -1983,13 +1983,13 @@ void ssh_ptl_shutdown(struct ssh_ptl *ptl)
>  		smp_mb__before_atomic();
>  		clear_bit(SSH_PACKET_SF_PENDING_BIT, &p->state);
>  
> -		list_move_tail(&p->pending_node, &complete_q);
> +		list_move_tail(&p->pending_node, &complete_p);
>  	}
>  	atomic_set(&ptl->pending.count, 0);
>  	spin_unlock(&ptl->pending.lock);
>  
>  	/* Complete and drop packets on complete_q. */
> -	list_for_each_entry(p, &complete_q, queue_node) {
> +	list_for_each_entry_safe(p, n, &complete_q, queue_node) {
>  		if (!test_and_set_bit(SSH_PACKET_SF_COMPLETED_BIT, &p->state))
>  			__ssh_ptl_complete(p, -ESHUTDOWN);
>  
> @@ -1997,7 +1997,7 @@ void ssh_ptl_shutdown(struct ssh_ptl *ptl)
>  	}
>  
>  	/* Complete and drop packets on complete_p. */
> -	list_for_each_entry(p, &complete_p, pending_node) {
> +	list_for_each_entry_safe(p, n, &complete_p, pending_node) {
>  		if (!test_and_set_bit(SSH_PACKET_SF_COMPLETED_BIT, &p->state))
>  			__ssh_ptl_complete(p, -ESHUTDOWN);
>  
> 

-- 
 i.