Gong, Sishuai <[email protected]> wrote:
> 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.
AFAICS reader uses same seqlock as the writer.
> ------------------------------------------
> 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);
This is called under write_seqlock_bh(hh->hh_lock)
> /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);
This samples the seqcount.
> 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);
[..]
> 492 } while (read_seqretry(&hh->hh_lock, seq));
... so this retries unless seqcount was stable.
