netdev - Re: [PATCH net] bonding: fix bond_neigh

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]

Message-Id: <20191209.095046.1448956021137333118.davem@davemloft.net>
Date:   Mon, 09 Dec 2019 09:50:46 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     edumazet@...gle.com
Cc:     netdev@...r.kernel.org, eric.dumazet@...il.com,
        syzkaller@...glegroups.com, j.vosburgh@...il.com,
        vfalico@...il.com, andy@...yhouse.net
Subject: Re: [PATCH net] bonding: fix bond_neigh_init()

From: Eric Dumazet <edumazet@...gle.com>
Date: Sat,  7 Dec 2019 14:10:34 -0800

> 1) syzbot reported an uninit-value in bond_neigh_setup() [1]
> 
>  bond_neigh_setup() uses a temporary on-stack 'struct neigh_parms parms',
>  but only clears parms.neigh_setup field.
> 
>  A stacked bonding device would then enter bond_neigh_setup()
>  and read garbage from parms->dev.
> 
>  If we get really unlucky and garbage is matching @dev, then we
>  could recurse and eventually crash.
> 
>  Let's make sure the whole structure is cleared to avoid surprises.
> 
> 2) bond_neigh_setup() can be called while another cpu manipulates
>  the master device, removing or adding a slave.
>  We need at least rcu protection to prevent use-after-free.
> 
> Note: Prior code does not support a stack of bonding devices,
>       this patch does not attempt to fix this, and leave a comment instead.
 ...
> Fixes: 9918d5bf329d ("bonding: modify only neigh_parms owned by us")
> Fixes: 234bcf8a499e ("net/bonding: correctly proxy slave neigh param setup ndo function")
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Reported-by: syzbot <syzkaller@...glegroups.com>
> Cc: Jay Vosburgh <j.vosburgh@...il.com>
> Cc: Veaceslav Falico <vfalico@...il.com>
> Cc: Andy Gospodarek <andy@...yhouse.net>
> ---
> 
> Note: needs to be applied after "neighbour: remove neigh_cleanup() method" patch

Aha, now I see this.

I moved the neigh_cleanup() removal to net and applied this too, all queued
up for -stable too.

Thanks.