On 8/20/19 12:04 PM, Björn Töpel wrote:
From: Björn Töpel <bjorn.to...@intel.com>The poll() implementation for AF_XDP sockets did not perform the proper state checks, prior accessing the socket umem. This patch fixes that by performing a xsk_is_bound() check. Suggested-by: Hillf Danton <hdan...@sina.com> Reported-by: syzbot+c82697e3043781e08...@syzkaller.appspotmail.com Fixes: 77cd0d7b3f25 ("xsk: add support for need_wakeup flag in AF_XDP rings") Signed-off-by: Björn Töpel <bjorn.to...@intel.com> --- net/xdp/xsk.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c index ee4428a892fa..08bed5e92af4 100644 --- a/net/xdp/xsk.c +++ b/net/xdp/xsk.c @@ -356,13 +356,20 @@ static int xsk_generic_xmit(struct sock *sk, struct msghdr *m, return err; }+static bool xsk_is_bound(struct xdp_sock *xs)+{ + struct net_device *dev = READ_ONCE(xs->dev); + + return dev && xs->state == XSK_BOUND; +} + static int xsk_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len) { bool need_wait = !(m->msg_flags & MSG_DONTWAIT); struct sock *sk = sock->sk; struct xdp_sock *xs = xdp_sk(sk);- if (unlikely(!xs->dev))+ if (unlikely(!xsk_is_bound(xs))) return -ENXIO; if (unlikely(!(xs->dev->flags & IFF_UP))) return -ENETDOWN; @@ -383,6 +390,9 @@ static unsigned int xsk_poll(struct file *file, struct socket *sock, struct net_device *dev = xs->dev; struct xdp_umem *umem = xs->umem;+ if (unlikely(!xsk_is_bound(xs)))+ return mask; + if (umem->need_wakeup) dev->netdev_ops->ndo_xsk_wakeup(dev, xs->queue_id, umem->need_wakeup); @@ -417,7 +427,7 @@ static void xsk_unbind_dev(struct xdp_sock *xs) { struct net_device *dev = xs->dev;- if (!dev || xs->state != XSK_BOUND)+ if (!xsk_is_bound(xs)) return;
I think I'm a bit confused by your READ_ONCE() usage. ;-/ I can see why you're using it in xsk_is_bound() above, but then at the same time all the other callbacks like xsk_poll() or xsk_unbind_dev() above have a struct net_device *dev = xs->dev right before the test. Could you elaborate? Thanks, Daniel
