Message ID | 20200923201815.388347-1-zenczykowski@gmail.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [v3] net/ipv4: always honour route mtu during forwarding | expand |
On Wed, Sep 23, 2020 at 10:18 PM Maciej Żenczykowski <zenczykowski@gmail.com> wrote: > > From: Maciej Żenczykowski <maze@google.com> > > 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 > > > Signed-off-by: Maciej Żenczykowski <maze@google.com> Reviewed-by: Eric Dumazet <edumazet@google.com>
From: Maciej Żenczykowski <zenczykowski@gmail.com> Date: Wed, 23 Sep 2020 13:18:15 -0700 > From: Maciej Żenczykowski <maze@google.com> > > 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> Applied and queued up for -stable, thank you.
diff --git a/include/net/ip.h b/include/net/ip.h index b09c48d862cc..2a52787db64a 100644 --- a/include/net/ip.h +++ b/include/net/ip.h @@ -436,12 +436,18 @@ static inline unsigned int ip_dst_mtu_maybe_forward(const struct dst_entry *dst, 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); }