netdev - [Race] data race between eth_heder_cache_update() and neigh_hh

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: <8B318E86-EED9-4EFE-A921-678532F36BBD@purdue.edu>
Date:   Mon, 30 Nov 2020 15:40: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 eth_heder_cache_update() and
 neigh_hh_output()

Hi,

We found a data race in linux kernel 5.3.11 that we are able to reproduce in x86 under specific interleavings. We are not sure about the consequence of this race now but it seems that the two memcpy() can lead to some inconsistency. We also noticed that both the writer and reader are protected by locks, but the writer is protected using seqlock while the reader is protected by rculock.

------------------------------------------
Write site

 /tmp/tmp.B7zb7od2zE-5.3.11/extract/linux-5.3.11/net/ethernet/eth.c:264
        252  /**
        253   * eth_header_cache_update - update cache entry
        254   * @hh: destination cache entry
        255   * @dev: network device
        256   * @haddr: new hardware address
        257   *
        258   * Called by Address Resolution module to notify changes in address.
        259   */
        260  void eth_header_cache_update(struct hh_cache *hh,
        261                   const struct net_device *dev,
        262                   const unsigned char *haddr)
        263  {
 ==>    264      memcpy(((u8 *) hh->hh_data) + HH_DATA_OFF(sizeof(struct ethhdr)),
        265             haddr, ETH_ALEN);
        266  }
        267  EXPORT_SYMBOL(eth_header_cache_update);

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

/tmp/tmp.B7zb7od2zE-5.3.11/extract/linux-5.3.11/include/net/neighbour.h:481
        463  static inline int neigh_hh_output(const struct hh_cache *hh, struct sk_buff *skb)
        464  {
        465      unsigned int hh_alen = 0;
        466      unsigned int seq;
        467      unsigned int hh_len;
        468
        469      do {
        470          seq = read_seqbegin(&hh->hh_lock);
        471          hh_len = hh->hh_len;
        472          if (likely(hh_len <= HH_DATA_MOD)) {
        473              hh_alen = HH_DATA_MOD;
        474
        475              /* skb_push() would proceed silently if we have room for
        476               * the unaligned size but not for the aligned size:
        477               * check headroom explicitly.
        478               */
        479              if (likely(skb_headroom(skb) >= HH_DATA_MOD)) {
        480                  /* this is inlined by gcc */
 ==>    481                  memcpy(skb->data - HH_DATA_MOD, hh->hh_data,
        482                         HH_DATA_MOD);
        483              }
        484          } else {
        485              hh_alen = HH_DATA_ALIGN(hh_len);
        486
        487              if (likely(skb_headroom(skb) >= hh_alen)) {
        488                  memcpy(skb->data - hh_alen, hh->hh_data,
        489                         hh_alen);
        490              }
        491          }
        492      } while (read_seqretry(&hh->hh_lock, seq));
        493
        494      if (WARN_ON_ONCE(skb_headroom(skb) < hh_alen)) {
        495          kfree_skb(skb);
        496          return NET_XMIT_DROP;
        497      }
        498
        499      __skb_push(skb, hh_len);
        500      return dev_queue_xmit(skb);
        501  }

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

- ksys_ioctl
-- do_vfs_ioctl 
--- vfs_ioctl
---- arp_ioctl
----- arp_req_set
------ neigh_update
------- __neigh_update

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

- __sys_sendto
-- sock_sendmsg
--- inet_sendmsg
---- ip_push_pending_frames
----- ip_send_skb
------ ip_local_out
------- ip_finish_output
-------- __ip_finish_output
--------- ip_finish_output2


Thanks,
Sishuai