Skip to content
  • Jann Horn's avatar
    dfe719fe
    seccomp: Make duplicate listener detection non-racy · dfe719fe
    Jann Horn authored
    
    
    Currently, init_listener() tries to prevent adding a filter with
    SECCOMP_FILTER_FLAG_NEW_LISTENER if one of the existing filters already
    has a listener. However, this check happens without holding any lock that
    would prevent another thread from concurrently installing a new filter
    (potentially with a listener) on top of the ones we already have.
    
    Theoretically, this is also a data race: The plain load from
    current->seccomp.filter can race with concurrent writes to the same
    location.
    
    Fix it by moving the check into the region that holds the siglock to guard
    against concurrent TSYNC.
    
    (The "Fixes" tag points to the commit that introduced the theoretical
    data race; concurrent installation of another filter with TSYNC only
    became possible later, in commit 51891498 ("seccomp: allow TSYNC and
    USER_NOTIF together").)
    
    Fixes: 6a21cc50 ("seccomp: add a return code to trap to userspace")
    Reviewed-by: default avatarTycho Andersen <tycho@tycho.pizza>
    Signed-off-by: default avatarJann Horn <jannh@google.com>
    Signed-off-by: default avatarKees Cook <keescook@chromium.org>
    Cc: stable@vger.kernel.org
    Link: https://lore.kernel.org/r/20201005014401.490175-1-jannh@google.com
    dfe719fe
    seccomp: Make duplicate listener detection non-racy
    Jann Horn authored
    
    
    Currently, init_listener() tries to prevent adding a filter with
    SECCOMP_FILTER_FLAG_NEW_LISTENER if one of the existing filters already
    has a listener. However, this check happens without holding any lock that
    would prevent another thread from concurrently installing a new filter
    (potentially with a listener) on top of the ones we already have.
    
    Theoretically, this is also a data race: The plain load from
    current->seccomp.filter can race with concurrent writes to the same
    location.
    
    Fix it by moving the check into the region that holds the siglock to guard
    against concurrent TSYNC.
    
    (The "Fixes" tag points to the commit that introduced the theoretical
    data race; concurrent installation of another filter with TSYNC only
    became possible later, in commit 51891498 ("seccomp: allow TSYNC and
    USER_NOTIF together").)
    
    Fixes: 6a21cc50 ("seccomp: add a return code to trap to userspace")
    Reviewed-by: default avatarTycho Andersen <tycho@tycho.pizza>
    Signed-off-by: default avatarJann Horn <jannh@google.com>
    Signed-off-by: default avatarKees Cook <keescook@chromium.org>
    Cc: stable@vger.kernel.org
    Link: https://lore.kernel.org/r/20201005014401.490175-1-jannh@google.com
Loading