ipv4: Return -ENETUNREACH if we can't create route but saddr is valid
Closes: #945023
This commit is contained in:
Salvatore Bonaccorso
parent
014f165375
commit
3e9a6acd20
|
|
@ -7,6 +7,8 @@ linux (4.19.67-3) UNRELEASED; urgency=medium
|
|||
|
||||
[ Salvatore Bonaccorso ]
|
||||
* ixgbe: Fix secpath usage for IPsec TX offload (Closes: #930443)
|
||||
* ipv4: Return -ENETUNREACH if we can't create route but saddr is valid
|
||||
(Closes: #945023)
|
||||
|
||||
[ Bastian Blank ]
|
||||
* [amd64/cloud-amd64] Re-enable RTC drivers. (closes: #931341)
|
||||
|
|
|
|||
90
debian/patches/bugfix/all/ipv4-Return-ENETUNREACH-if-we-can-t-create-route-but.patch
vendored
Normal file
90
debian/patches/bugfix/all/ipv4-Return-ENETUNREACH-if-we-can-t-create-route-but.patch
vendored
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
From: Stefano Brivio <sbrivio@redhat.com>
|
||||
Date: Wed, 16 Oct 2019 20:52:09 +0200
|
||||
Subject: ipv4: Return -ENETUNREACH if we can't create route but saddr is valid
|
||||
Origin: https://git.kernel.org/linus/595e0651d0296bad2491a4a29a7a43eae6328b02
|
||||
Bug-Debian: https://bugs.debian.org/945023
|
||||
|
||||
...instead of -EINVAL. An issue was found with older kernel versions
|
||||
while unplugging a NFS client with pending RPCs, and the wrong error
|
||||
code here prevented it from recovering once link is back up with a
|
||||
configured address.
|
||||
|
||||
Incidentally, this is not an issue anymore since commit 4f8943f80883
|
||||
("SUNRPC: Replace direct task wakeups from softirq context"), included
|
||||
in 5.2-rc7, had the effect of decoupling the forwarding of this error
|
||||
by using SO_ERROR in xs_wake_error(), as pointed out by Benjamin
|
||||
Coddington.
|
||||
|
||||
To the best of my knowledge, this isn't currently causing any further
|
||||
issue, but the error code doesn't look appropriate anyway, and we
|
||||
might hit this in other paths as well.
|
||||
|
||||
In detail, as analysed by Gonzalo Siero, once the route is deleted
|
||||
because the interface is down, and can't be resolved and we return
|
||||
-EINVAL here, this ends up, courtesy of inet_sk_rebuild_header(),
|
||||
as the socket error seen by tcp_write_err(), called by
|
||||
tcp_retransmit_timer().
|
||||
|
||||
In turn, tcp_write_err() indirectly calls xs_error_report(), which
|
||||
wakes up the RPC pending tasks with a status of -EINVAL. This is then
|
||||
seen by call_status() in the SUN RPC implementation, which aborts the
|
||||
RPC call calling rpc_exit(), instead of handling this as a
|
||||
potentially temporary condition, i.e. as a timeout.
|
||||
|
||||
Return -EINVAL only if the input parameters passed to
|
||||
ip_route_output_key_hash_rcu() are actually invalid (this is the case
|
||||
if the specified source address is multicast, limited broadcast or
|
||||
all zeroes), but return -ENETUNREACH in all cases where, at the given
|
||||
moment, the given source address doesn't allow resolving the route.
|
||||
|
||||
While at it, drop the initialisation of err to -ENETUNREACH, which
|
||||
was added to __ip_route_output_key() back then by commit
|
||||
0315e3827048 ("net: Fix behaviour of unreachable, blackhole and
|
||||
prohibit routes"), but actually had no effect, as it was, and is,
|
||||
overwritten by the fib_lookup() return code assignment, and anyway
|
||||
ignored in all other branches, including the if (fl4->saddr) one:
|
||||
I find this rather confusing, as it would look like -ENETUNREACH is
|
||||
the "default" error, while that statement has no effect.
|
||||
|
||||
Also note that after commit fc75fc8339e7 ("ipv4: dont create routes
|
||||
on down devices"), we would get -ENETUNREACH if the device is down,
|
||||
but -EINVAL if the source address is specified and we can't resolve
|
||||
the route, and this appears to be rather inconsistent.
|
||||
|
||||
Reported-by: Stefan Walter <walteste@inf.ethz.ch>
|
||||
Analysed-by: Benjamin Coddington <bcodding@redhat.com>
|
||||
Analysed-by: Gonzalo Siero <gsierohu@redhat.com>
|
||||
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
|
||||
Signed-off-by: David S. Miller <davem@davemloft.net>
|
||||
---
|
||||
net/ipv4/route.c | 9 ++++++---
|
||||
1 file changed, 6 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
|
||||
index 14654876127e..5bc172abd143 100644
|
||||
--- a/net/ipv4/route.c
|
||||
+++ b/net/ipv4/route.c
|
||||
@@ -2470,14 +2470,17 @@ struct rtable *ip_route_output_key_hash_rcu(struct net *net, struct flowi4 *fl4,
|
||||
int orig_oif = fl4->flowi4_oif;
|
||||
unsigned int flags = 0;
|
||||
struct rtable *rth;
|
||||
- int err = -ENETUNREACH;
|
||||
+ int err;
|
||||
|
||||
if (fl4->saddr) {
|
||||
- rth = ERR_PTR(-EINVAL);
|
||||
if (ipv4_is_multicast(fl4->saddr) ||
|
||||
ipv4_is_lbcast(fl4->saddr) ||
|
||||
- ipv4_is_zeronet(fl4->saddr))
|
||||
+ ipv4_is_zeronet(fl4->saddr)) {
|
||||
+ rth = ERR_PTR(-EINVAL);
|
||||
goto out;
|
||||
+ }
|
||||
+
|
||||
+ rth = ERR_PTR(-ENETUNREACH);
|
||||
|
||||
/* I removed check for oif == dev_out->oif here.
|
||||
It was wrong for two reasons:
|
||||
--
|
||||
2.20.1
|
||||
|
||||
|
|
@ -106,6 +106,7 @@ bugfix/all/rtc-s35390a-set-uie_unsupported.patch
|
|||
bugfix/all/dm-disable-discard-if-the-underlying-storage-no-longer-supports-it.patch
|
||||
bugfix/all/xfs-fix-missing-ILOCK-unlock-when-xfs_setattr_nonsiz.patch
|
||||
bugfix/all/ixgbe-Fix-secpath-usage-for-IPsec-TX-offload.patch
|
||||
bugfix/all/ipv4-Return-ENETUNREACH-if-we-can-t-create-route-but.patch
|
||||
|
||||
# Miscellaneous features
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue