netdev - Re: [PATCH] macvlan: verify MTU before lowerdev xmit

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87fu9g2pdh.fsf@linkitivity.dja.id.au>
Date:   Wed, 15 Nov 2017 14:27:06 +1100
From:   Daniel Axtens <dja@...ens.net>
To:     Shannon Nelson <shannon.nelson@...cle.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] macvlan: verify MTU before lowerdev xmit

Hi Shannon,

> Now that I think about this a little more, why is this not already 
> getting handled by the NETDEV_CHANGEMTU notifier?  In what case are you 
> running into this, and why is it not triggering the notifier?

My commit message was probably not super clear here - apologies for any
mangling of terminology/concepts. The changed MTU is getting propagated
from the lowerdev to the macvtap interface just fine, it's just not
getting enforced.

Hopefully the following diagrams make it clearer.

This is the current behaviour: a VM sends a packet of 1514 bytes and it
makes it through the macvtap/macvlan and lowerdev, to the destination,
even though the intermediate MTUs are lower.

*-------------------*
|        VM         |
|                   |
|     mtu 1500      |
*-------------------*
          |
	  V 1514 byte packet
	  |
*-------------------*
|      macvtap      |
|                   |
|     mtu 1480      |
*-------------------*
          |
	  V 1514 byte packet
	  |
*-------------------*
|     lowerdev      |
|                   |
|     mtu 1480      |
*-------------------*
          |
	  V 1514 byte packet
	  |
*-------------------*
|       dest        |
|                   |
|     mtu 1500      |
*-------------------*

My thought here is that the lowerdev should not be asked to transmit a
packet that is larger than its MTU. The patch causes the following
behaviour:

*-------------------*
|        VM         |
|                   |
|     mtu 1500      |
*-------------------*
          |
	  V 1514 byte packet
	  |
*-------------------*
|      macvtap      |
|                   |
|     mtu 1480      |
*-------------------*
          |
	  | packet dropped
	  X  1500 > 1480

I think this makes macvlan consistent with bridges.

>> As for the other paths, I see that the call to dev_forward_skb() already 
>> has this protection in it, but does the call to dev_queue_xmit_accel() 
>> in macvlan_start_xmit() need similar protection?

I think so. I will have a look and do a v2 if the core idea of the patch
is OK.

Regards,
Daniel

>> 
>
>
> sln
>
>
>> 
>> 
>>>       skb->dev = vlan->lowerdev;
>>>       return dev_queue_xmit(skb);
>>>   }
>>>