History

skbuff: fix a data race in skb_queue_len()

sk_buff.qlen can be accessed concurrently as noticed by KCSAN, BUG: KCSAN: data-race in __skb_try_recv_from_queue / unix_dgram_sendmsg read to 0xffff8a1b1d8a81c0 of 4 bytes by task 5371 on cpu 96: unix_dgram_sendmsg+0x9a9/0xb70 include/linux/skbuff.h:1821 net/unix/af_unix.c:1761 ____sys_sendmsg+0x33e/0x370 ___sys_sendmsg+0xa6/0xf0 __sys_sendmsg+0x69/0xf0 __x64_sys_sendmsg+0x51/0x70 do_syscall_64+0x91/0xb47 entry_SYSCALL_64_after_hwframe+0x49/0xbe write to 0xffff8a1b1d8a81c0 of 4 bytes by task 1 on cpu 99: __skb_try_recv_from_queue+0x327/0x410 include/linux/skbuff.h:2029 __skb_try_recv_datagram+0xbe/0x220 unix_dgram_recvmsg+0xee/0x850 ____sys_recvmsg+0x1fb/0x210 ___sys_recvmsg+0xa2/0xf0 __sys_recvmsg+0x66/0xf0 __x64_sys_recvmsg+0x51/0x70 do_syscall_64+0x91/0xb47 entry_SYSCALL_64_after_hwframe+0x49/0xbe Since only the read is operating as lockless, it could introduce a logic bug in unix_recvq_full() due to the load tearing. Fix it by adding a lockless variant of skb_queue_len() and unix_recvq_full() where READ_ONCE() is on the read while WRITE_ONCE() is on the write similar to the commit d7d16a89350a ("net: add skb_queue_empty_lockless()").

Qian Cai committed 6 years ago

Commit 86b18aa1 parent c35947b2 files · +22 −3Browse files

Showing the changes to include/linux (1 of 2 files). Show all

Unified · Split

include/linux/skbuff.h+13−1
@@ -1821,6 +1821,18 @@ static inline __u32 skb_queue_len(const struct sk_buff_head *list_) · 1820 unmodified lines
1821 return list_->qlen;1821 return list_->qlen;
1822}1822}
1823 1823
1824/**
1825 * skb_queue_len_lockless - get queue length
1826 * @list_: list to measure
1827 *
1828 * Return the length of an &sk_buff queue.
1829 * This variant can be used in lockless contexts.
1830 */
1831static inline __u32 skb_queue_len_lockless(const struct sk_buff_head *list_)
1832{
1833 return READ_ONCE(list_->qlen);
1834}
1835
1824/**1836/**
1825 * __skb_queue_head_init - initialize non-spinlock portions of sk_buff_head1837 * __skb_queue_head_init - initialize non-spinlock portions of sk_buff_head
1826 * @list: queue to initialize1838 * @list: queue to initialize
@@ -2026,7 +2038,7 @@ static inline void __skb_unlink(struct sk_buff *skb, struct sk_buff_head *list) · 199 unmodified lines
2026{2038{
2027 struct sk_buff *next, *prev;2039 struct sk_buff *next, *prev;
2028 2040
2029 list->qlen--;2041 WRITE_ONCE(list->qlen, list->qlen - 1);
2030 next = skb->next;2042 next = skb->next;
2031 prev = skb->prev;2043 prev = skb->prev;
2032 skb->next = skb->prev = NULL;2044 skb->next = skb->prev = NULL;