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
| ||
|
Date: Mon, 23 Jun 2014 10:53:47 +0200 From: Hannes Frederic Sowa <hannes@...essinduktion.org> To: roopa@...ulusnetworks.com Cc: davem@...emloft.net, netdev@...r.kernel.org, roque@...fc.ul.pt, yoshfuji@...gi.com Subject: Re: [PATCH net-next] ipv6: fix multipath route cleanup path for duplicate nexthops error case On Fr, 2014-06-20 at 09:05 -0700, roopa@...ulusnetworks.com wrote: > From: Roopa Prabhu <roopa@...ulusnetworks.com> > > Problem: > (route output trimmed to show example): > # ip -6 route add 2001:10:1:3::/64 nexthop via 2001:10:1:2::99 > > # ip -6 route show > 2001:10:1:3::/64 via 2001:10:1:2::99 dev swp26 metric 1024 > > # ip -6 route add 2001:10:1:3::/64 nexthop via 2001:10:1:2::99 > RTNETLINK answers: File exists > > # ip route show > # > /* Previously added route vanished */ > > When the route nexthop add fails with -EXISTS, cleanup path tries > to delete all nexthops added so far. The cleanup logic has a bug. > It tries to delete all the nexthops given by user. In the above case though > the route was added by the user in his previous attempts, just because it > matches with the nexthop in the new request, the previously added route is > deleted. This patch fixes the cleanup path to only delete nexthops that > were successfully added during this request. > > Signed-off-by: Roopa Prabhu <roopa@...ulusnetworks.com> > --- > net/ipv6/route.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > index f23fbd2..51e9b25 100644 > --- a/net/ipv6/route.c > +++ b/net/ipv6/route.c > @@ -2438,10 +2438,13 @@ static int ip6_route_multipath(struct fib6_config *cfg, int add) > int remaining; > int attrlen; > int err = 0, last_err = 0; > + int nh_processed_cnt, nh_processed_cnt_last = 0; > + int rollback = 0; bool rollback = false; > beginning: > rtnh = (struct rtnexthop *)cfg->fc_mp; > remaining = cfg->fc_mp_len; > + nh_processed_cnt = 0; > > /* Parse a Multipath Entry */ > while (rtnh_ok(rtnh, remaining)) { > @@ -2471,6 +2474,10 @@ beginning: > * next hops that have been already added. > */ > add = 0; > + rollback = 1; rollback = true; > + if (!nh_processed_cnt) > + break; > + nh_processed_cnt_last = nh_processed_cnt; > goto beginning; > } > } > @@ -2480,6 +2487,9 @@ beginning: > * fib6_add_rt2node() has reject it). > */ > cfg->fc_nlinfo.nlh->nlmsg_flags &= ~NLM_F_EXCL; > + if (rollback && nh_processed_cnt == nh_processed_cnt_last) > + break; > + nh_processed_cnt++; > rtnh = rtnh_next(rtnh, &remaining); > } > It would be nice if you could bring the comments in that function a bit more in line with your changes. Could you simplify your code a bit if you don't count the routes added but a pointer to the last rtnexthop structure that was successfully processed? I didn't try, just a thought. Thanks, Hannes -- 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
Powered by blists - more mailing lists