From: Eric Dumazet <edumazet@google.com>
[ Upstream commit 0dbffbb5335a1e3aa6855e4ee317e25e669dd302 ]
sk_ll_usec is read locklessly from sk_can_busy_loop()
while another thread can change its value in sock_setsockopt()
This is correct but needs annotations.
BUG: KCSAN: data-race in __skb_try_recv_datagram / sock_setsockopt
write to 0xffff88814eb5f904 of 4 bytes by task 14011 on cpu 0:
sock_setsockopt+0x1287/0x2090 net/core/sock.c:1175
__sys_setsockopt+0x14f/0x200 net/socket.c:2100
__do_sys_setsockopt net/socket.c:2115 [inline]
__se_sys_setsockopt net/socket.c:2112 [inline]
__x64_sys_setsockopt+0x62/0x70 net/socket.c:2112
do_syscall_64+0x4a/0x90 arch/x86/entry/common.c:47
entry_SYSCALL_64_after_hwframe+0x44/0xae
read to 0xffff88814eb5f904 of 4 bytes by task 14001 on cpu 1:
sk_can_busy_loop include/net/busy_poll.h:41 [inline]
__skb_try_recv_datagram+0x14f/0x320 net/core/datagram.c:273
unix_dgram_recvmsg+0x14c/0x870 net/unix/af_unix.c:2101
unix_seqpacket_recvmsg+0x5a/0x70 net/unix/af_unix.c:2067
____sys_recvmsg+0x15d/0x310 include/linux/uio.h:244
___sys_recvmsg net/socket.c:2598 [inline]
do_recvmmsg+0x35c/0x9f0 net/socket.c:2692
__sys_recvmmsg net/socket.c:2771 [inline]
__do_sys_recvmmsg net/socket.c:2794 [inline]
__se_sys_recvmmsg net/socket.c:2787 [inline]
__x64_sys_recvmmsg+0xcf/0x150 net/socket.c:2787
do_syscall_64+0x4a/0x90 arch/x86/entry/common.c:47
entry_SYSCALL_64_after_hwframe+0x44/0xae
value changed: 0x00000000 -> 0x00000101
Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 14001 Comm: syz-executor.3 Not tainted 5.13.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <redacted>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/net/busy_poll.h | 2 +-
net/core/sock.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
@@ -1172,7 +1172,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,if(val<0)ret=-EINVAL;else-sk->sk_ll_usec=val;+WRITE_ONCE(sk->sk_ll_usec,val);}break;caseSO_PREFER_BUSY_POLL:
From: Xin Long <lucien.xin@gmail.com>
[ Upstream commit 1d11fa231cabeae09a95cb3e4cf1d9dd34e00f08 ]
The doc draft-stewart-tsvwg-sctp-ipv4-00 that restricts 198 addresses
was never published. These addresses as private addresses should be
allowed to use in SCTP.
As Michael Tuexen suggested, this patch is to move 198 addresses from
unusable to private scope.
Reported-by: Sérgio <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/net/sctp/constants.h | 4 +---
net/sctp/protocol.c | 3 ++-
2 files changed, 3 insertions(+), 4 deletions(-)
@@ -342,8 +342,7 @@ enum {#define SCTP_SCOPE_POLICY_MAX SCTP_SCOPE_POLICY_LINK/* Based on IPv4 scoping <draft-stewart-tsvwg-sctp-ipv4-00.txt>,-*SCTPIPv4unusableaddresses:0.0.0.0/8,224.0.0.0/4,198.18.0.0/24,-*192.88.99.0/24.+*SCTPIPv4unusableaddresses:0.0.0.0/8,224.0.0.0/4,192.88.99.0/24.*Also,RFC8.4,non-unicastaddressesarenotconsideredvalidSCTP*addresses.*/
@@ -351,7 +350,6 @@ enum {((htonl(INADDR_BROADCAST)==a)||\ipv4_is_multicast(a)||\ipv4_is_zeronet(a)||\-ipv4_is_test_198(a)||\ipv4_is_anycast_6to4(a))/* Flags used for the bind address copy functions. */
@@ -60,10 +60,38 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *{structdst_entry*dst=skb_dst(skb);structnet_device*dev=dst->dev;+unsignedinthh_len=LL_RESERVED_SPACE(dev);+intdelta=hh_len-skb_headroom(skb);conststructin6_addr*nexthop;structneighbour*neigh;intret;+/* Be paranoid, rather than too clever. */+if(unlikely(delta>0)&&dev->header_ops){+/* pskb_expand_head() might crash, if skb is shared */+if(skb_shared(skb)){+structsk_buff*nskb=skb_clone(skb,GFP_ATOMIC);++if(likely(nskb)){+if(skb->sk)+skb_set_owner_w(skb,skb->sk);+consume_skb(skb);+}else{+kfree_skb(skb);+}+skb=nskb;+}+if(skb&&+pskb_expand_head(skb,SKB_DATA_ALIGN(delta),0,GFP_ATOMIC)){+kfree_skb(skb);+skb=NULL;+}+if(!skb){+IP6_INC_STATS(net,ip6_dst_idev(dst),IPSTATS_MIB_OUTDISCARDS);+return-ENOMEM;+}+}+if(ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)){structinet6_dev*idev=ip6_dst_idev(skb_dst(skb));
From: Íñigo Huguet <redacted>
[ Upstream commit 788bc000d4c2f25232db19ab3a0add0ba4e27671 ]
Commit 99ba0ea616aa ("sfc: adjust efx->xdp_tx_queue_count with the real
number of initialized queues") intended to fix a problem caused by a
round up when calculating the number of XDP channels and queues.
However, this was not the real problem. The real problem was that the
number of XDP TX queues had been reduced to half in
commit e26ca4b53582 ("sfc: reduce the number of requested xdp ev queues"),
but the variable xdp_tx_queue_count had remained the same.
Once the correct number of XDP TX queues is created again in the
previous patch of this series, this also can be reverted since the error
doesn't actually exist.
Only in the case that there is a bug in the code we can have different
values in xdp_queue_number and efx->xdp_tx_queue_count. Because of this,
and per Edward Cree's suggestion, I add instead a WARN_ON to catch if it
happens again in the future.
Note that the number of allocated queues can be higher than the number
of used ones due to the round up, as explained in the existing comment
in the code. That's why we also have to stop increasing xdp_queue_number
beyond efx->xdp_tx_queue_count.
Signed-off-by: Íñigo Huguet <redacted>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/sfc/efx_channels.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
@@ -891,18 +891,20 @@ int efx_set_channels(struct efx_nic *efx)if(efx_channel_is_xdp_tx(channel)){efx_for_each_channel_tx_queue(tx_queue,channel){tx_queue->queue=next_queue++;-netif_dbg(efx,drv,efx->net_dev,"Channel %u TXQ %u is XDP %u, HW %u\n",-channel->channel,tx_queue->label,-xdp_queue_number,tx_queue->queue);+/* We may have a few left-over XDP TX*queuesowingtoxdp_tx_queue_count*notdividingevenlybyEFX_MAX_TXQ_PER_CHANNEL.*Westillallocateandprobethose*TXQs,butneverusethem.*/-if(xdp_queue_number<efx->xdp_tx_queue_count)+if(xdp_queue_number<efx->xdp_tx_queue_count){+netif_dbg(efx,drv,efx->net_dev,"Channel %u TXQ %u is XDP %u, HW %u\n",+channel->channel,tx_queue->label,+xdp_queue_number,tx_queue->queue);efx->xdp_tx_queues[xdp_queue_number]=tx_queue;-xdp_queue_number++;+xdp_queue_number++;+}}}else{efx_for_each_channel_tx_queue(tx_queue,channel){
@@ -914,8 +916,7 @@ int efx_set_channels(struct efx_nic *efx)}}}-if(xdp_queue_number)-efx->xdp_tx_queue_count=xdp_queue_number;+WARN_ON(xdp_queue_number!=efx->xdp_tx_queue_count);rc=netif_set_real_num_tx_queues(efx->net_dev,efx->n_tx_channels);if(rc)
@@ -60,10 +60,38 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *{structdst_entry*dst=skb_dst(skb);structnet_device*dev=dst->dev;+unsignedinthh_len=LL_RESERVED_SPACE(dev);+intdelta=hh_len-skb_headroom(skb);conststructin6_addr*nexthop;structneighbour*neigh;intret;+/* Be paranoid, rather than too clever. */+if(unlikely(delta>0)&&dev->header_ops){+/* pskb_expand_head() might crash, if skb is shared */+if(skb_shared(skb)){+structsk_buff*nskb=skb_clone(skb,GFP_ATOMIC);++if(likely(nskb)){+if(skb->sk)+skb_set_owner_w(skb,skb->sk);+consume_skb(skb);+}else{+kfree_skb(skb);+}+skb=nskb;+}+if(skb&&+pskb_expand_head(skb,SKB_DATA_ALIGN(delta),0,GFP_ATOMIC)){+kfree_skb(skb);+skb=NULL;+}+if(!skb){+IP6_INC_STATS(net,ip6_dst_idev(dst),IPSTATS_MIB_OUTDISCARDS);+return-ENOMEM;+}+}+if(ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)){structinet6_dev*idev=ip6_dst_idev(skb_dst(skb));
Applying this commit alone could be problematic, leading even to
kernel crash in certain situations.
The real fix is the previous one of the series:
f43a24f446da sfc: fix lack of XDP TX queues - error XDP TX failed (-22)
This one can be applied too, but not really a must-have.
quoted hunk
Commit 99ba0ea616aa ("sfc: adjust efx->xdp_tx_queue_count with the real
number of initialized queues") intended to fix a problem caused by a
round up when calculating the number of XDP channels and queues.
However, this was not the real problem. The real problem was that the
number of XDP TX queues had been reduced to half in
commit e26ca4b53582 ("sfc: reduce the number of requested xdp ev queues"),
but the variable xdp_tx_queue_count had remained the same.
Once the correct number of XDP TX queues is created again in the
previous patch of this series, this also can be reverted since the error
doesn't actually exist.
Only in the case that there is a bug in the code we can have different
values in xdp_queue_number and efx->xdp_tx_queue_count. Because of this,
and per Edward Cree's suggestion, I add instead a WARN_ON to catch if it
happens again in the future.
Note that the number of allocated queues can be higher than the number
of used ones due to the round up, as explained in the existing comment
in the code. That's why we also have to stop increasing xdp_queue_number
beyond efx->xdp_tx_queue_count.
Signed-off-by: Íñigo Huguet <redacted>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/sfc/efx_channels.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
@@ -891,18 +891,20 @@ int efx_set_channels(struct efx_nic *efx)if(efx_channel_is_xdp_tx(channel)){efx_for_each_channel_tx_queue(tx_queue,channel){tx_queue->queue=next_queue++;-netif_dbg(efx,drv,efx->net_dev,"Channel %u TXQ %u is XDP %u, HW %u\n",-channel->channel,tx_queue->label,-xdp_queue_number,tx_queue->queue);+/* We may have a few left-over XDP TX*queuesowingtoxdp_tx_queue_count*notdividingevenlybyEFX_MAX_TXQ_PER_CHANNEL.*Westillallocateandprobethose*TXQs,butneverusethem.*/-if(xdp_queue_number<efx->xdp_tx_queue_count)+if(xdp_queue_number<efx->xdp_tx_queue_count){+netif_dbg(efx,drv,efx->net_dev,"Channel %u TXQ %u is XDP %u, HW %u\n",+channel->channel,tx_queue->label,+xdp_queue_number,tx_queue->queue);efx->xdp_tx_queues[xdp_queue_number]=tx_queue;-xdp_queue_number++;+xdp_queue_number++;+}}}else{efx_for_each_channel_tx_queue(tx_queue,channel){
@@ -914,8 +916,7 @@ int efx_set_channels(struct efx_nic *efx)}}}-if(xdp_queue_number)-efx->xdp_tx_queue_count=xdp_queue_number;+WARN_ON(xdp_queue_number!=efx->xdp_tx_queue_count);rc=netif_set_real_num_tx_queues(efx->net_dev,efx->n_tx_channels);if(rc)--
On Fri, Jul 23, 2021 at 12:12 PM Íñigo Huguet [off-list ref] wrote:
This one can be applied too, but not really a must-have.
Sorry, I have to correct myself. Both must be applied:
58e3bb77bf1b sfc: ensure correct number of XDP queues
f43a24f446da sfc: fix lack of XDP TX queues - error XDP TX failed (-22)
Otherwise, if there are some left-over TXQs because of round up,
xdp_tx_queue_count coud be set to a wrong value, higher than it should
be.
Regards
--
Íñigo Huguet