[PATCH 4/3 nf,v4] netfilter: nf_conntrack_helper: call .destroy() when helper is unregistered

[PATCH 4/3 nf,v4] netfilter: nf_conntrack_helper: call .destroy() when helper is unregistered

[Pablo Neira Ayuso]

When the helper is removed, call .destroy to release the pptp binding
with gre conntrack entries, otherwise the gre keymap stays with stale
list entries.

Fixes: f09943fefe6b ("[NETFILTER]: nf_conntrack/nf_nat: add PPTP helper port")
Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_helper.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 9a10b3449957..b226291a5c7f 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -241,9 +241,17 @@ EXPORT_SYMBOL_GPL(__nf_ct_try_assign_helper);
 static int unhelp(struct nf_conn *ct, void *me)
 {
 	struct nf_conn_help *help = nfct_help(ct);
+	struct nf_conntrack_helper *helper;
+
+	if (!help)
+		return 0;
 
-	if (help && rcu_dereference_raw(help->helper) == me) {
+	helper = rcu_dereference_raw(help->helper);
+	if (helper == me) {
 		nf_conntrack_event(IPCT_HELPER, ct);
+		if (helper->destroy)
+			helper->destroy(ct);
+
 		RCU_INIT_POINTER(help->helper, NULL);
 	}
 
-- 
2.47.3

Re: [PATCH 4/3 nf,v4] netfilter: nf_conntrack_helper: call .destroy() when helper is unregistered

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> When the helper is removed, call .destroy to release the pptp binding
> with gre conntrack entries, otherwise the gre keymap stays with stale
> list entries.
> 
> Fixes: f09943fefe6b ("[NETFILTER]: nf_conntrack/nf_nat: add PPTP helper port")
> Reported-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nf_conntrack_helper.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
> index 9a10b3449957..b226291a5c7f 100644
> --- a/net/netfilter/nf_conntrack_helper.c
> +++ b/net/netfilter/nf_conntrack_helper.c
> @@ -241,9 +241,17 @@ EXPORT_SYMBOL_GPL(__nf_ct_try_assign_helper);
>  static int unhelp(struct nf_conn *ct, void *me)
>  {
>  	struct nf_conn_help *help = nfct_help(ct);
> +	struct nf_conntrack_helper *helper;
> +
> +	if (!help)
> +		return 0;
>  
> -	if (help && rcu_dereference_raw(help->helper) == me) {
> +	helper = rcu_dereference_raw(help->helper);
> +	if (helper == me) {
>  		nf_conntrack_event(IPCT_HELPER, ct);
> +		if (helper->destroy)
> +			helper->destroy(ct);
> +

There is a nf_ct_helper_destroy() helper that could be used here instead
of open-coding.  Other than that this looks correct.

Re: [PATCH 4/3 nf,v4] netfilter: nf_conntrack_helper: call .destroy() when helper is unregistered

[Pablo Neira Ayuso]

On Tue, May 19, 2026 at 12:17:56AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > When the helper is removed, call .destroy to release the pptp binding
> > with gre conntrack entries, otherwise the gre keymap stays with stale
> > list entries.
> > 
> > Fixes: f09943fefe6b ("[NETFILTER]: nf_conntrack/nf_nat: add PPTP helper port")
> > Reported-by: Florian Westphal <fw@strlen.de>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  net/netfilter/nf_conntrack_helper.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
> > index 9a10b3449957..b226291a5c7f 100644
> > --- a/net/netfilter/nf_conntrack_helper.c
> > +++ b/net/netfilter/nf_conntrack_helper.c
> > @@ -241,9 +241,17 @@ EXPORT_SYMBOL_GPL(__nf_ct_try_assign_helper);
> >  static int unhelp(struct nf_conn *ct, void *me)
> >  {
> >  	struct nf_conn_help *help = nfct_help(ct);
> > +	struct nf_conntrack_helper *helper;
> > +
> > +	if (!help)
> > +		return 0;
> >  
> > -	if (help && rcu_dereference_raw(help->helper) == me) {
> > +	helper = rcu_dereference_raw(help->helper);
> > +	if (helper == me) {
> >  		nf_conntrack_event(IPCT_HELPER, ct);
> > +		if (helper->destroy)
> > +			helper->destroy(ct);
> > +
> 
> There is a nf_ct_helper_destroy() helper that could be used here instead
> of open-coding.  Other than that this looks correct.

Yes, but it is refetching the helper again which seems redundant to
me. I can use it if you prefer though.