[PATCH net v2] xfrm: iptfs: only publish mode_data after clone setup

Paul Moses posted 1 patch 3 weeks, 5 days ago
There is a newer version of this series
net/xfrm/xfrm_iptfs.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH net v2] xfrm: iptfs: only publish mode_data after clone setup
Posted by Paul Moses 3 weeks, 5 days ago
iptfs_clone_state() stores x->mode_data before allocating the reorder
window. If that allocation fails, the code frees the cloned state and
returns -ENOMEM, leaving x->mode_data pointing at freed memory.

The xfrm clone unwind later runs destroy_state() through x->mode_data,
so the failed clone path tears down IPTFS state that clone_state()
already freed.

Keep the cloned IPTFS state private until all allocations succeed so
failed clones leave x->mode_data unset. The destroy path already
handles a NULL mode_data pointer.

Fixes: 6be02e3e4f37 ("xfrm: iptfs: handle reordering of received packets")
Cc: stable@vger.kernel.org
Signed-off-by: Paul Moses <p@1g4.org>
---
Changes in v2:
- Fix Fixes tag to point to 6be02e3e4f37

 net/xfrm/xfrm_iptfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c
index 050a82101ca51..4d7a925f59b7c 100644
--- a/net/xfrm/xfrm_iptfs.c
+++ b/net/xfrm/xfrm_iptfs.c
@@ -2653,9 +2653,6 @@ static int iptfs_clone_state(struct xfrm_state *x, struct xfrm_state *orig)
 	if (!xtfs)
 		return -ENOMEM;
 
-	x->mode_data = xtfs;
-	xtfs->x = x;
-
 	xtfs->ra_newskb = NULL;
 	if (xtfs->cfg.reorder_win_size) {
 		xtfs->w_saved = kzalloc_objs(*xtfs->w_saved,
@@ -2666,6 +2663,9 @@ static int iptfs_clone_state(struct xfrm_state *x, struct xfrm_state *orig)
 		}
 	}
 
+	x->mode_data = xtfs;
+	xtfs->x = x;
+
 	return 0;
 }
 
-- 
2.53.GIT
Re: [PATCH net v2] xfrm: iptfs: only publish mode_data after clone setup
Posted by Simon Horman 3 weeks, 4 days ago
On Thu, Mar 12, 2026 at 11:39:09AM +0000, Paul Moses wrote:
> iptfs_clone_state() stores x->mode_data before allocating the reorder
> window. If that allocation fails, the code frees the cloned state and
> returns -ENOMEM, leaving x->mode_data pointing at freed memory.
> 
> The xfrm clone unwind later runs destroy_state() through x->mode_data,
> so the failed clone path tears down IPTFS state that clone_state()
> already freed.
> 
> Keep the cloned IPTFS state private until all allocations succeed so
> failed clones leave x->mode_data unset. The destroy path already
> handles a NULL mode_data pointer.
> 
> Fixes: 6be02e3e4f37 ("xfrm: iptfs: handle reordering of received packets")
> Cc: stable@vger.kernel.org
> Signed-off-by: Paul Moses <p@1g4.org>
> ---
> Changes in v2:
> - Fix Fixes tag to point to 6be02e3e4f37

Thanks for the update.

Reviewed-by: Simon Horman <horms@kernel.org>
Re: [PATCH net v2] xfrm: iptfs: only publish mode_data after clone setup
Posted by Steffen Klassert 3 weeks, 4 days ago
On Thu, Mar 12, 2026 at 11:39:09AM +0000, Paul Moses wrote:
> iptfs_clone_state() stores x->mode_data before allocating the reorder
> window. If that allocation fails, the code frees the cloned state and
> returns -ENOMEM, leaving x->mode_data pointing at freed memory.
> 
> The xfrm clone unwind later runs destroy_state() through x->mode_data,
> so the failed clone path tears down IPTFS state that clone_state()
> already freed.
> 
> Keep the cloned IPTFS state private until all allocations succeed so
> failed clones leave x->mode_data unset. The destroy path already
> handles a NULL mode_data pointer.
> 
> Fixes: 6be02e3e4f37 ("xfrm: iptfs: handle reordering of received packets")
> Cc: stable@vger.kernel.org
> Signed-off-by: Paul Moses <p@1g4.org>
> ---
> Changes in v2:
> - Fix Fixes tag to point to 6be02e3e4f37
> 
>  net/xfrm/xfrm_iptfs.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c
> index 050a82101ca51..4d7a925f59b7c 100644
> --- a/net/xfrm/xfrm_iptfs.c
> +++ b/net/xfrm/xfrm_iptfs.c
> @@ -2653,9 +2653,6 @@ static int iptfs_clone_state(struct xfrm_state *x, struct xfrm_state *orig)
>  	if (!xtfs)
>  		return -ENOMEM;
>  
> -	x->mode_data = xtfs;
> -	xtfs->x = x;
> -
>  	xtfs->ra_newskb = NULL;
>  	if (xtfs->cfg.reorder_win_size) {
>  		xtfs->w_saved = kzalloc_objs(*xtfs->w_saved,
> @@ -2666,6 +2663,9 @@ static int iptfs_clone_state(struct xfrm_state *x, struct xfrm_state *orig)
>  		}
>  	}
>  
> +	x->mode_data = xtfs;
> +	xtfs->x = x;
> +
>  	return 0;
>  }

This does not apply to the ipsec tree. Can you rebase on top of the
current ipsec tree?

Thanks!