diff --git a/debian/changelog b/debian/changelog index 9bef1db2a..f89759dc8 100644 --- a/debian/changelog +++ b/debian/changelog @@ -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) diff --git a/debian/patches/bugfix/all/ipv4-Return-ENETUNREACH-if-we-can-t-create-route-but.patch b/debian/patches/bugfix/all/ipv4-Return-ENETUNREACH-if-we-can-t-create-route-but.patch new file mode 100644 index 000000000..a13b7810b --- /dev/null +++ b/debian/patches/bugfix/all/ipv4-Return-ENETUNREACH-if-we-can-t-create-route-but.patch @@ -0,0 +1,90 @@ +From: Stefano Brivio +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 +Analysed-by: Benjamin Coddington +Analysed-by: Gonzalo Siero +Signed-off-by: Stefano Brivio +Signed-off-by: David S. Miller +--- + 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 + diff --git a/debian/patches/series b/debian/patches/series index d59fd8b2d..3f2ff81f2 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -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