| From foo@baz Sat Oct 17 01:06:13 PM CEST 2020 |
| From: "Maciej Żenczykowski" <maze@google.com> |
| Date: Wed, 23 Sep 2020 13:18:15 -0700 |
| Subject: net/ipv4: always honour route mtu during forwarding |
| |
| From: "Maciej Żenczykowski" <maze@google.com> |
| |
| [ Upstream commit 02a1b175b0e92d9e0fa5df3957ade8d733ceb6a0 ] |
| |
| Documentation/networking/ip-sysctl.txt:46 says: |
| ip_forward_use_pmtu - BOOLEAN |
| By default we don't trust protocol path MTUs while forwarding |
| because they could be easily forged and can lead to unwanted |
| fragmentation by the router. |
| You only need to enable this if you have user-space software |
| which tries to discover path mtus by itself and depends on the |
| kernel honoring this information. This is normally not the case. |
| Default: 0 (disabled) |
| Possible values: |
| 0 - disabled |
| 1 - enabled |
| |
| Which makes it pretty clear that setting it to 1 is a potential |
| security/safety/DoS issue, and yet it is entirely reasonable to want |
| forwarded traffic to honour explicitly administrator configured |
| route mtus (instead of defaulting to device mtu). |
| |
| Indeed, I can't think of a single reason why you wouldn't want to. |
| Since you configured a route mtu you probably know better... |
| |
| It is pretty common to have a higher device mtu to allow receiving |
| large (jumbo) frames, while having some routes via that interface |
| (potentially including the default route to the internet) specify |
| a lower mtu. |
| |
| Note that ipv6 forwarding uses device mtu unless the route is locked |
| (in which case it will use the route mtu). |
| |
| This approach is not usable for IPv4 where an 'mtu lock' on a route |
| also has the side effect of disabling TCP path mtu discovery via |
| disabling the IPv4 DF (don't frag) bit on all outgoing frames. |
| |
| I'm not aware of a way to lock a route from an IPv6 RA, so that also |
| potentially seems wrong. |
| |
| Signed-off-by: Maciej Żenczykowski <maze@google.com> |
| Cc: Eric Dumazet <edumazet@google.com> |
| Cc: Willem de Bruijn <willemb@google.com> |
| Cc: Lorenzo Colitti <lorenzo@google.com> |
| Cc: Sunmeet Gill (Sunny) <sgill@quicinc.com> |
| Cc: Vinay Paradkar <vparadka@qti.qualcomm.com> |
| Cc: Tyler Wear <twear@quicinc.com> |
| Cc: David Ahern <dsahern@kernel.org> |
| Reviewed-by: Eric Dumazet <edumazet@google.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| include/net/ip.h | 6 ++++++ |
| 1 file changed, 6 insertions(+) |
| |
| --- a/include/net/ip.h |
| +++ b/include/net/ip.h |
| @@ -317,12 +317,18 @@ static inline unsigned int ip_dst_mtu_ma |
| bool forwarding) |
| { |
| struct net *net = dev_net(dst->dev); |
| + unsigned int mtu; |
| |
| if (net->ipv4.sysctl_ip_fwd_use_pmtu || |
| ip_mtu_locked(dst) || |
| !forwarding) |
| return dst_mtu(dst); |
| |
| + /* 'forwarding = true' case should always honour route mtu */ |
| + mtu = dst_metric_raw(dst, RTAX_MTU); |
| + if (mtu) |
| + return mtu; |
| + |
| return min(READ_ONCE(dst->dev->mtu), IP_MAX_MTU); |
| } |
| |