Received: by 2002:a25:d7c1:0:0:0:0:0 with SMTP id o184csp2238844ybg; Sun, 27 Oct 2019 14:08:11 -0700 (PDT) X-Google-Smtp-Source: APXvYqz493cirv7IV1fDSdphRGBpom0FciYUUKQi6XwWZ7Uqk2FjUHyy7PYUatIwIvpsUn3TZZ1D X-Received: by 2002:a17:906:49d1:: with SMTP id w17mr13979287ejv.101.1572210491732; Sun, 27 Oct 2019 14:08:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1572210491; cv=none; d=google.com; s=arc-20160816; b=sJOM5GQthYGMBL/EyM5bVFteEOe6tJzMu4i4jVS8A4W6yvRggJkc/n4umurADeE76N Paw25Au+UYo+39OzATHTSX0hvmA/P5V+UF8UPTaSzofoky8QlSw0QBPQIe9clfcxQuQS /HBcEuw1uDDyHwcjMhdGL2oM7f+/U4P9eM9W7hUVGKK6dUqxOgGLBy2C6Gw6zjP/n6Ut cWmHI8zJO8+nmMCo9qpivU2AlBCum3h0xkQ8YhPr/as641FAolMoVlYMRI+K9KRX08NT UaHD9W/cRVLm+F5sH8QRjJCSP1Lg446CgifSYhh5s0HApAgC8umHkmJHTmP0ahPOi/Jm uoBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=QUp1T71aTdANI4OVTjx7fwcCJxJgCM9fP9Z+RfYA5kw=; b=0IA1WD6RtXjj5oM8CQAUKr3ClMSryweefksTUIUHKZJjnjwaocscmmFNJFVsIvnPyX xNUFI67vVFqW+mwPpcEuLm0o/v8HOCs8GDIlUKYjCnkm/9uUFMToPnG76udkZc3sPFqF D/NjQ7YrAhZkop8U/ZnETdzphp/gKDBeCQRHkh9ke4rCIKFu17AkKnt9DN4VDpLUOfXk +Ya63OyX+l0ndTre7D2wD9zGO3cTfbu9+Pdi90KZUZ1FBCgVJd8XghSdErGnUTsLP8xN pP8Wvsyyvf0i9S9eY/wzKk1BM+Cl0zlpuH5Rn7s+1vL1D78Gm8O0jPdoEqY6zKWaq8yP YU+A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=0wdyXsmd; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id b19si5092760ejj.83.2019.10.27.14.07.48; Sun, 27 Oct 2019 14:08:11 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=0wdyXsmd; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728191AbfJ0VC7 (ORCPT + 99 others); Sun, 27 Oct 2019 17:02:59 -0400 Received: from mail.kernel.org ([198.145.29.99]:48104 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728174AbfJ0VC5 (ORCPT ); Sun, 27 Oct 2019 17:02:57 -0400 Received: from localhost (100.50.158.77.rev.sfr.net [77.158.50.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 85E8220873; Sun, 27 Oct 2019 21:02:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1572210176; bh=8gIA3V41/3J43CAYj47tmKFyGTBoCvmym5f6K35owe0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=0wdyXsmdRXXzXKPe0DX67gsZCNEuTjm6IwMYUN3ctI65et8/pxXDBPupvhVKRkWLX IcuhsITtXTqi+iJWiFW/Vo4RqpyNWQhP9CDjLjGge0dOSQqPuneJTAc9r5likHBeZP G8CrHN8wUqB/fw3+CMJJ3/BOVsIepnzU2DNxB7qc= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Stefan Walter , Stefano Brivio , "David S. Miller" , Benjamin Coddington , Gonzalo Siero Subject: [PATCH 4.4 17/41] ipv4: Return -ENETUNREACH if we cant create route but saddr is valid Date: Sun, 27 Oct 2019 22:00:55 +0100 Message-Id: <20191027203114.166284431@linuxfoundation.org> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191027203056.220821342@linuxfoundation.org> References: <20191027203056.220821342@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Stefano Brivio [ Upstream commit 595e0651d0296bad2491a4a29a7a43eae6328b02 ] ...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 Signed-off-by: Greg Kroah-Hartman --- net/ipv4/route.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -2209,7 +2209,7 @@ struct rtable *__ip_route_output_key_has struct fib_result res; struct rtable *rth; int orig_oif; - int err = -ENETUNREACH; + int err; res.tclassid = 0; res.fi = NULL; @@ -2224,11 +2224,14 @@ struct rtable *__ip_route_output_key_has rcu_read_lock(); 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: