diff mbox

答复: [scr265482] ip_tunnel.c

Message ID F41877A268BCDE49BAEE2286FA8C269F6FECFBBF@LETV-MBX-IDC09.letv.local
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Liyang Yu (于立洋1) Nov. 24, 2016, 7:45 a.m. UTC
Yeah,I means that recreate the tunnel again, 
But I don’t think the patch can fix the bug. It only can make the first packet received successed. And the follow packet will droped also.
In function __gre_xmit  line 366 
  tunnel->o_seqno++;

If you restart from UINT_MAX, the 'o_seqno' of second packet will return to 0 again. 

BTW:
   Can you read Chinese? :)

On Wed, Nov 23, 2016 at 6:47 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
> Hi:

>         I found that the GRE tunnel in same case can cause integer 

> overflow in ip_tunnel.c:397

>

> Cause of the problem:

>         When tpi->seq less than tunnel->i_seqno, the packet will be droped.

>

> How to recurrence problem

>         1. Create an tunnel use kernel GRE module.

>     2. Use the tunnel to send packets for awile.

>     3.Reboot one site of the tunnel.

>     4. Communication interrupted


What do you mean by "reboot one site of the tunnel"?

If you mean something like delete and create it again, it has nothing related to integer overflow, the tunnel->o_seqno will restart from 0 and the tunnel->i_seqno will remain as it is since we can't detect the interruption of the tunnel traffic.
If so, the following patch could help?

Comments

Cong Wang Nov. 24, 2016, 5:38 p.m. UTC | #1
On Wed, Nov 23, 2016 at 11:45 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
> Yeah,I means that recreate the tunnel again,
> But I don’t think the patch can fix the bug. It only can make the first packet received successed. And the follow packet will droped also.
> In function __gre_xmit  line 366
>   tunnel->o_seqno++;
>
> If you restart from UINT_MAX, the 'o_seqno' of second packet will return to 0 again.

The first packet after restart: o_seqno == UINT_MAX, the other end: i_seqno = 0
The second packet after restart: o_seqno == 0, the other end: i_seqno = 1

So traffic should be back to normal.

UINT_MAX is also what RFC suggests.
diff mbox

Patch

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c index 5719d6b..2738ff2 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -277,6 +277,7 @@  static struct net_device *__ip_tunnel_create(struct net *net,
        tunnel = netdev_priv(dev);
        tunnel->parms = *parms;
        tunnel->net = net;
+       tunnel->o_seqno = UINT_MAX;

        err = register_netdevice(dev);
        if (err)