Skip to content

Commit 1596dae

Browse files
mfijalkoborkmann
mfijalko
authored and
borkmann
committed
xsk: check IFF_UP earlier in Tx path
Xsk Tx can be triggered via either sendmsg() or poll() syscalls. These two paths share a call to common function xsk_xmit() which has two sanity checks within. A pseudo code example to show the two paths: __xsk_sendmsg() : xsk_poll(): if (unlikely(!xsk_is_bound(xs))) if (unlikely(!xsk_is_bound(xs))) return -ENXIO; return mask; if (unlikely(need_wait)) (...) return -EOPNOTSUPP; xsk_xmit() mark napi id (...) xsk_xmit() xsk_xmit(): if (unlikely(!(xs->dev->flags & IFF_UP))) return -ENETDOWN; if (unlikely(!xs->tx)) return -ENOBUFS; As it can be observed above, in sendmsg() napi id can be marked on interface that was not brought up and this causes a NULL ptr dereference: [31757.505631] BUG: kernel NULL pointer dereference, address: 0000000000000018 [31757.512710] #PF: supervisor read access in kernel mode [31757.517936] #PF: error_code(0x0000) - not-present page [31757.523149] PGD 0 P4D 0 [31757.525726] Oops: 0000 [#1] PREEMPT SMP NOPTI [31757.530154] CPU: 26 PID: 95641 Comm: xdpsock Not tainted 6.2.0-rc5+ #40 [31757.536871] Hardware name: Intel Corporation S2600WFT/S2600WFT, BIOS SE5C620.86B.02.01.0008.031920191559 03/19/2019 [31757.547457] RIP: 0010:xsk_sendmsg+0xde/0x180 [31757.551799] Code: 00 75 a2 48 8b 00 a8 04 75 9b 84 d2 74 69 8b 85 14 01 00 00 85 c0 75 1b 48 8b 85 28 03 00 00 48 8b 80 98 00 00 00 48 8b 40 20 <8b> 40 18 89 85 14 01 00 00 8b bd 14 01 00 00 81 ff 00 01 00 00 0f [31757.570840] RSP: 0018:ffffc90034f27dc0 EFLAGS: 00010246 [31757.576143] RAX: 0000000000000000 RBX: ffffc90034f27e18 RCX: 0000000000000000 [31757.583389] RDX: 0000000000000001 RSI: ffffc90034f27e18 RDI: ffff88984cf3c100 [31757.590631] RBP: ffff88984714a800 R08: ffff88984714a800 R09: 0000000000000000 [31757.597877] R10: 0000000000000001 R11: 0000000000000000 R12: 00000000fffffffa [31757.605123] R13: 0000000000000000 R14: 0000000000000003 R15: 0000000000000000 [31757.612364] FS: 00007fb4c5931180(0000) GS:ffff88afdfa00000(0000) knlGS:0000000000000000 [31757.620571] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [31757.626406] CR2: 0000000000000018 CR3: 000000184b41c003 CR4: 00000000007706e0 [31757.633648] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [31757.640894] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [31757.648139] PKRU: 55555554 [31757.650894] Call Trace: [31757.653385] <TASK> [31757.655524] sock_sendmsg+0x8f/0xa0 [31757.659077] ? sockfd_lookup_light+0x12/0x70 [31757.663416] __sys_sendto+0xfc/0x170 [31757.667051] ? do_sched_setscheduler+0xdb/0x1b0 [31757.671658] __x64_sys_sendto+0x20/0x30 [31757.675557] do_syscall_64+0x38/0x90 [31757.679197] entry_SYSCALL_64_after_hwframe+0x72/0xdc [31757.687969] Code: 8e f6 ff 44 8b 4c 24 2c 4c 8b 44 24 20 41 89 c4 44 8b 54 24 28 48 8b 54 24 18 b8 2c 00 00 00 48 8b 74 24 10 8b 7c 24 08 0f 05 <48> 3d 00 f0 ff ff 77 3a 44 89 e7 48 89 44 24 08 e8 b5 8e f6 ff 48 [31757.707007] RSP: 002b:00007ffd49c73c70 EFLAGS: 00000293 ORIG_RAX: 000000000000002c [31757.714694] RAX: ffffffffffffffda RBX: 000055a996565380 RCX: 00007fb4c5727c16 [31757.721939] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003 [31757.729184] RBP: 0000000000000040 R08: 0000000000000000 R09: 0000000000000000 [31757.736429] R10: 0000000000000040 R11: 0000000000000293 R12: 0000000000000000 [31757.743673] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [31757.754940] </TASK> To fix this, let's make xsk_xmit a function that will be responsible for generic Tx, where RCU is handled accordingly and pull out sanity checks and xs->zc handling. Populate sanity checks to __xsk_sendmsg() and xsk_poll(). Fixes: ca2e1a6 ("xsk: Mark napi_id on sendmsg()") Fixes: 18b1ab7 ("xsk: Fix race at socket teardown") Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com> Link: https://lore.kernel.org/r/20230215143309.13145-1-maciej.fijalkowski@intel.com Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
1 parent df71a42 commit 1596dae

