[Patch net] team: use a larger struct for mac address

[Patch net] team: use a larger struct for mac address

[Cong Wang]

IPv6 tunnels use sizeof(struct in6_addr) as dev->addr_len,
but in many places especially bonding, we use struct sockaddr
to copy and set mac addr, this could lead to stack out-of-bounds
access.

Fix it by using a larger address storage like bonding.

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 drivers/net/team/team.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 464570409796..3c1cdd57db58 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -60,10 +60,10 @@ static struct team_port *team_port_get_rtnl(const struct net_device *dev)
 static int __set_port_dev_addr(struct net_device *port_dev,
 			       const unsigned char *dev_addr)
 {
-	struct sockaddr addr;
+	struct sockaddr_storage addr;
 
-	memcpy(addr.sa_data, dev_addr, port_dev->addr_len);
-	addr.sa_family = port_dev->type;
+	memcpy(addr.__data, dev_addr, port_dev->addr_len);
+	addr.ss_family = port_dev->type;
 	return dev_set_mac_address(port_dev, &addr);
 }
 
-- 
2.13.0

Re: [Patch net] team: use a larger struct for mac address

[Cong Wang]

On Thu, Jul 20, 2017 at 4:06 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> IPv6 tunnels use sizeof(struct in6_addr) as dev->addr_len,
> but in many places especially bonding, we use struct sockaddr
> to copy and set mac addr, this could lead to stack out-of-bounds
> access.
>
> Fix it by using a larger address storage like bonding.
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

For DaveM:

This patch depends on my previous patch "net: check mac
address length for dev_set_mac_address()", otherwise
dev_set_mac_address() doesn't accept struct sockaddr_storage.

I can add a cast there if you feel this is easier for backport, but
I think we need to backport both.

Sorry for any confusion.

Re: [Patch net] team: use a larger struct for mac address

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2476 bytes --]

Hi Cong,

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Cong-Wang/team-use-a-larger-struct-for-mac-address/20170721-203849
config: i386-randconfig-x019-07211017 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/net/team/team.c: In function '__set_port_dev_addr':
>> drivers/net/team/team.c:67:39: error: passing argument 2 of 'dev_set_mac_address' from incompatible pointer type [-Werror=incompatible-pointer-types]
     return dev_set_mac_address(port_dev, &addr);
                                          ^
   In file included from drivers/net/team/team.c:20:0:
   include/linux/netdevice.h:3290:5: note: expected 'struct sockaddr *' but argument is of type 'struct __kernel_sockaddr_storage *'
    int dev_set_mac_address(struct net_device *, struct sockaddr *);
        ^~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/dev_set_mac_address +67 drivers/net/team/team.c

3d249d4c Jiri Pirko 2011-11-11  55  
3d249d4c Jiri Pirko 2011-11-11  56  /*
1d76efe1 Jiri Pirko 2012-08-17  57   * Since the ability to change device address for open port device is tested in
3d249d4c Jiri Pirko 2011-11-11  58   * team_port_add, this function can be called without control of return value
3d249d4c Jiri Pirko 2011-11-11  59   */
1d76efe1 Jiri Pirko 2012-08-17  60  static int __set_port_dev_addr(struct net_device *port_dev,
3d249d4c Jiri Pirko 2011-11-11  61  			       const unsigned char *dev_addr)
3d249d4c Jiri Pirko 2011-11-11  62  {
44118243 Cong Wang  2017-07-20  63  	struct sockaddr_storage addr;
3d249d4c Jiri Pirko 2011-11-11  64  
44118243 Cong Wang  2017-07-20  65  	memcpy(addr.__data, dev_addr, port_dev->addr_len);
44118243 Cong Wang  2017-07-20  66  	addr.ss_family = port_dev->type;
3d249d4c Jiri Pirko 2011-11-11 @67  	return dev_set_mac_address(port_dev, &addr);
3d249d4c Jiri Pirko 2011-11-11  68  }
3d249d4c Jiri Pirko 2011-11-11  69  

:::::: The code at line 67 was first introduced by commit
:::::: 3d249d4ca7d0ed6629a135ea1ea21c72286c0d80 net: introduce ethernet teaming device

:::::: TO: Jiri Pirko <jpirko@redhat.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27281 bytes --]

Re: [Patch net] team: use a larger struct for mac address

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2479 bytes --]

Hi Cong,

[auto build test WARNING on net/master]

url:    https://github.com/0day-ci/linux/commits/Cong-Wang/team-use-a-larger-struct-for-mac-address/20170721-203849
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   drivers/net/team/team.c: In function '__set_port_dev_addr':
>> drivers/net/team/team.c:67:9: warning: passing argument 2 of 'dev_set_mac_address' from incompatible pointer type
     return dev_set_mac_address(port_dev, &addr);
            ^
   In file included from drivers/net/team/team.c:20:0:
   include/linux/netdevice.h:3290:5: note: expected 'struct sockaddr *' but argument is of type 'struct __kernel_sockaddr_storage *'
    int dev_set_mac_address(struct net_device *, struct sockaddr *);
        ^

vim +/dev_set_mac_address +67 drivers/net/team/team.c

3d249d4c Jiri Pirko 2011-11-11  55  
3d249d4c Jiri Pirko 2011-11-11  56  /*
1d76efe1 Jiri Pirko 2012-08-17  57   * Since the ability to change device address for open port device is tested in
3d249d4c Jiri Pirko 2011-11-11  58   * team_port_add, this function can be called without control of return value
3d249d4c Jiri Pirko 2011-11-11  59   */
1d76efe1 Jiri Pirko 2012-08-17  60  static int __set_port_dev_addr(struct net_device *port_dev,
3d249d4c Jiri Pirko 2011-11-11  61  			       const unsigned char *dev_addr)
3d249d4c Jiri Pirko 2011-11-11  62  {
44118243 Cong Wang  2017-07-20  63  	struct sockaddr_storage addr;
3d249d4c Jiri Pirko 2011-11-11  64  
44118243 Cong Wang  2017-07-20  65  	memcpy(addr.__data, dev_addr, port_dev->addr_len);
44118243 Cong Wang  2017-07-20  66  	addr.ss_family = port_dev->type;
3d249d4c Jiri Pirko 2011-11-11 @67  	return dev_set_mac_address(port_dev, &addr);
3d249d4c Jiri Pirko 2011-11-11  68  }
3d249d4c Jiri Pirko 2011-11-11  69  

:::::: The code at line 67 was first introduced by commit
:::::: 3d249d4ca7d0ed6629a135ea1ea21c72286c0d80 net: introduce ethernet teaming device

:::::: TO: Jiri Pirko <jpirko@redhat.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51034 bytes --]