linux-kernel - Re: [net-next,v2,7/9] net: use core MTU range checking in misc drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 22 Oct 2016 09:17:18 +0200
From:   Sven Eckelmann <sven@...fation.org>
To:     Jarod Wilson <jarod@...hat.com>
Cc:     linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        linux-rdma@...r.kernel.org,
        Stefan Richter <stefanr@...6.in-berlin.de>,
        Faisal Latif <faisal.latif@...el.com>,
        Cliff Whickman <cpw@....com>,
        Robin Holt <robinmholt@...il.com>,
        Jes Sorensen <jes@...ined-monkey.org>,
        Marek Lindner <mareklindner@...mailbox.ch>,
        Simon Wunderlich <sw@...onwunderlich.de>,
        Antonio Quartulli <a@...table.cc>,
        Sathya Prakash <sathya.prakash@...adcom.com>,
        Chaitra P B <chaitra.basappa@...adcom.com>,
        Suganath Prabu Subramani 
        <suganath-prabu.subramani@...adcom.com>,
        MPT-FusionLinux.pdl@...adcom.com,
        Sebastian Reichel <sre@...nel.org>,
        Felipe Balbi <balbi@...nel.org>,
        Arvid Brodin <arvid.brodin@...en.se>,
        Remi Denis-Courmont <courmisch@...il.com>
Subject: Re: [net-next,v2,7/9] net: use core MTU range checking in misc drivers

On Donnerstag, 20. Oktober 2016 13:55:22 CEST Jarod Wilson wrote:
[...]
> batman-adv:
> - set max_mtu
> - remove batadv_interface_change_mtu
> - initialization is a little async, not 100% certain that max_mtu is set
>   in the optimal place, don't have hardware to test with

batman-adv is creating a virtual interface - so there are no
hardware requirements (ok, ethernet compatible hardware - even
when only virtual/emulated).

[...]
> diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
> index 49e16b6..112679d 100644
> --- a/net/batman-adv/soft-interface.c
> +++ b/net/batman-adv/soft-interface.c
> @@ -158,17 +158,6 @@ static int batadv_interface_set_mac_addr(struct net_device *dev, void *p)
>  	return 0;
>  }
>  
> -static int batadv_interface_change_mtu(struct net_device *dev, int new_mtu)
> -{
> -	/* check ranges */
> -	if ((new_mtu < 68) || (new_mtu > batadv_hardif_min_mtu(dev)))
> -		return -EINVAL;
> -
> -	dev->mtu = new_mtu;
> -
> -	return 0;
> -}
> -
>  /**
>   * batadv_interface_set_rx_mode - set the rx mode of a device
>   * @dev: registered network device to modify
> @@ -920,7 +909,6 @@ static const struct net_device_ops batadv_netdev_ops = {
>  	.ndo_vlan_rx_add_vid = batadv_interface_add_vid,
>  	.ndo_vlan_rx_kill_vid = batadv_interface_kill_vid,
>  	.ndo_set_mac_address = batadv_interface_set_mac_addr,
> -	.ndo_change_mtu = batadv_interface_change_mtu,
>  	.ndo_set_rx_mode = batadv_interface_set_rx_mode,
>  	.ndo_start_xmit = batadv_interface_tx,
>  	.ndo_validate_addr = eth_validate_addr,
> @@ -987,6 +975,7 @@ struct net_device *batadv_softif_create(struct net *net, const char *name)
>  	dev_net_set(soft_iface, net);
>  
>  	soft_iface->rtnl_link_ops = &batadv_link_ops;
> +	soft_iface->max_mtu = batadv_hardif_min_mtu(soft_iface);
>  
>  	ret = register_netdevice(soft_iface);
>  	if (ret < 0) {

This looks bogus to me. You are now setting max_mtu during initialization of
the virtual interface. But at this time no slave interfaces were added to the
master batman-adv interface. So the batadv_hardif_min_mtu will not return the
correct value here. Especially if you don't have fragmentation enabled.

So this change looks like a bug to me

Kind regards,
	Sven
Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)