[v3] xfrm: cleanup error path in xfrm_add_policy()

[PATCH ipsec-next v3] xfrm: cleanup error path in xfrm_add_policy()

Posted by Deepanshu Kartikey 2 months ago

Replace the open-coded manual cleanup in the error path of
xfrm_add_policy() with xfrm_policy_destroy(), which already
handles all the necessary cleanup internally. This is consistent
with how xfrm_policy_construct() handles its own error paths.

The walk.dead flag must be set before calling xfrm_policy_destroy()
as required by BUG_ON(!policy->walk.dead).

Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
v3:
  - Changed prefix to ipsec-next as this is a cleanup
  - Dropped syzbot references as suggested by Sabrina Dubroca
v2:
  - Reworded commit message to reflect cleanup rather than bugfix
    as suggested by Sabrina Dubroca
  - Removed incorrect Fixes: and Closes: tags
  - Corrected subject prefix to PATCH ipsec
---
 net/xfrm/xfrm_user.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index d56450f61669..ae144d1e4a65 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2267,9 +2267,8 @@ static int xfrm_add_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	if (err) {
 		xfrm_dev_policy_delete(xp);
-		xfrm_dev_policy_free(xp);
-		security_xfrm_policy_free(xp->security);
-		kfree(xp);
+		xp->walk.dead = 1;
+		xfrm_policy_destroy(xp);
 		return err;
 	}
 
-- 
2.43.0

Re: [PATCH ipsec-next v3] xfrm: cleanup error path in xfrm_add_policy()

Posted by Deepanshu Kartikey 1 month, 2 weeks ago

On Tue, Apr 14, 2026 at 7:39 AM Deepanshu Kartikey
<kartikey406@gmail.com> wrote:
>
> Replace the open-coded manual cleanup in the error path of
> xfrm_add_policy() with xfrm_policy_destroy(), which already
> handles all the necessary cleanup internally. This is consistent
> with how xfrm_policy_construct() handles its own error paths.
>
> The walk.dead flag must be set before calling xfrm_policy_destroy()
> as required by BUG_ON(!policy->walk.dead).
>
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> v3:
>   - Changed prefix to ipsec-next as this is a cleanup
>   - Dropped syzbot references as suggested by Sabrina Dubroca
> v2:
>   - Reworded commit message to reflect cleanup rather than bugfix
>     as suggested by Sabrina Dubroca
>   - Removed incorrect Fixes: and Closes: tags
>   - Corrected subject prefix to PATCH ipsec
> ---
>  net/xfrm/xfrm_user.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index d56450f61669..ae144d1e4a65 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -2267,9 +2267,8 @@ static int xfrm_add_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
>
>         if (err) {
>                 xfrm_dev_policy_delete(xp);
> -               xfrm_dev_policy_free(xp);
> -               security_xfrm_policy_free(xp->security);
> -               kfree(xp);
> +               xp->walk.dead = 1;
> +               xfrm_policy_destroy(xp);
>                 return err;
>         }
>
> --
> 2.43.0
>
Gentle ping on this patch . Please let me know the status of this patch.
If anything is required from my side

Thanks

Re: [PATCH ipsec-next v3] xfrm: cleanup error path in xfrm_add_policy()

Posted by Steffen Klassert 1 month, 2 weeks ago

