[PATCH nf 1/1] netfilter: nf_conntrack_sip: guard against missing skb dst

[PATCH nf 1/1] netfilter: nf_conntrack_sip: guard against missing skb dst

[Ren Wei]

From: Zhengyang Chen <chzhengyang2023@lzu.edu.cn>

set_expected_rtp_rtcp() dereferences skb_dst(skb)->dev when
sip_external_media is enabled. The SIP helper can run from tc ingress
before routing has attached a dst to the skb, so skb_dst(skb) can be
NULL and the helper crashes while parsing SDP media expectations.

Handle a missing skb dst by skipping the same-interface external-media
optimization. Still release the routed media dst when one was obtained,
and keep the existing expectation setup path unchanged.

Fixes: a3419ce3356c ("netfilter: nf_conntrack_sip: add sip_external_media logic")
Cc: stable@vger.kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zhengyang Chen <chzhengyang2023@lzu.edu.cn>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>

---
 net/netfilter/nf_conntrack_sip.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 5ec3a4a4bbd7..302dc60c5381 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -956,7 +956,8 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
 			return NF_ACCEPT;
 		saddr = &ct->tuplehash[!dir].tuple.src.u3;
 	} else if (sip_external_media) {
-		struct net_device *dev = skb_dst(skb)->dev;
+		struct dst_entry *skbdst = skb_dst(skb);
+		struct net_device *dev = skbdst ? skbdst->dev : NULL;
 		struct dst_entry *dst = NULL;
 		struct flowi fl;
 
@@ -977,12 +978,14 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
 		/* Don't predict any conntracks when media endpoint is reachable
 		 * through the same interface as the signalling peer.
 		 */
-		if (dst) {
+		if (dst && dev) {
 			bool external_media = (dst->dev == dev);
 
 			dst_release(dst);
 			if (external_media)
 				return NF_ACCEPT;
+		} else if (dst) {
+			dst_release(dst);
 		}
 	}
 
-- 
2.43.0

Re: [PATCH nf 1/1] netfilter: nf_conntrack_sip: guard against missing skb dst

[Pablo Neira Ayuso]

On Fri, Jun 26, 2026 at 02:49:37PM +0800, Ren Wei wrote:
> From: Zhengyang Chen <chzhengyang2023@lzu.edu.cn>
> 
> set_expected_rtp_rtcp() dereferences skb_dst(skb)->dev when
> sip_external_media is enabled. The SIP helper can run from tc ingress
> before routing has attached a dst to the skb, so skb_dst(skb) can be
> NULL and the helper crashes while parsing SDP media expectations.

If SIP helper can run from tc ingress, then this has not ever worked?
Else tc needs to be fixed to set a router to skb before calling the
helper.

I don't think this fix belong here.

> Handle a missing skb dst by skipping the same-interface external-media
> optimization. Still release the routed media dst when one was obtained,
> and keep the existing expectation setup path unchanged.
> 
> Fixes: a3419ce3356c ("netfilter: nf_conntrack_sip: add sip_external_media logic")
> Cc: stable@vger.kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Zhengyang Chen <chzhengyang2023@lzu.edu.cn>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> 
> ---
>  net/netfilter/nf_conntrack_sip.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
> index 5ec3a4a4bbd7..302dc60c5381 100644
> --- a/net/netfilter/nf_conntrack_sip.c
> +++ b/net/netfilter/nf_conntrack_sip.c
> @@ -956,7 +956,8 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
>  			return NF_ACCEPT;
>  		saddr = &ct->tuplehash[!dir].tuple.src.u3;
>  	} else if (sip_external_media) {
> -		struct net_device *dev = skb_dst(skb)->dev;
> +		struct dst_entry *skbdst = skb_dst(skb);
> +		struct net_device *dev = skbdst ? skbdst->dev : NULL;
>  		struct dst_entry *dst = NULL;
>  		struct flowi fl;
>  
> @@ -977,12 +978,14 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
>  		/* Don't predict any conntracks when media endpoint is reachable
>  		 * through the same interface as the signalling peer.
>  		 */
> -		if (dst) {
> +		if (dst && dev) {
>  			bool external_media = (dst->dev == dev);
>  
>  			dst_release(dst);
>  			if (external_media)
>  				return NF_ACCEPT;
> +		} else if (dst) {
> +			dst_release(dst);
>  		}
>  	}
>  
> -- 
> 2.43.0
> 

Re: [PATCH nf 1/1] netfilter: nf_conntrack_sip: guard against missing skb dst

[Pablo Neira Ayuso]

On Fri, Jun 26, 2026 at 12:47:31PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Jun 26, 2026 at 02:49:37PM +0800, Ren Wei wrote:
> > From: Zhengyang Chen <chzhengyang2023@lzu.edu.cn>
> > 
> > set_expected_rtp_rtcp() dereferences skb_dst(skb)->dev when
> > sip_external_media is enabled. The SIP helper can run from tc ingress
> > before routing has attached a dst to the skb, so skb_dst(skb) can be
> > NULL and the helper crashes while parsing SDP media expectations.
> 
> If SIP helper can run from tc ingress, then this has not ever worked?
> Else tc needs to be fixed to set a router to skb before calling the
> helper.
> 
> I don't think this fix belong here.

Actually, it is the most simple way to fix it here, but I posted a
different approach:

https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260626112449.848283-1-pablo@netfilter.org/

> > Handle a missing skb dst by skipping the same-interface external-media
> > optimization. Still release the routed media dst when one was obtained,
> > and keep the existing expectation setup path unchanged.
> > 
> > Fixes: a3419ce3356c ("netfilter: nf_conntrack_sip: add sip_external_media logic")

I am pointing to different Fixes: tag for practical reasons, to
highlight this is a dependencies for the tc and ovs subsystems.
It seems sip_external_media came _after_ ovs but a bit before tc
act_ct, so a3419ce3356c is not precise either.

> > Cc: stable@vger.kernel.org
> > Reported-by: Yuan Tan <yuantan098@gmail.com>
> > Reported-by: Yifan Wu <yifanwucs@gmail.com>
> > Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> > Reported-by: Xin Liu <bird@lzu.edu.cn>
> > Assisted-by: Codex:gpt-5.4
> > Signed-off-by: Zhengyang Chen <chzhengyang2023@lzu.edu.cn>
> > Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> > 
> > ---
> >  net/netfilter/nf_conntrack_sip.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
> > index 5ec3a4a4bbd7..302dc60c5381 100644
> > --- a/net/netfilter/nf_conntrack_sip.c
> > +++ b/net/netfilter/nf_conntrack_sip.c
> > @@ -956,7 +956,8 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
> >  			return NF_ACCEPT;
> >  		saddr = &ct->tuplehash[!dir].tuple.src.u3;
> >  	} else if (sip_external_media) {
> > -		struct net_device *dev = skb_dst(skb)->dev;
> > +		struct dst_entry *skbdst = skb_dst(skb);
> > +		struct net_device *dev = skbdst ? skbdst->dev : NULL;
> >  		struct dst_entry *dst = NULL;
> >  		struct flowi fl;
> >  
> > @@ -977,12 +978,14 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
> >  		/* Don't predict any conntracks when media endpoint is reachable
> >  		 * through the same interface as the signalling peer.
> >  		 */
> > -		if (dst) {
> > +		if (dst && dev) {
> >  			bool external_media = (dst->dev == dev);
> >  
> >  			dst_release(dst);
> >  			if (external_media)
> >  				return NF_ACCEPT;
> > +		} else if (dst) {
> > +			dst_release(dst);
> >  		}
> >  	}
> >  
> > -- 
> > 2.43.0
> > 

Re: Re: [PATCH nf 1/1] netfilter: nf_conntrack_sip: guard against missing skb dst

[陈正阳]

> -----原始邮件-----
> 发件人: "Pablo Neira Ayuso" <pablo@netfilter.org>
> 发送时间:2026-06-26 19:35:05 (星期五)
> 收件人: "Ren Wei" <n05ec@lzu.edu.cn>
> 抄送: netfilter-devel@vger.kernel.org, fw@strlen.de, phil@nwl.cc, alin.nastac@gmail.com, yuantan098@gmail.com, yifanwucs@gmail.com, tomapufckgml@gmail.com, bird@lzu.edu.cn, chzhengyang2023@lzu.edu.cn
> 主题: Re: [PATCH nf 1/1] netfilter: nf_conntrack_sip: guard against missing skb dst
> 
> On Fri, Jun 26, 2026 at 12:47:31PM +0200, Pablo Neira Ayuso wrote:
> > On Fri, Jun 26, 2026 at 02:49:37PM +0800, Ren Wei wrote:
> > > From: Zhengyang Chen <chzhengyang2023@lzu.edu.cn>
> > > 
> > > set_expected_rtp_rtcp() dereferences skb_dst(skb)->dev when
> > > sip_external_media is enabled. The SIP helper can run from tc ingress
> > > before routing has attached a dst to the skb, so skb_dst(skb) can be
> > > NULL and the helper crashes while parsing SDP media expectations.
> > 
> > If SIP helper can run from tc ingress, then this has not ever worked?
> > Else tc needs to be fixed to set a router to skb before calling the
> > helper.
> > 
> > I don't think this fix belong here.
>
> Actually, it is the most simple way to fix it here, but I posted a
> different approach:
>
> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260626112449.848283-1-pablo@netfilter.org/

I checked your patch and it addresses the issue.

If that patch moves forward, could the following tags please be
considered for our team?

Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Reported-by: Zhengyang Chen <chzhengyang2023@lzu.edu.cn>

> > > Handle a missing skb dst by skipping the same-interface external-media
> > > optimization. Still release the routed media dst when one was obtained,
> > > and keep the existing expectation setup path unchanged.
> > > 
> > > Fixes: a3419ce3356c ("netfilter: nf_conntrack_sip: add sip_external_media logic")
>
> I am pointing to different Fixes: tag for practical reasons, to
> highlight this is a dependencies for the tc and ovs subsystems.
> It seems sip_external_media came _after_ ovs but a bit before tc
> act_ct, so a3419ce3356c is not precise either.

I used a3419ce3356c because it introduced the sip_external_media
branch and the skb_dst() dependency. After checking the affected
entry points, the tc/ovs Fixes tags look more suitable for this
submission because the crash depends on those helper users.

>
> > > Cc: stable@vger.kernel.org
> > > Reported-by: Yuan Tan <yuantan098@gmail.com>
> > > Reported-by: Yifan Wu <yifanwucs@gmail.com>
> > > Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> > > Reported-by: Xin Liu <bird@lzu.edu.cn>
> > > Assisted-by: Codex:gpt-5.4
> > > Signed-off-by: Zhengyang Chen <chzhengyang2023@lzu.edu.cn>
> > > Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> > > 
> > > ---
> > >  net/netfilter/nf_conntrack_sip.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
> > > index 5ec3a4a4bbd7..302dc60c5381 100644
> > > --- a/net/netfilter/nf_conntrack_sip.c
> > > +++ b/net/netfilter/nf_conntrack_sip.c
> > > @@ -956,7 +956,8 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
> > >  			return NF_ACCEPT;
> > >  		saddr = &ct->tuplehash[!dir].tuple.src.u3;
> > >  	} else if (sip_external_media) {
> > > -		struct net_device *dev = skb_dst(skb)->dev;
> > > +		struct dst_entry *skbdst = skb_dst(skb);
> > > +		struct net_device *dev = skbdst ? skbdst->dev : NULL;
> > >  		struct dst_entry *dst = NULL;
> > >  		struct flowi fl;
> > >  
> > > @@ -977,12 +978,14 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
> > >  		/* Don't predict any conntracks when media endpoint is reachable
> > >  		 * through the same interface as the signalling peer.
> > >  		 */
> > > -		if (dst) {
> > > +		if (dst && dev) {
> > >  			bool external_media = (dst->dev == dev);
> > >  
> > >  			dst_release(dst);
> > >  			if (external_media)
> > >  				return NF_ACCEPT;
> > > +		} else if (dst) {
> > > +			dst_release(dst);
> > >  		}
> > >  	}
> > >  
> > > -- 
> > > 2.43.0
> > >