netdev - dst->obsolete has become pointless

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20111104.230910.520924516201406800.davem@davemloft.net>
Date:	Fri, 04 Nov 2011 23:09:10 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	netdev@...r.kernel.org
CC:	steffen.klassert@...unet.com, timo.teras@....fi
Subject: dst->obsolete has become pointless

While researching the things unearthed by Steffen Klassert wrt. PMTU
handling in the current tree I went to do some research on what the
real story is wrt. dst->obsolete.

And sure enough EVERY SINGLE ipv4 and ipv6 route is created with
obsolete set to -1, so we unconditionally always invoke ->dst_check().

This makes it completely pointless as an optimization to avoid calling
the dst_ops->dst_check() method.  It never triggers.

This stems from Timo's change to make route expiry properly visible
to IPSEC stacked routes:

--
commit d11a4dc18bf41719c9f0d7ed494d295dd2973b92
Author: Timo Teräs <timo.teras@....fi>
Date:   Thu Mar 18 23:20:20 2010 +0000

    ipv4: check rt_genid in dst_check
...
--

Only DecNET creates routes with obsolete initially set to zero, and
therefore only hits ->dst_check() when dst_free is invoked on the route
during a flush of the decnet routing tables.

And actually this is how ipv4 operated before we started using
generation counts instead of flushing the entire table.  IPV6 seems to
always have used the FIB6 tree serial numbers for expiration checking
and therefore always set obsolete to -1 on new routes.

So we can't just get rid of the dst->obsolete check in dst_check() and
__sk_dst_check() because that will break DecNET because DecNET's
->dst_check() handler assumes that if it was called then the route
is obsolete and it just plainly returns NULL to tell the caller the
route is in fact invalid.

The current situation looks quite terrible, because these functions look
like they optimize away the check op call, but in reality for ipv4 and
ipv6 they do not.
--
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