netdev - Re: [PATCH net-next v5] mpls: support for dead routes

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]

Message-Id: <20151125.111827.2264696682456564840.davem@davemloft.net>
Date:	Wed, 25 Nov 2015 11:18:27 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	roopa@...ulusnetworks.com
Cc:	ebiederm@...ssion.com, rshearma@...cade.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v5] mpls: support for dead routes

From: Roopa Prabhu <roopa@...ulusnetworks.com>
Date: Tue, 24 Nov 2015 15:22:22 -0800

> v4 -v5
> 	- if kmemdup fails, modify the original route in place. This is a
> 	corner case and only side effect is that in the remote case
> 	of kmemdup failure, the changes will not be atomically visible
> 	to datapath.

I really don't like this.

Either you need to make the changes appear atomic to the data path,
and you therefore must fail the operation if kmemdup() fails, or it
doesn't matter and you should just always change the route in-place.

As far as I can tell it can't possibly matter.  The alive counter is
never modified by the data path, it is only tested to make a multipath
decision.  Likewise it's rather harmless to send a frame or two via a
device currently going down.

But if you're convinced it matters, then is matters, and you can't
fake things when kmemdup() fails.  And in that case I would recommend
that you use a two pass algorithm, one pass allocates all of the
new routes, and the second fills them in, inserts them, and frees
the old ones.

That is the only way you can unwind and fail cleanly.

And oh yeah, that's right, you can't really fail this and make the
ifdown not proceed.

So you're stuck, right?

That's why this has to be designed in a way where memory allocations
are not necessary.  These notifiers aren't really designed to facilitate
situations that require resource acquisitions that can fail.

--
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