netdev - Re: xfrm_neigh_lookup and dst->path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170214071335.GI30338@gauss.secunet.com>
Date:   Tue, 14 Feb 2017 08:13:35 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Julian Anastasov <ja@....bg>
CC:     <netdev@...r.kernel.org>
Subject: Re: xfrm_neigh_lookup and dst->path

On Sat, Feb 11, 2017 at 03:12:44PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Fri, 3 Feb 2017, Steffen Klassert wrote:
> 
> > On Thu, Feb 02, 2017 at 01:04:34AM +0200, Julian Anastasov wrote:
> > > 
> > > 	For now I installed common handler, just like
> > > xfrm_neigh_lookup. BTW this function has problem from
> > > commit f894cbf847c9, it looks like dst is wrongly provided,
> > > first arg should be dst->path.
> > 
> > Yes, this should use dst->path of course. I really wonder why
> > nobody noticed this for the last five years.
> 
> 	IMHO, xfrm_neigh_lookup is not used often.

I really wonder if it is used at all, given that this bug exists
for more than five years now and nobody reported it so far.

> If used,
> it should be from a place before skb->dst xforms are popped in
> xfrm_output_one, i.e. somewhere before the output processing.
> I see that dst_neigh_lookup*() is used mostly in xmit handlers
> of tunnel devices, I assume, after xfrm_output_one().
> Not sure for br_nf_pre_routing_finish_bridge().
> 
> 	I'm trying to fix this code but we have the
> same problem as in xfrm_confirm_neigh: to find address
> of last tunnel. Should we use the same loop?

Yes, I think this is the right thing to do.

Thanks!