• Björn Töpel's avatar
    xsk: use state member for socket synchronization · 42fddcc7
    Björn Töpel authored
    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 if used in
    conjunction with state check.
    
    In all syscalls and the xsk_rcv path we check if state is
    XSK_BOUND. If that is the case we do a SMP read barrier, and this
    implies that the dev, umem and all rings are correctly setup. Note
    that no READ_ONCE are needed for these variable if used when state is
    XSK_BOUND (plus the read barrier).
    
    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. Now, 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())
    
    Note that dev and queue_id do not need a WRITE_ONCE or READ_ONCE, due
    to the introduce state synchronization (XSK_BOUND plus smp_rmb()).
    
    Introducing the state check also fixes a race, found by syzcaller, in
    xsk_poll() where umem could be accessed when stale.
    Suggested-by: default avatarHillf Danton <hdanton@sina.com>
    Reported-by: syzbot+c82697e3043781e08802@syzkaller.appspotmail.com
    Fixes: 77cd0d7b ("xsk: add support for need_wakeup flag in AF_XDP rings")
    Signed-off-by: default avatarBjörn Töpel <bjorn.topel@intel.com>
    Acked-by: default avatarJonathan Lemon <jonathan.lemon@gmail.com>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    42fddcc7
xsk.c 25.7 KB