netdev - [Race] data race between dev_ifsioc_locked() and eth_commit_mac_addr

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-next>] [day] [month] [year] [list]

Message-ID: <F35A5D81-F42C-49BC-9F0B-94563C5B7436@purdue.edu>
Date:   Mon, 30 Nov 2020 15:20:02 +0000
From:   "Gong, Sishuai" <sishuai@...due.edu>
To:     "davem@...emloft.net" <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: [Race] data race between dev_ifsioc_locked() and
 eth_commit_mac_addr_change()

Hi,

We found a data race in linux kernel 5.3.11 that we are able to reproduce in x86 under specific interleavings. Currently, we are not sure about the consequence of this race but it seems that the two memcpy can lead to some inconsistency.

------------------------------------------
Writer site

 /tmp/tmp.B7zb7od2zE-5.3.11/extract/linux-5.3.11/net/ethernet/eth.c:307
        298  /**
        299   * eth_commit_mac_addr_change - commit mac change
        300   * @dev: network device
        301   * @p: socket address
        302   */
        303  void eth_commit_mac_addr_change(struct net_device *dev, void *p)
        304  {
        305      struct sockaddr *addr = p;
        306
 ==>    307      memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
        308  }

------------------------------------------
Reader site

 /tmp/tmp.B7zb7od2zE-5.3.11/extract/linux-5.3.11/net/core/dev_ioctl.c:130
        110
        111      switch (cmd) {
        112      case SIOCGIFFLAGS:  /* Get interface flags */
        113          ifr->ifr_flags = (short) dev_get_flags(dev);
        114          return 0;
        115
        116      case SIOCGIFMETRIC: /* Get the metric on the interface
        117                     (currently unused) */
        118          ifr->ifr_metric = 0;
        119          return 0;
        120
        121      case SIOCGIFMTU:    /* Get the MTU of a device */
        122          ifr->ifr_mtu = dev->mtu;
        123          return 0;
        124
        125      case SIOCGIFHWADDR:
        126          if (!dev->addr_len)
        127              memset(ifr->ifr_hwaddr.sa_data, 0,
        128                     sizeof(ifr->ifr_hwaddr.sa_data));
        129          else
 ==>    130              memcpy(ifr->ifr_hwaddr.sa_data, dev->dev_addr,
        131                     min(sizeof(ifr->ifr_hwaddr.sa_data),
        132                     (size_t)dev->addr_len));
        133          ifr->ifr_hwaddr.sa_family = dev->type;
        134          return 0;
        135
        136      case SIOCGIFSLAVE:
        137          err = -EINVAL;
        138          break;
        139
        140      case SIOCGIFMAP:
        141          ifr->ifr_map.mem_start = dev->mem_start;
        142          ifr->ifr_map.mem_end   = dev->mem_end;
        143          ifr->ifr_map.base_addr = dev->base_addr;
        144          ifr->ifr_map.irq       = dev->irq;
        145          ifr->ifr_map.dma       = dev->dma;
        146          ifr->ifr_map.port      = dev->if_port;
        147          return 0;
        148
        149      case SIOCGIFINDEX:
        150          ifr->ifr_ifindex = dev->ifindex;


------------------------------------------
Writer calling trace

- __sys_sendmsg 
-- ___sys_sendmsg 
--- sock_sendmsg
---- netlink_unicast
----- netlink_rcv_skb
------ __rtnl_newlink
------- do_setlink
-------- dev_set_mac_address
--------- eth_commit_mac_addr_change

------------------------------------------
Reader calling trace

- ksys_ioctl
-- do_vfs_ioctl
--- vfs_ioctl
---- dev_ioctl



Thanks,
Sishuai