path: root/net/xdp/xsk_diag.c
diff options
authorDaniel Borkmann <daniel@iogearbox.net>2019-09-05 14:11:52 +0200
committerDaniel Borkmann <daniel@iogearbox.net>2019-09-05 14:11:53 +0200
commit593f191a8005110e20302039834c116676d69be1 (patch)
tree44b3cf7745546d08d882b8e09403bbe14d6fcf04 /net/xdp/xsk_diag.c
parentselftests/bpf: precision tracking tests (diff)
parentxsk: lock the control mutex in sock_diag interface (diff)
Merge branch 'bpf-af-xdp-barrier-fixes'
Björn Töpel says: ==================== This is a four patch series of various barrier, {READ, WRITE}_ONCE cleanups in the AF_XDP socket code. More details can be found in the corresponding commit message. Previous revisions: v1 [4] and v2 [5]. For an AF_XDP socket, most control plane operations are done under the control mutex (struct xdp_sock, mutex), but there are some places where members of the struct is read outside the control mutex. The dev, queue_id members are set in bind() and cleared at cleanup. The umem, fq, cq, tx, rx, and state member are all assigned in various places, e.g. bind() and setsockopt(). When the members are assigned, they are protected by the control mutex, but since they are read outside the mutex, a WRITE_ONCE is required to avoid store-tearing on the read-side. Prior the state variable was introduced by Ilya, the dev member was used to determine whether the socket was bound or not. However, when dev was read, proper SMP barriers and READ_ONCE were missing. In order to address the missing barriers and READ_ONCE, we start using the state variable as a point of synchronization. The state member read/write is paired with proper SMP barriers, and from this follows that the members described above does not need READ_ONCE statements if used in conjunction with state check. To summarize: The members struct xdp_sock members dev, queue_id, umem, fq, cq, tx, rx, and state were read lock-less, with incorrect barriers and missing {READ, WRITE}_ONCE. After this series umem, fq, cq, tx, rx, and state are read lock-less. When these members are updated, WRITE_ONCE is used. When read, READ_ONCE are only used when read outside the control mutex (e.g. mmap) or, not synchronized with the state member (XSK_BOUND plus smp_rmb()) [1] https://lore.kernel.org/bpf/beef16bb-a09b-40f1-7dd0-c323b4b89b17@iogearbox.net/ [2] https://lwn.net/Articles/793253/ [3] https://github.com/google/ktsan/wiki/READ_ONCE-and-WRITE_ONCE [4] https://lore.kernel.org/bpf/20190822091306.20581-1-bjorn.topel@gmail.com/ [5] https://lore.kernel.org/bpf/20190826061053.15996-1-bjorn.topel@gmail.com/ v2->v3: Minor restructure of commits. Improve cover and commit messages. (Daniel) v1->v2: Removed redundant dev check. (Jonathan) ==================== Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Diffstat (limited to 'net/xdp/xsk_diag.c')
1 files changed, 3 insertions, 0 deletions
diff --git a/net/xdp/xsk_diag.c b/net/xdp/xsk_diag.c
index 9986a759fe06..f59791ba43a0 100644
--- a/net/xdp/xsk_diag.c
+++ b/net/xdp/xsk_diag.c
@@ -97,6 +97,7 @@ static int xsk_diag_fill(struct sock *sk, struct sk_buff *nlskb,
msg->xdiag_ino = sk_ino;
sock_diag_save_cookie(sk, msg->xdiag_cookie);
+ mutex_lock(&xs->mutex);
if ((req->xdiag_show & XDP_SHOW_INFO) && xsk_diag_put_info(xs, nlskb))
goto out_nlmsg_trim;
@@ -117,10 +118,12 @@ static int xsk_diag_fill(struct sock *sk, struct sk_buff *nlskb,
sock_diag_put_meminfo(sk, nlskb, XDP_DIAG_MEMINFO))
goto out_nlmsg_trim;
+ mutex_unlock(&xs->mutex);
nlmsg_end(nlskb, nlh);
return 0;
+ mutex_unlock(&xs->mutex);
nlmsg_cancel(nlskb, nlh);
return -EMSGSIZE;