On Wed, Apr 29, 2026 at 07:31:40AM +0530, Deepanshu Kartikey wrote:
> On Tue, Apr 14, 2026 at 7:39 AM Deepanshu Kartikey
> <kartikey406@gmail.com> wrote:
> >
> > Replace the open-coded manual cleanup in the error path of
> > xfrm_add_policy() with xfrm_policy_destroy(), which already
> > handles all the necessary cleanup internally. This is consistent
> > with how xfrm_policy_construct() handles its own error paths.
> >
> > The walk.dead flag must be set before calling xfrm_policy_destroy()
> > as required by BUG_ON(!policy->walk.dead).
> >
> > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > ---
> > v3:
> >   - Changed prefix to ipsec-next as this is a cleanup
> >   - Dropped syzbot references as suggested by Sabrina Dubroca
> > v2:
> >   - Reworded commit message to reflect cleanup rather than bugfix
> >     as suggested by Sabrina Dubroca
> >   - Removed incorrect Fixes: and Closes: tags
> >   - Corrected subject prefix to PATCH ipsec
> > ---
> >  net/xfrm/xfrm_user.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> > index d56450f61669..ae144d1e4a65 100644
> > --- a/net/xfrm/xfrm_user.c
> > +++ b/net/xfrm/xfrm_user.c
> > @@ -2267,9 +2267,8 @@ static int xfrm_add_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
> >
> >         if (err) {
> >                 xfrm_dev_policy_delete(xp);
> > -               xfrm_dev_policy_free(xp);
> > -               security_xfrm_policy_free(xp->security);
> > -               kfree(xp);
> > +               xp->walk.dead = 1;
> > +               xfrm_policy_destroy(xp);
> >                 return err;
> >         }
> >
> > --
> > 2.43.0
> >
> Gentle ping on this patch . Please let me know the status of this patch.
> If anything is required from my side

Your patch was submitted during the merge window. The net-next
and ipsec-next trees don't accept patches during this period.

The merge window ended last Sunday with the release of 7.1-rc1.
I prepared the ipsec-next tree for the new development cycle
yesterday. I'll consider your patch now.

Re: [PATCH ipsec-next v3] xfrm: cleanup error path in xfrm_add_policy()

Posted by Steffen Klassert 1 month, 1 week ago

On Wed, Apr 29, 2026 at 09:33:32AM +0200, Steffen Klassert wrote:
> On Wed, Apr 29, 2026 at 07:31:40AM +0530, Deepanshu Kartikey wrote:
> > On Tue, Apr 14, 2026 at 7:39 AM Deepanshu Kartikey
> > <kartikey406@gmail.com> wrote:
> > >
> > > Replace the open-coded manual cleanup in the error path of
> > > xfrm_add_policy() with xfrm_policy_destroy(), which already
> > > handles all the necessary cleanup internally. This is consistent
> > > with how xfrm_policy_construct() handles its own error paths.
> > >
> > > The walk.dead flag must be set before calling xfrm_policy_destroy()
> > > as required by BUG_ON(!policy->walk.dead).
> > >
> > > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > > ---
> > > v3:
> > >   - Changed prefix to ipsec-next as this is a cleanup
> > >   - Dropped syzbot references as suggested by Sabrina Dubroca
> > > v2:
> > >   - Reworded commit message to reflect cleanup rather than bugfix
> > >     as suggested by Sabrina Dubroca
> > >   - Removed incorrect Fixes: and Closes: tags
> > >   - Corrected subject prefix to PATCH ipsec
> > > ---
> > >  net/xfrm/xfrm_user.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> > > index d56450f61669..ae144d1e4a65 100644
> > > --- a/net/xfrm/xfrm_user.c
> > > +++ b/net/xfrm/xfrm_user.c
> > > @@ -2267,9 +2267,8 @@ static int xfrm_add_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
> > >
> > >         if (err) {
> > >                 xfrm_dev_policy_delete(xp);
> > > -               xfrm_dev_policy_free(xp);
> > > -               security_xfrm_policy_free(xp->security);
> > > -               kfree(xp);
> > > +               xp->walk.dead = 1;
> > > +               xfrm_policy_destroy(xp);
> > >                 return err;
> > >         }
> > >
> > > --
> > > 2.43.0
> > >
> > Gentle ping on this patch . Please let me know the status of this patch.
> > If anything is required from my side
> 
> Your patch was submitted during the merge window. The net-next
> and ipsec-next trees don't accept patches during this period.
> 
> The merge window ended last Sunday with the release of 7.1-rc1.
> I prepared the ipsec-next tree for the new development cycle
> yesterday. I'll consider your patch now.

Now applied to ipsec-next, thanks Deepanshu!