netdev - Re: [PATCHv1] ipv6: Fix RTM_GETROUTE's interpretation of RTA

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 4 Apr 2012 10:52:04 +0300
From:	Shmulik Ladkani <shmulik.ladkani@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	eric.dumazet@...il.com, kuznet@....inr.ac.ru, jmorris@...ei.org,
	yoshfuji@...ux-ipv6.org, kaber@...sh.net
Subject: Re: [PATCHv1] ipv6: Fix RTM_GETROUTE's interpretation of RTA_IIF to
 be consistent with ipv4

Hi David,

On Sun, 01 Apr 2012 17:30:32 -0400 (EDT) David Miller <davem@...emloft.net> wrote:
> > Fix 'inet6_rtm_getroute()' so that RTA_IIF is interpreted as "lookup a
> > route as if a packet was received on the specified interface", similar
> > to IPv4's 'inet_rtm_getroute()' interpretation.
> > 
> > Reported-by: Ami Koren <amikoren@...oo.com>
> > Signed-off-by: Shmulik Ladkani <shmulik.ladkani@...il.com>
> 
> Applied, thanks.
> 
> > 1) An alternative: construction of an skb within 'inet6_rtm_getroute()'
> >    and then calling 'ip6_route_input()' with the skb as an argument.
> >    Thus, no need to split common code of 'ip6_route_input()'.
> >    Less elegant IMO.
> 
> Agreed.
> 
> > 2) Better name for the new common function 'ip6_route_input_lookup()'
> >    Will happily accept any better suggestions.
> 
> No, it's fine.
> 
> > 3) In IPv4 the 'ip_route_input()' call within 'inet_rtm_getroute()'is
> >    protected by a 'local_bh_disable()' since dawn of history.
> >    Not sure if similar protection needed within 'inet6_rtm_getroute()'.
> 
> Since all the code paths are shared more than on the ipv4 side, both
> output and input route lookups can be done with and without BH
> disabling.

Thanks for the answers.

Looking at the patch again, I suspect I've missed one thing.

After applying patch, the code getting the 'rt' gets placed prior the
'alloc_skb()' - snip of new code:

	if (iif) {
		// some stuff ...

		rt = (struct rt6_info *)ip6_route_input_lookup(net, dev, &fl6,
							       flags);
	} else {
		fl6.flowi6_oif = oif;

		rt = (struct rt6_info *)ip6_route_output(net, NULL, &fl6);
	}

	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
	if (!skb) {
		err = -ENOBUFS;
		goto errout;
	}

In case of 'alloc_skb' failure, shouldn't we issue a
'dst_release(&rt->dst)'?

If correct, I'll submit a fix. Should have spotted this earlier.

Regards,
Shmulik
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html