File tree

1 file changed

+33
-26
lines changed

1 file changed

+33
-26
lines changed

‎net/xdp/xsk.c‎

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
511511
return skb;
512512
}
513513

514-
static int xsk_generic_xmit(struct sock *sk)
514+
static int __xsk_generic_xmit(struct sock *sk)
515515
{
516516
struct xdp_sock *xs = xdp_sk(sk);
517517
u32 max_batch = TX_BATCH_SIZE;
@@ -594,22 +594,13 @@ static int xsk_generic_xmit(struct sock *sk)
594594
return err;
595595
}
596596

597-
static int xsk_xmit(struct sock *sk)
597+
static int xsk_generic_xmit(struct sock *sk)
598598
{
599-
struct xdp_sock *xs = xdp_sk(sk);
600599
int ret;
601600

602-
if (unlikely(!(xs->dev->flags & IFF_UP)))
603-
return -ENETDOWN;
604-
if (unlikely(!xs->tx))
605-
return -ENOBUFS;
606-
607-
if (xs->zc)
608-
return xsk_wakeup(xs, XDP_WAKEUP_TX);
609-
610601
/* Drop the RCU lock since the SKB path might sleep. */
611602
rcu_read_unlock();
612-
ret = xsk_generic_xmit(sk);
603+
ret = __xsk_generic_xmit(sk);
613604
/* Reaquire RCU lock before going into common code. */
614605
rcu_read_lock();
615606

@@ -627,17 +618,31 @@ static bool xsk_no_wakeup(struct sock *sk)
627618
#endif
628619
}
629620

621+
static int xsk_check_common(struct xdp_sock *xs)
622+
{
623+
if (unlikely(!xsk_is_bound(xs)))
624+
return -ENXIO;
625+
if (unlikely(!(xs->dev->flags & IFF_UP)))
626+
return -ENETDOWN;
627+
628+
return 0;
629+
}
630+
630631
static int __xsk_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
631632
{
632633
bool need_wait = !(m->msg_flags & MSG_DONTWAIT);
633634
struct sock *sk = sock->sk;
634635
struct xdp_sock *xs = xdp_sk(sk);
635636
struct xsk_buff_pool *pool;
637+
int err;
636638

637-
if (unlikely(!xsk_is_bound(xs)))
638-
return -ENXIO;
639+
err = xsk_check_common(xs);
640+
if (err)
641+
return err;
639642
if (unlikely(need_wait))
640643
return -EOPNOTSUPP;
644+
if (unlikely(!xs->tx))
645+
return -ENOBUFS;
641646

642647
if (sk_can_busy_loop(sk)) {
643648
if (xs->zc)
@@ -649,8 +654,11 @@ static int __xsk_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len
649654
return 0;
650655

651656
pool = xs->pool;
652-
if (pool->cached_need_wakeup & XDP_WAKEUP_TX)
653-
return xsk_xmit(sk);
657+
if (pool->cached_need_wakeup & XDP_WAKEUP_TX) {
658+
if (xs->zc)
659+
return xsk_wakeup(xs, XDP_WAKEUP_TX);
660+
return xsk_generic_xmit(sk);
661+
}
654662
return 0;
655663
}
656664

@@ -670,11 +678,11 @@ static int __xsk_recvmsg(struct socket *sock, struct msghdr *m, size_t len, int
670678
bool need_wait = !(flags & MSG_DONTWAIT);
671679
struct sock *sk = sock->sk;
672680
struct xdp_sock *xs = xdp_sk(sk);
681+
int err;
673682

674-
if (unlikely(!xsk_is_bound(xs)))
675-
return -ENXIO;
676-
if (unlikely(!(xs->dev->flags & IFF_UP)))
677-
return -ENETDOWN;
683+
err = xsk_check_common(xs);
684+
if (err)
685+
return err;
678686
if (unlikely(!xs->rx))
679687
return -ENOBUFS;
680688
if (unlikely(need_wait))
@@ -713,21 +721,20 @@ static __poll_t xsk_poll(struct file *file, struct socket *sock,
713721
sock_poll_wait(file, sock, wait);
714722

715723
rcu_read_lock();
716-
if (unlikely(!xsk_is_bound(xs))) {
717-
rcu_read_unlock();
718-
return mask;
719-
}
724+
if (xsk_check_common(xs))
725+
goto skip_tx;
720726

721727
pool = xs->pool;
722728

723729
if (pool->cached_need_wakeup) {
724730
if (xs->zc)
725731
xsk_wakeup(xs, pool->cached_need_wakeup);
726-
else
732+
else if (xs->tx)
727733
/* Poll needs to drive Tx also in copy mode */
728-
xsk_xmit(sk);
734+
xsk_generic_xmit(sk);
729735
}
730736

737+
skip_tx:
731738
if (xs->rx && !xskq_prod_is_empty(xs->rx))
732739
mask |= EPOLLIN | EPOLLRDNORM;
733740
if (xs->tx && xsk_tx_writeable(xs))

0 commit comments

Comments
 (0)